Hold a reference to collection instead of create one for each request (…

…#16688) ## Description For tenant manager in riddler and get delta in alfred, we resolve collections for every request. The metric for db request is collected periodically and using object created on each request would not collect data properly. Co-authored-by: Xin Zhang <[email protected]>
microsoft · Aug 3, 2023 · 1e991af · 1e991af
1 parent 5bb1f9b
commit 1e991af
Show file tree

Hide file tree

Showing 9 changed files with 46 additions and 61 deletions.
diff --git a/server/routerlicious/packages/routerlicious-base/src/alfred/runnerFactory.ts b/server/routerlicious/packages/routerlicious-base/src/alfred/runnerFactory.ts
@@ -540,7 +540,7 @@ export class AlfredResourcesFactory implements core.IResourcesFactory<AlfredReso
 		// This wanst to create stuff
 		const port = utils.normalizePort(process.env.PORT || "3000");
 
-		const deltaService = new DeltaService(operationsDbMongoManager, tenantManager);
+		const deltaService = new DeltaService(opsCollection, tenantManager);
 		const documentDeleteService =
 			customizations?.documentDeleteService ?? new DocumentDeleteService();
 

diff --git a/server/routerlicious/packages/routerlicious-base/src/alfred/services/deltaService.ts b/server/routerlicious/packages/routerlicious-base/src/alfred/services/deltaService.ts
@@ -6,15 +6,15 @@
 import { toUtf8 } from "@fluidframework/common-utils";
 import { ISequencedDocumentMessage } from "@fluidframework/protocol-definitions";
 import {
+	ICollection,
 	IDeltaService,
 	ISequencedOperationMessage,
 	ITenantManager,
-	MongoManager,
 } from "@fluidframework/server-services-core";
 
 export class DeltaService implements IDeltaService {
 	constructor(
-		protected readonly mongoManager: MongoManager,
+		protected readonly deltasCollection: ICollection<ISequencedOperationMessage>,
 		protected readonly tenantManager: ITenantManager,
 	) {}
 
@@ -68,9 +68,7 @@ export class DeltaService implements IDeltaService {
 		query: any,
 		sort: any,
 	): Promise<ISequencedDocumentMessage[]> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ISequencedOperationMessage>(collectionName);
-		const dbDeltas = await collection.find(query, sort);
+		const dbDeltas = await this.deltasCollection.find(query, sort);
 		return dbDeltas.map((delta) => delta.operation);
 	}
 

diff --git a/server/routerlicious/packages/routerlicious-base/src/riddler/api.ts b/server/routerlicious/packages/routerlicious-base/src/riddler/api.ts
@@ -5,21 +5,20 @@
 
 import { getRandomName } from "@fluidframework/server-services-client";
 import {
-	MongoManager,
 	ISecretManager,
 	ITenantStorage,
 	ITenantOrderer,
 	ITenantCustomData,
 	ICache,
+	ICollection,
 } from "@fluidframework/server-services-core";
 import { handleResponse } from "@fluidframework/server-services";
 import { Router } from "express";
 import { getParam } from "@fluidframework/server-services-utils";
-import { TenantManager } from "./tenantManager";
+import { ITenantDocument, TenantManager } from "./tenantManager";
 
 export function create(
-	collectionName: string,
-	mongoManager: MongoManager,
+	tenantsCollection: ICollection<ITenantDocument>,
 	baseOrderUrl: string,
 	defaultHistorianUrl: string,
 	defaultInternalHistorianUrl: string,
@@ -30,8 +29,7 @@ export function create(
 ): Router {
 	const router: Router = Router();
 	const manager = new TenantManager(
-		mongoManager,
-		collectionName,
+		tenantsCollection,
 		baseOrderUrl,
 		defaultHistorianUrl,
 		defaultInternalHistorianUrl,

diff --git a/server/routerlicious/packages/routerlicious-base/src/riddler/app.ts b/server/routerlicious/packages/routerlicious-base/src/riddler/app.ts
@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { MongoManager, ISecretManager, ICache } from "@fluidframework/server-services-core";
+import { ISecretManager, ICache, ICollection } from "@fluidframework/server-services-core";
 import { BaseTelemetryProperties } from "@fluidframework/server-services-telemetry";
 import * as bodyParser from "body-parser";
 import express from "express";
@@ -14,10 +14,10 @@ import {
 } from "@fluidframework/server-services-utils";
 import { catch404, getTenantIdFromRequest, handleError } from "../utils";
 import * as api from "./api";
+import { ITenantDocument } from "./tenantManager";
 
 export function create(
-	collectionName: string,
-	mongoManager: MongoManager,
+	tenantsCollection: ICollection<ITenantDocument>,
 	loggerFormat: string,
 	baseOrdererUrl: string,
 	defaultHistorianUrl: string,
@@ -52,8 +52,7 @@ export function create(
 	app.use(
 		"/api",
 		api.create(
-			collectionName,
-			mongoManager,
+			tenantsCollection,
 			baseOrdererUrl,
 			defaultHistorianUrl,
 			defaultInternalHistorianUrl,

diff --git a/server/routerlicious/packages/routerlicious-base/src/riddler/runner.ts b/server/routerlicious/packages/routerlicious-base/src/riddler/runner.ts
@@ -5,26 +5,26 @@
 
 import { Deferred } from "@fluidframework/common-utils";
 import {
-	MongoManager,
 	IRunner,
 	ISecretManager,
 	IWebServerFactory,
 	IWebServer,
 	ICache,
+	ICollection,
 } from "@fluidframework/server-services-core";
 import { Lumberjack } from "@fluidframework/server-services-telemetry";
 import * as winston from "winston";
 import * as app from "./app";
+import { ITenantDocument } from "./tenantManager";
 
 export class RiddlerRunner implements IRunner {
 	private server: IWebServer;
 	private runningDeferred: Deferred<void>;
 
 	constructor(
 		private readonly serverFactory: IWebServerFactory,
-		private readonly collectionName: string,
+		private readonly tenantsCollection: ICollection<ITenantDocument>,
 		private readonly port: string | number,
-		private readonly mongoManager: MongoManager,
 		private readonly loggerFormat: string,
 		private readonly baseOrdererUrl: string,
 		private readonly defaultHistorianUrl: string,
@@ -41,8 +41,7 @@ export class RiddlerRunner implements IRunner {
 
 		// Create the HTTP server and attach alfred to it
 		const riddler = app.create(
-			this.collectionName,
-			this.mongoManager,
+			this.tenantsCollection,
 			this.loggerFormat,
 			this.baseOrdererUrl,
 			this.defaultHistorianUrl,

diff --git a/server/routerlicious/packages/routerlicious-base/src/riddler/runnerFactory.ts b/server/routerlicious/packages/routerlicious-base/src/riddler/runnerFactory.ts
@@ -15,6 +15,7 @@ import {
 	IRunner,
 	IRunnerFactory,
 	IWebServerFactory,
+	ICollection,
 } from "@fluidframework/server-services-core";
 import * as utils from "@fluidframework/server-services-utils";
 import { Provider } from "nconf";
@@ -29,6 +30,7 @@ export class RiddlerResources implements IResources {
 
 	constructor(
 		public readonly config: Provider,
+		public readonly tenantsCollection: ICollection<ITenantDocument>,
 		public readonly tenantsCollectionName: string,
 		public readonly mongoManager: MongoManager,
 		public readonly port: any,
@@ -147,6 +149,7 @@ export class RiddlerResourcesFactory implements IResourcesFactory<RiddlerResourc
 
 		return new RiddlerResources(
 			config,
+			collection,
 			tenantsCollectionName,
 			mongoManager,
 			port,
@@ -166,9 +169,8 @@ export class RiddlerRunnerFactory implements IRunnerFactory<RiddlerResources> {
 	public async create(resources: RiddlerResources): Promise<IRunner> {
 		return new RiddlerRunner(
 			resources.webServerFactory,
-			resources.tenantsCollectionName,
+			resources.tenantsCollection,
 			resources.port,
-			resources.mongoManager,
 			resources.loggerFormat,
 			resources.baseOrdererUrl,
 			resources.defaultHistorianUrl,

diff --git a/server/routerlicious/packages/routerlicious-base/src/riddler/tenantManager.ts b/server/routerlicious/packages/routerlicious-base/src/riddler/tenantManager.ts
@@ -13,9 +13,9 @@ import {
 	ITenantOrderer,
 	ITenantStorage,
 	KeyName,
-	MongoManager,
 	ISecretManager,
 	ICache,
+	ICollection,
 } from "@fluidframework/server-services-core";
 import { isNetworkError, NetworkError } from "@fluidframework/server-services-client";
 import { BaseTelemetryProperties, Lumberjack } from "@fluidframework/server-services-telemetry";
@@ -84,8 +84,7 @@ export class TenantManager {
 		Object.values(StorageRequestMetric),
 	);
 	constructor(
-		private readonly mongoManager: MongoManager,
-		private readonly collectionName: string,
+		private readonly tenantsCollection: ICollection<ITenantDocument>,
 		private readonly baseOrdererUrl: string,
 		private readonly defaultHistorianUrl: string,
 		private readonly defaultInternalHistorianUrl: string,
@@ -288,8 +287,6 @@ export class TenantManager {
 		orderer: ITenantOrderer,
 		customData: ITenantCustomData,
 	): Promise<ITenantConfig & { key: string }> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
 		const latestKeyVersion = this.secretManager.getLatestKeyVersion();
 
 		const tenantKey1 = this.generateTenantKey();
@@ -318,7 +315,7 @@ export class TenantManager {
 		}
 
 		const id = await this.runWithDatabaseRequestCounter(async () =>
-			collection.insertOne({
+			this.tenantsCollection.insertOne({
 				_id: tenantId,
 				key: encryptedTenantKey1,
 				secondaryKey: encryptedTenantKey2,
@@ -337,11 +334,8 @@ export class TenantManager {
 	 * Updates the tenant configured storage provider
 	 */
 	public async updateStorage(tenantId: string, storage: ITenantStorage): Promise<ITenantStorage> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
-
 		await this.runWithDatabaseRequestCounter(async () =>
-			collection.update({ _id: tenantId }, { storage }, null),
+			this.tenantsCollection.update({ _id: tenantId }, { storage }, null),
 		);
 
 		return (await this.getTenantDocument(tenantId)).storage;
@@ -351,10 +345,7 @@ export class TenantManager {
 	 * Updates the tenant configured orderer
 	 */
 	public async updateOrderer(tenantId: string, orderer: ITenantOrderer): Promise<ITenantOrderer> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
-
-		await collection.update({ _id: tenantId }, { orderer }, null);
+		await this.tenantsCollection.update({ _id: tenantId }, { orderer }, null);
 
 		return (await this.getTenantDocument(tenantId)).orderer;
 	}
@@ -366,14 +357,12 @@ export class TenantManager {
 		tenantId: string,
 		customData: ITenantCustomData,
 	): Promise<ITenantCustomData> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
 		const accessInfo = customData.externalStorageData?.accessInfo;
 		if (accessInfo) {
 			customData.externalStorageData.accessInfo = this.encryptAccessInfo(accessInfo);
 		}
 		await this.runWithDatabaseRequestCounter(async () =>
-			collection.update({ _id: tenantId }, { customData }, null),
+			this.tenantsCollection.update({ _id: tenantId }, { customData }, null),
 		);
 		const tenantDocument = await this.getTenantDocument(tenantId, true);
 		if (tenantDocument.disabled) {
@@ -534,10 +523,8 @@ export class TenantManager {
 			keyName === KeyName.key2
 				? { secondaryKey: encryptedNewTenantKey }
 				: { key: encryptedNewTenantKey };
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
 		await this.runWithDatabaseRequestCounter(async () =>
-			collection.update({ _id: tenantId }, updateKey, null),
+			this.tenantsCollection.update({ _id: tenantId }, updateKey, null),
 		);
 
 		return tenantKeys;
@@ -662,11 +649,8 @@ export class TenantManager {
 		tenantId: string,
 		includeDisabledTenant = false,
 	): Promise<ITenantDocument> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
-
 		const found = await this.runWithDatabaseRequestCounter(async () =>
-			collection.findOne({ _id: tenantId }),
+			this.tenantsCollection.findOne({ _id: tenantId }),
 		);
 		if (!found || (found.disabled && !includeDisabledTenant)) {
 			return null;
@@ -681,10 +665,9 @@ export class TenantManager {
 	 * Retrieves all the raw database tenant documents
 	 */
 	private async getAllTenantDocuments(includeDisabledTenant = false): Promise<ITenantDocument[]> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
-
-		const allFound = await this.runWithDatabaseRequestCounter(async () => collection.findAll());
+		const allFound = await this.runWithDatabaseRequestCounter(async () =>
+			this.tenantsCollection.findAll(),
+		);
 
 		allFound.forEach((found) => {
 			this.attachDefaultsToTenantDocument(found);
@@ -700,8 +683,6 @@ export class TenantManager {
 	 * If no scheduledDeletionTime is provided the tenant is only soft-deleted.
 	 */
 	public async deleteTenant(tenantId: string, scheduledDeletionTime?: Date): Promise<void> {
-		const db = await this.mongoManager.getDatabase();
-		const collection = db.collection<ITenantDocument>(this.collectionName);
 		const softDelete = !scheduledDeletionTime || scheduledDeletionTime.getTime() > Date.now();
 		if (softDelete) {
 			const query = {
@@ -710,7 +691,7 @@ export class TenantManager {
 			};
 
 			await this.runWithDatabaseRequestCounter(async () =>
-				collection.update(
+				this.tenantsCollection.update(
 					query,
 					{
 						disabled: true,
@@ -721,7 +702,7 @@ export class TenantManager {
 			);
 		} else {
 			await this.runWithDatabaseRequestCounter(async () =>
-				collection.deleteOne({ _id: tenantId }),
+				this.tenantsCollection.deleteOne({ _id: tenantId }),
 			);
 		}
 		// invalidate cache

diff --git a/server/routerlicious/packages/routerlicious-base/src/test/alfred/api.spec.ts b/server/routerlicious/packages/routerlicious-base/src/test/alfred/api.spec.ts
@@ -115,7 +115,11 @@ describe("Routerlicious", () => {
 				scopes,
 			)}`;
 			const defaultProducer = new TestProducer(new TestKafka());
-			const defaultDeltaService = new DeltaService(defaultMongoManager, defaultTenantManager);
+			const deltasCollection = await defaultDbManager.getDeltaCollection(
+				undefined,
+				undefined,
+			);
+			const defaultDeltaService = new DeltaService(deltasCollection, defaultTenantManager);
 			const defaultDocumentRepository = new TestNotImplementedDocumentRepository();
 			const defaultDocumentDeleteService = new DocumentDeleteService();
 			let app: express.Application;

diff --git a/server/routerlicious/packages/routerlicious-base/src/test/riddler/api.spec.ts b/server/routerlicious/packages/routerlicious-base/src/test/riddler/api.spec.ts
@@ -15,10 +15,12 @@ import {
 } from "@fluidframework/server-services-core";
 import * as riddlerApp from "../../riddler/app";
 import Sinon from "sinon";
+import { ITenantDocument } from "../../riddler";
 
 const documentsCollectionName = "testDocuments";
 const deltasCollectionName = "testDeltas";
 const rawDeltasCollectionName = "testRawDeltas";
+const testTenantsCollectionName = "test-tenantAdmins";
 
 class TestSecretManager implements ISecretManager {
 	constructor(private readonly encryptionKey: string) {}
@@ -38,7 +40,7 @@ class TestSecretManager implements ISecretManager {
 
 describe("Routerlicious", () => {
 	describe("Riddler", () => {
-		describe("API", () => {
+		describe("API", async () => {
 			const document = {
 				_id: "doc-id",
 				content: "Hello, World!",
@@ -47,8 +49,12 @@ describe("Routerlicious", () => {
 				[documentsCollectionName]: [document],
 				[deltasCollectionName]: [],
 				[rawDeltasCollectionName]: [],
+				[testTenantsCollectionName]: [],
 			});
 			const defaultMongoManager = new MongoManager(defaultDbFactory);
+			const defaultDb = await defaultMongoManager.getDatabase();
+			const defaultTenantsCollection =
+				defaultDb.collection<ITenantDocument>(testTenantsCollectionName);
 			const testTenantId = "test-tenant-id";
 
 			let app: express.Application;
@@ -73,7 +79,6 @@ describe("Routerlicious", () => {
 				};
 
 				beforeEach(() => {
-					const testCollectionName = "test-tenantAdmins";
 					const testLoggerFormat = "test-logger-format";
 					const testBaseOrdererUrl = "test-base-orderer-url";
 					const testExtHistorianUrl = "test-ext-historian-url";
@@ -86,8 +91,7 @@ describe("Routerlicious", () => {
 					const testRiddlerStorageRequestMetricIntervalMs = 60000;
 
 					app = riddlerApp.create(
-						testCollectionName,
-						defaultMongoManager,
+						defaultTenantsCollection,
 						testLoggerFormat,
 						testBaseOrdererUrl,
 						testExtHistorianUrl,