Просмотр исходного кода

implement caching for Azure credentials and clients to optimize performance

Yuki Takei 6 месяцев назад
Родитель
Сommit
40317e4d4a

+ 0 - 81
.serena/memories/azure-upload-memory-leak-analysis-report.md

@@ -1,81 +0,0 @@
-# Azureアップロード機能 メモリリーク分析レポート
-
-## 概要
-`/workspace/growi/apps/app/src/server/service/file-uploader/azure.ts` ファイルにおけるメモリリークの可能性を詳細分析した結果です。
-
-## 🔴 高リスク:メモリリークの可能性が高い箇所
-
-### 1. Azure クライアントの繰り返し作成
-**場所**: `getContainerClient()` 関数(行 74-78)  
-**問題コード**:
-```typescript
-async function getContainerClient(): Promise<ContainerClient> {
-  const { accountName, containerName } = getAzureConfig();
-  const blobServiceClient = new BlobServiceClient(`https://${accountName}.blob.core.windows.net`, getCredential());
-  return blobServiceClient.getContainerClient(containerName);
-}
-```
-
-**問題点**:
-- 毎回新しい`BlobServiceClient`インスタンスを作成
-- 内部で保持されるHTTP接続プール、認証トークン、タイマーが蓄積
-- `ClientSecretCredential`が毎回作成され、内部のHTTPクライアントが解放されない
-- 長時間稼働時にAzure接続リソースが指数的に増加
-- OAuth トークンキャッシュの重複管理
-
-**影響度**: 高 - 連続アップロード/ダウンロードで深刻な影響
-
-### 2. generateTemporaryUrl での重複クライアント作成
-**場所**: `generateTemporaryUrl`メソッド(行 188-237)  
-**問題コード**:
-```typescript
-const sasToken = await (async() => {
-  const { accountName, containerName } = getAzureConfig();
-  const blobServiceClient = new BlobServiceClient(`https://${accountName}.blob.core.windows.net`, getCredential());
-  
-  const userDelegationKey = await blobServiceClient.getUserDelegationKey(startsOn, expiresOn);
-  // ...
-})();
-```
-
-**問題点**:
-- URLの構築とSASトークン生成で別々に`BlobServiceClient`を作成
-- 同一メソッド内で複数のクライアントインスタンスが同時存在
-- ユーザーデリゲーションキーの取得で長時間接続を保持
-- 認証処理の重複実行でCPUとメモリの無駄使用
-- SASトークン生成時の一時的な大量メモリ消費
-
-**影響度**: 高 - URL生成処理の度に重複リソース消費
-
-
-### 3. 認証クレデンシャルの繰り返し作成
-**場所**: `getCredential()` 関数(行 62-72)  
-**問題コード**:
-```typescript
-function getCredential(): TokenCredential {
-  const tenantId = toNonBlankStringOrUndefined(configManager.getConfig('azure:tenantId'));
-  const clientId = toNonBlankStringOrUndefined(configManager.getConfig('azure:clientId'));
-  const clientSecret = toNonBlankStringOrUndefined(configManager.getConfig('azure:clientSecret'));
-
-  return new ClientSecretCredential(tenantId, clientId, clientSecret);
-}
-```
-
-**問題点**:
-- 毎回新しい`ClientSecretCredential`インスタンスを作成
-- 内部のHTTPクライアント、トークンキャッシュが重複作成
-- OAuthトークンの取得処理が重複実行
-- 認証状態の管理が非効率
-
-**対策**:
-- singleton インスタンスを作成
-- configManager.getConfig で取得する値に更新があればインスタンスを再作成
-
-**影響度**: 中 - 認証処理の頻度に依存
-
-
----
-**作成日**: 2025年9月12日  
-**対象ファイル**: `/workspace/growi/apps/app/src/server/service/file-uploader/azure.ts`  
-**分析者**: GitHub Copilot  
-**重要度**: 高(Azureファイルアップロード機能の安定性に直結)

+ 50 - 4
apps/app/src/server/service/file-uploader/azure.ts

@@ -45,6 +45,11 @@ type AzureConfig = {
   containerName: string,
   containerName: string,
 }
 }
 
 
+// Cache holders to avoid repeated instantiation of credential and clients
+let cachedCredential: { key: string, credential: TokenCredential } | null = null;
+let cachedBlobServiceClient: { key: string, client: BlobServiceClient } | null = null;
+let cachedContainerClient: { key: string, client: ContainerClient } | null = null;
+
 
 
 function getAzureConfig(): AzureConfig {
 function getAzureConfig(): AzureConfig {
   const accountName = configManager.getConfig('azure:storageAccountName');
   const accountName = configManager.getConfig('azure:storageAccountName');
@@ -61,6 +66,7 @@ function getAzureConfig(): AzureConfig {
 }
 }
 
 
 function getCredential(): TokenCredential {
 function getCredential(): TokenCredential {
+  // Build cache key from credential-related configs
   const tenantId = toNonBlankStringOrUndefined(configManager.getConfig('azure:tenantId'));
   const tenantId = toNonBlankStringOrUndefined(configManager.getConfig('azure:tenantId'));
   const clientId = toNonBlankStringOrUndefined(configManager.getConfig('azure:clientId'));
   const clientId = toNonBlankStringOrUndefined(configManager.getConfig('azure:clientId'));
   const clientSecret = toNonBlankStringOrUndefined(configManager.getConfig('azure:clientSecret'));
   const clientSecret = toNonBlankStringOrUndefined(configManager.getConfig('azure:clientSecret'));
@@ -69,13 +75,52 @@ function getCredential(): TokenCredential {
     throw new Error(`Azure Blob Storage missing required configuration: tenantId=${tenantId}, clientId=${clientId}, clientSecret=${clientSecret}`);
     throw new Error(`Azure Blob Storage missing required configuration: tenantId=${tenantId}, clientId=${clientId}, clientSecret=${clientSecret}`);
   }
   }
 
 
-  return new ClientSecretCredential(tenantId, clientId, clientSecret);
+  const key = `${tenantId}|${clientId}|${clientSecret}`;
+
+  // Reuse cached credential when config has not changed
+  if (cachedCredential != null && cachedCredential.key === key) {
+    return cachedCredential.credential;
+  }
+
+  const credential = new ClientSecretCredential(tenantId, clientId, clientSecret);
+  cachedCredential = { key, credential };
+  return credential;
+}
+
+function getBlobServiceClient(): BlobServiceClient {
+  const { accountName } = getAzureConfig();
+  // Include credential cache key to ensure we re-create if cred changed
+  const credential = getCredential();
+  const credentialKey = (cachedCredential?.key) ?? 'unknown-cred';
+  const key = `${accountName}|${credentialKey}`;
+
+  if (cachedBlobServiceClient != null && cachedBlobServiceClient.key === key) {
+    return cachedBlobServiceClient.client;
+  }
+
+  // Use keep-alive to minimize socket churn; reuse client across calls
+  const client = new BlobServiceClient(
+    `https://${accountName}.blob.core.windows.net`,
+    credential,
+    { keepAliveOptions: { enable: true } },
+  );
+  cachedBlobServiceClient = { key, client };
+  return client;
 }
 }
 
 
 async function getContainerClient(): Promise<ContainerClient> {
 async function getContainerClient(): Promise<ContainerClient> {
   const { accountName, containerName } = getAzureConfig();
   const { accountName, containerName } = getAzureConfig();
-  const blobServiceClient = new BlobServiceClient(`https://${accountName}.blob.core.windows.net`, getCredential());
-  return blobServiceClient.getContainerClient(containerName);
+  const credentialKey = (cachedCredential?.key) ?? 'unknown-cred';
+  const key = `${accountName}|${containerName}|${credentialKey}`;
+
+  if (cachedContainerClient != null && cachedContainerClient.key === key) {
+    return cachedContainerClient.client;
+  }
+
+  const blobServiceClient = getBlobServiceClient();
+  const client = blobServiceClient.getContainerClient(containerName);
+  cachedContainerClient = { key, client };
+  return client;
 }
 }
 
 
 function getFilePathOnStorage(attachment: IAttachmentDocument) {
 function getFilePathOnStorage(attachment: IAttachmentDocument) {
@@ -221,7 +266,8 @@ class AzureFileUploader extends AbstractFileUploader {
 
 
     const sasToken = await (async() => {
     const sasToken = await (async() => {
       const { accountName, containerName } = getAzureConfig();
       const { accountName, containerName } = getAzureConfig();
-      const blobServiceClient = new BlobServiceClient(`https://${accountName}.blob.core.windows.net`, getCredential());
+      // Reuse the same BlobServiceClient (singleton)
+      const blobServiceClient = getBlobServiceClient();
 
 
       const now = Date.now();
       const now = Date.now();
       const startsOn = new Date(now - 30 * 1000);
       const startsOn = new Date(now - 30 * 1000);