server: add more Ephemeral Container optimizations (microsoft#19349)

## Description

We need to trim down as many unnecessary Redis API calls as possible in
the Ephemeral Container scenario. This PR adds the following
optimizations:

1. **Stat Optimization** (behind flag): Uses `strlen` instead of `get`
to check if a path exists and if it's a directory or file. Empty files
are assumed to be directories.
2. **Disable Persist Latest Full Summary** (behind flag): Disables
storing the latest full summary as its own blob in RedisFs, because it's
redundant to Historian's Redis cache.
3. **Disable Delete Summary on Deli Close** (behind flag): Disables
Deli's call to Historian/Gitrest to proactively delete an ephemeral
container's summaries on session end because they will be cleaned up by
TTL eventually.
4. **HashMap "rootDir" optimizations**: Don't bother getting/setting
fields that are included in the hashmap's key, because those are always
directories with an empty value.
5. **HashMap "expire" optmization**: Update expire once on hashmap
creation and never again so that we don't keep calling `expire`
everytime a recursive mkdir is called.

server/charts/historian/templates/gitrest-configmap.yaml

@@ -54,6 +54,7 @@ data:
                 "name": "{{ .Values.gitrest.git.ephemeralfilesystem.name }}"
             },
             "persistLatestFullSummary": {{ .Values.gitrest.git.persistLatestFullSummary }},
+            "persistLatestFullEphemeralSummary": {{ .Values.gitrest.git.persistLatestFullEphemeralSummary }},
             "repoPerDocEnabled": {{ .Values.gitrest.git.repoPerDocEnabled }},
             "enableRepositoryManagerMetrics": {{ .Values.gitrest.git.enableRepositoryManagerMetrics }},
             "apiMetricsSamplingPeriod": {{ .Values.gitrest.git.apiMetricsSamplingPeriod }},
@@ -66,6 +67,7 @@ data:
             "enableSlimGitInit": {{ .Values.gitrest.git.enableSlimGitInit }},
             "enableRedisFsMetrics": {{ .Values.gitrest.git.enableRedisFsMetrics }},
             "enableHashmapRedisFs": {{ .Values.gitrest.git.enableHashmapRedisFs }},
+            "enableRedisFsOptimizedStat": {{ .Values.gitrest.git.enableRedisFsOptimizedStat }},
             "redisApiMetricsSamplingPeriod": {{ .Values.gitrest.git.redisApiMetricsSamplingPeriod }}
         },
         "redis": {

server/charts/historian/values.yaml

@@ -62,6 +62,7 @@ gitrest:
     ephemeralfilesystem:
       name: redisFs
     persistLatestFullSummary: false
+    persistLatestFullEphemeralSummary: false
     repoPerDocEnabled: false
     enableRepositoryManagerMetrics: false
     apiMetricsSamplingPeriod: 100
@@ -70,6 +71,7 @@ gitrest:
     enableSlimGitInit: false
     enableRedisFsMetrics: false
     enableHashmapRedisFs: false
+    enableRedisFsOptimizedStat: false
     redisApiMetricsSamplingPeriod: 0
   redis:
     host: redis

server/gitrest/packages/gitrest-base/src/routes/summaries.ts

@@ -50,14 +50,18 @@ async function getSummary(
 	repoManagerParams: IRepoManagerParams,
 	externalWriterConfig?: IExternalWriterConfig,
 	persistLatestFullSummary = false,
+	persistLatestFullEphemeralSummary = false,
 	enforceStrictPersistedFullSummaryReads = false,
 ): Promise<IWholeFlatSummary> {
 	const lumberjackProperties = {
 		...getLumberjackBasePropertiesFromRepoManagerParams(repoManagerParams),
 		[BaseGitRestTelemetryProperties.sha]: sha,
 	};
 
-	if (persistLatestFullSummary && sha === latestSummarySha) {
+	const enablePersistLatestFullSummary = repoManagerParams.isEphemeralContainer
+		? persistLatestFullEphemeralSummary
+		: persistLatestFullSummary;
+	if (enablePersistLatestFullSummary && sha === latestSummarySha) {
 		try {
 			const latestFullSummaryFromStorage = await retrieveLatestFullSummaryFromStorage(
 				fileSystemManager,
@@ -104,7 +108,7 @@ async function getSummary(
 
 	// Now that we computed the summary from scratch, we can persist it to storage if
 	// the following conditions are met.
-	if (persistLatestFullSummary && sha === latestSummarySha && fullSummary) {
+	if (enablePersistLatestFullSummary && sha === latestSummarySha && fullSummary) {
 		// We persist the full summary in a fire-and-forget way because we don't want it
 		// to impact getSummary latency. So upon computing the full summary above, we should
 		// return as soon as possible. Also, we don't care about failures much, since the
@@ -134,6 +138,7 @@ async function createSummary(
 	externalWriterConfig?: IExternalWriterConfig,
 	isInitialSummary?: boolean,
 	persistLatestFullSummary = false,
+	persistLatestFullEphemeralSummary = false,
 	enableLowIoWrite: "initial" | boolean = false,
 	optimizeForInitialSummary: boolean = false,
 ): Promise<IWriteSummaryResponse | IWholeFlatSummary> {
@@ -177,7 +182,10 @@ async function createSummary(
 					return undefined;
 			  });
 		if (latestFullSummary) {
-			if (persistLatestFullSummary) {
+			const enablePersistLatestFullSummary = repoManagerParams.isEphemeralContainer
+				? persistLatestFullEphemeralSummary
+				: persistLatestFullSummary;
+			if (enablePersistLatestFullSummary) {
 				// Send latest full summary to storage for faster read access.
 				const persistP = persistLatestFullSummaryInStorage(
 					fileSystemManager,
@@ -271,6 +279,8 @@ export function create(
 ): Router {
 	const router: Router = Router();
 	const persistLatestFullSummary: boolean = store.get("git:persistLatestFullSummary") ?? false;
+	const persistLatestFullEphemeralSummary: boolean =
+		store.get("git:persistLatestFullEphemeralSummary") ?? false;
 	const enableLowIoWrite: "initial" | boolean = store.get("git:enableLowIoWrite") ?? false;
 	const enableOptimizedInitialSummary: boolean =
 		store.get("git:enableOptimizedInitialSummary") ?? false;
@@ -321,6 +331,7 @@ export function create(
 						repoManagerParams,
 						getExternalWriterParams(request.query?.config as string | undefined),
 						persistLatestFullSummary,
+						persistLatestFullEphemeralSummary,
 						enforceStrictPersistedFullSummaryReads,
 					);
 				})
@@ -401,6 +412,7 @@ export function create(
 					getExternalWriterParams(request.query?.config as string | undefined),
 					isInitialSummary,
 					persistLatestFullSummary,
+					persistLatestFullEphemeralSummary,
 					enableLowIoWrite,
 					optimizeForInitialSummary,
 				);

server/gitrest/packages/gitrest-base/src/test/utils.ts

@@ -48,6 +48,7 @@ export const defaultProvider = new nconf.Provider({}).use("memory").defaults({
 			name: "redisFs",
 		},
 		persistLatestFullSummary: false,
+		persistLatestFullEphemeralSummary: false,
 		repoPerDocEnabled: false,
 		enableRepositoryManagerMetrics: false,
 		apiMetricsSamplingPeriod: 100,

server/gitrest/packages/gitrest-base/src/utils/filesystems.ts

@@ -40,6 +40,7 @@ export class RedisFsManagerFactory implements IFileSystemManagerFactory {
 			enableRedisFsMetrics: (config.get("git:enableRedisFsMetrics") as boolean) ?? true,
 			redisApiMetricsSamplingPeriod:
 				(config.get("git:redisApiMetricsSamplingPeriod") as number) ?? 0,
+			enableOptimizedStat: (config.get("git:enableRedisFsOptimizedStat") as boolean) ?? false,
 		};
 		const redisConfig = config.get("redis");
 		this.redisOptions = {

server/gitrest/packages/gitrest-base/src/utils/redisFs/helpers.ts

@@ -19,6 +19,7 @@ export enum RedisFsApis {
 	Rmdir = "Rmdir",
 	KeysByPrefix = "keysByPrefix",
 	HKeysByPrefix = "hkeysByPrefix",
+	InitHashmapFs = "initHashmapFs",
 }
 
 export enum RedisFSConstants {

server/gitrest/packages/gitrest-base/src/utils/redisFs/redis.ts

@@ -3,6 +3,7 @@
  * Licensed under the MIT License.
  */
 
+import { runWithRetry } from "@fluidframework/server-services-core";
 import { IRedisParameters } from "@fluidframework/server-services-utils";
 import { Lumberjack } from "@fluidframework/server-services-telemetry";
 import * as IoRedis from "ioredis";
@@ -15,14 +16,37 @@ export interface RedisParams extends IRedisParameters {
 }
 
 export interface IRedis {
+	/**
+	 * Get the value stored at the given key.
+	 */
 	get<T>(key: string): Promise<T>;
+	/**
+	 * Store the given value the given key.
+	 */
 	set<T>(key: string, value: T, expireAfterSeconds?: number): Promise<void>;
+	/**
+	 * Store the given key/value pairs.
+	 */
 	setMany<T>(
 		keyValuePairs: { key: string; value: T }[],
 		expireAfterSeconds?: number,
 	): Promise<void>;
+	/**
+	 * Check the existence of the given key.
+	 * @returns the size of the value stored at the given key. -1 if the key does not exist, 0 if the key is empty.
+	 */
+	peek(key: string): Promise<number>;
+	/**
+	 * Delete the given key. Optionally, append a prefix to the key before deleting.
+	 */
 	del(key: string, appendPrefixToKey?: boolean): Promise<boolean>;
+	/**
+	 * Delete all keys with the given prefix.
+	 */
 	delAll(keyPrefix: string): Promise<boolean>;
+	/**
+	 * Retrieve all keys with the given prefix.
+	 */
 	keysByPrefix(keyPrefix: string): Promise<string[]>;
 }
 
@@ -79,6 +103,13 @@ export class Redis implements IRedis {
 		await Promise.all(setPs);
 	}
 
+	public async peek(key: string): Promise<number> {
+		const strlen = await this.client.strlen(this.getKey(key));
+		// If the key does not exist, strlen will return 0.
+		// Otherwise, we are stringifying everything we store in Redis, so strlen will always be at least 2 from the stringified quotes.
+		return strlen === 0 ? -1 : strlen - 2;
+	}
+
 	public async del(key: string, appendPrefixToKey = true): Promise<boolean> {
 		// If 'appendPrefixToKey' is true, we prepend a prefix to the 'key' parameter.
 		// This is useful in scenarios where we want to consistently manage keys with a common prefix,
@@ -125,6 +156,7 @@ export class Redis implements IRedis {
 export class HashMapRedis implements IRedis {
 	private readonly expireAfterSeconds: number = 60 * 60 * 24;
 	private readonly prefix: string = "fs";
+	private initialized: boolean = false;
 
 	constructor(
 		/**
@@ -148,6 +180,10 @@ export class HashMapRedis implements IRedis {
 	}
 
 	public async get<T>(field: string): Promise<T> {
+		if (this.isFieldRootDirectory(field)) {
+			// Field is part of the root hashmap key, so return empty string.
+			return "" as unknown as T;
+		}
 		const stringValue = await this.client.hget(this.getMapKey(), this.getMapField(field));
 		return JSON.parse(stringValue) as T;
 	}
@@ -157,33 +193,48 @@ export class HashMapRedis implements IRedis {
 		value: T,
 		expireAfterSeconds: number = this.expireAfterSeconds,
 	): Promise<void> {
+		await this.initHashmap();
+		if (this.isFieldRootDirectory(field)) {
+			// Field is part of the root hashmap key, so do nothing.
+			return;
+		}
 		// Set values in the hash map and returns the count of set field/value pairs.
 		// However, if it's a duplicate field, it will return 0, so we can't rely on the return value to determine success.
 		await this.client.hset(this.getMapKey(), this.getMapField(field), JSON.stringify(value));
-		this.updateHashMapExpiration([field], expireAfterSeconds);
 	}
 
 	public async setMany<T>(
 		fieldValuePairs: { key: string; value: T }[],
 		expireAfterSeconds: number = this.expireAfterSeconds,
 	): Promise<void> {
+		await this.initHashmap();
+		// Filter out root directory fields, since they are not necessary fields in the HashMap.
+		// Then, map each field/value pair to array of arguments for the HSET command (f1, v1, f2, v2...).
+		const fieldValueArgs = fieldValuePairs
+			.filter(({ key: field, value }) => this.isFieldRootDirectory(field))
+			.flatMap(({ key: field, value }) => [this.getMapField(field), JSON.stringify(value)]);
+		if (fieldValueArgs.length === 0) {
+			// Don't do anything if there are no fields to set.
+			return;
+		}
 		// Set values in the hash map and returns the count of set field/value pairs.
 		// However, if it's a duplicate field, it will return 0, so we can't rely on the return value to determine success.
-		await this.client.hset(
-			this.getMapKey(),
-			...fieldValuePairs.flatMap(({ key: field, value }) => [
-				this.getMapField(field),
-				JSON.stringify(value),
-			]),
-		);
-		this.updateHashMapExpiration(
-			fieldValuePairs.map(({ key: field }) => field),
-			expireAfterSeconds,
-		);
+		await this.client.hset(this.getMapKey(), fieldValueArgs);
+	}
+
+	public async peek(field: string): Promise<number> {
+		if (this.isFieldRootDirectory(field)) {
+			// Field is part of the root hashmap key, so it exists but is empty.
+			return 0;
+		}
+		const strlen = await this.client.hstrlen(this.getMapKey(), this.getMapField(field));
+		// If the key does not exist, strlen will return 0.
+		// Otherwise, we are stringifying everything we store in Redis, so strlen will always be at least 2 from the stringified quotes.
+		return strlen === 0 ? -1 : strlen - 2;
 	}
 
 	public async del(field: string): Promise<boolean> {
-		if (this.hashMapKey.startsWith(field)) {
+		if (this.isFieldRootDirectory(field)) {
 			// The deleted field is a root directory, so we need to delete the whole HashMap.
 			return this.delAll(field);
 		}
@@ -195,7 +246,7 @@ export class HashMapRedis implements IRedis {
 	}
 
 	public async delAll(keyPrefix: string): Promise<boolean> {
-		if (this.hashMapKey.startsWith(keyPrefix)) {
+		if (this.isFieldRootDirectory(keyPrefix)) {
 			// The key prefix matches a root directory, so we need to delete the whole HashMap.
 			const unlinkResult = await this.client.unlink(this.getMapKey());
 			// The UNLINK API in Redis returns the number of keys that were removed.
@@ -239,19 +290,45 @@ export class HashMapRedis implements IRedis {
 	}
 
 	/**
-	 * Asynchronously updates the HashMap key's expiration time when the field matching mapKey is set.
+	 * Initializes the hashmap if it doesn't exist, and sets the expiration on the hashmap key.
+	 * This is a no-op if it has already been called once in this HashMapRedis instance.
 	 */
-	private updateHashMapExpiration(
-		fields: string[],
-		expireAfterSeconds: number = this.expireAfterSeconds,
-	) {
-		if (new Set(fields).has(this.hashMapKey)) {
-			// We need to set the expiration of the HashMap key such that it is deleted after the correct expiration time.
-			// However, we only want to do this _once_, so we set the expiration when the field matching the hashmap key is set.
-			// This means that the expiration will be set when the repository directory is created, then never again.
-			this.client.expire(this.getMapKey(), expireAfterSeconds).catch((error) => {
-				Lumberjack.error("Failed to set initial map key with expiration", undefined, error);
-			});
+	private async initHashmap(): Promise<void> {
+		if (this.initialized) {
+			// only initialize once
+			return;
 		}
+		const initializeHashMapIfNotExists = async (): Promise<void> => {
+			const exists = await this.client.exists(this.getMapKey());
+			if (!exists) {
+				await executeRedisFsApiWithMetric(
+					async () => {
+						// Set a blank field/value pair to initialize the hashmap.
+						await this.client.hset(this.getMapKey(), "", "");
+						await this.client.expire(this.getMapKey(), this.expireAfterSeconds);
+					},
+					RedisFsApis.InitHashmapFs,
+					this.parameters?.enableRedisMetrics,
+					this.parameters?.redisApiMetricsSamplingPeriod,
+				);
+			}
+			this.initialized = true;
+		};
+		// Setting expiration on the hashmap key is vital, so we should retry it on failure
+		runWithRetry(
+			async () => initializeHashMapIfNotExists(),
+			"InitializeRedisFsHashMap",
+			3,
+			1000,
+		).catch((error) => {
+			Lumberjack.error("Failed to initialize hashmap with expiration", undefined, error);
+		});
+	}
+
+	/**
+	 * Checks if the provided field is a root directory field contained in the hashmap key.
+	 */
+	private isFieldRootDirectory(field: string): boolean {
+		return this.hashMapKey.startsWith(field);
 	}
 }

server/gitrest/packages/gitrest-base/src/utils/redisFs/redisFsManager.ts

@@ -35,6 +35,7 @@ import {
 export interface RedisFsConfig {
 	enableRedisFsMetrics: boolean;
 	redisApiMetricsSamplingPeriod: number;
+	enableOptimizedStat: boolean;
 }
 
 export class RedisFsManager implements IFileSystemManager {
@@ -362,8 +363,17 @@ export class RedisFs implements IFileSystemPromises {
 	public async stat(filepath: PathLike, options?: StatOptions): Promise<Stats | BigIntStats>;
 	public async stat(filepath: PathLike, options?: any): Promise<Stats | BigIntStats> {
 		const filepathString = filepath.toString();
-		const data = await executeRedisFsApiWithMetric(
-			async () => this.redisFsClient.get<string | Buffer>(filepathString),
+		const dataLength = await executeRedisFsApiWithMetric(
+			async () => {
+				if (this.redisFsConfig.enableOptimizedStat) {
+					return this.redisFsClient.peek(filepathString);
+				}
+				const data = await this.redisFsClient.get<string | Buffer>(filepathString);
+				if (data === null) {
+					return -1;
+				}
+				return data.length;
+			},
 			RedisFsApis.Stat,
 			this.redisFsConfig.enableRedisFsMetrics,
 			this.redisFsConfig.redisApiMetricsSamplingPeriod,
@@ -373,11 +383,11 @@ export class RedisFs implements IFileSystemPromises {
 			true,
 		);
 
-		if (data === null) {
+		if (dataLength === -1) {
 			throw new RedisFsError(SystemErrors.ENOENT, filepath.toString());
 		}
 
-		const fsEntityType = data === "" ? RedisFSConstants.directory : RedisFSConstants.file;
+		const fsEntityType = dataLength === 0 ? RedisFSConstants.directory : RedisFSConstants.file;
 
 		return getStats(fsEntityType);
 	}

server/gitrest/packages/gitrest/config.json

@@ -44,6 +44,7 @@
 			"name": "redisFs"
 		},
 		"persistLatestFullSummary": false,
+		"persistLatestFullEphemeralSummary": false,
 		"repoPerDocEnabled": false,
 		"enableRepositoryManagerMetrics": false,
 		"apiMetricsSamplingPeriod": 100,
@@ -52,6 +53,7 @@
 		"enableSlimGitInit": false,
 		"enableRedisFsMetrics": true,
 		"enableHashmapRedisFs": false,
+		"enableRedisFsOptimizedStat": false,
 		"redisApiMetricsSamplingPeriod": 0,
 		"enforceStrictPersistedFullSummaryReads": false
 	},