From 04c8c2ffa0a58bf34c352ee4e9a4824da18dcf80 Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Fri, 8 Nov 2024 17:36:15 +0700 Subject: [PATCH] UBERF-8578: Fix extra stat call for storage adapter (#7132) Signed-off-by: Andrey Sobolev --- server/collaboration/src/utils/storage.ts | 23 ++++---- server/core/src/types.ts | 13 +++- server/front/src/index.ts | 2 +- server/s3/src/index.ts | 32 ++++++---- server/server-storage/src/fallback.ts | 72 +++++++++++------------ 5 files changed, 79 insertions(+), 63 deletions(-) diff --git a/server/collaboration/src/utils/storage.ts b/server/collaboration/src/utils/storage.ts index 76389d128a..be2cb7976d 100644 --- a/server/collaboration/src/utils/storage.ts +++ b/server/collaboration/src/utils/storage.ts @@ -28,17 +28,20 @@ export async function yDocFromStorage ( ydoc?: YDoc ): Promise { // stat the object to ensure it exists, because read will throw an error in this case - const blob = await storageAdapter.stat(ctx, workspace, documentId) - if (blob === undefined) { - return undefined + try { + const buffer = await storageAdapter.read(ctx, workspace, documentId) + + // no need to apply gc because we load existing document + // it is either already gc-ed, or gc not needed and it is disabled + ydoc ??= new YDoc({ guid: generateId(), gc: false }) + + return yDocFromBuffer(Buffer.concat(buffer as any), ydoc) + } catch (err: any) { + if (err.code === 'NoSuchKey') { + return undefined + } + throw err } - - // no need to apply gc because we load existing document - // it is either already gc-ed, or gc not needed and it is disabled - ydoc ??= new YDoc({ guid: generateId(), gc: false }) - - const buffer = await storageAdapter.read(ctx, workspace, documentId) - return yDocFromBuffer(Buffer.concat(buffer as any), ydoc) } /** @public */ diff --git a/server/core/src/types.ts b/server/core/src/types.ts index e570de7e97..bf80c0fbdf 100644 --- a/server/core/src/types.ts +++ b/server/core/src/types.ts @@ -48,8 +48,8 @@ import type { Request, Response } from '@hcengineering/rpc' import type { Token } from '@hcengineering/server-token' import { type Readable } from 'stream' import type { DbAdapter, DomainHelper } from './adapter' -import { type StorageAdapter } from './storage' import type { StatisticsElement } from './stats' +import { type StorageAdapter } from './storage' export interface ServerFindOptions extends FindOptions { domain?: Domain // Allow to find for Doc's in specified domain only. @@ -519,6 +519,17 @@ export interface StorageConfig { port?: number } +export class NoSuchKeyError extends Error { + code: string + constructor ( + msg: string, + readonly cause?: any + ) { + super(msg) + this.code = 'NoSuchKey' + } +} + export interface StorageConfiguration { default: string storages: StorageConfig[] diff --git a/server/front/src/index.ts b/server/front/src/index.ts index 493c620dbe..13895a1340 100644 --- a/server/front/src/index.ts +++ b/server/front/src/index.ts @@ -401,7 +401,7 @@ export function start ( } let blobInfo = await ctx.with( - 'notoken-stat', + 'stat', { workspace: payload.workspace.name }, async (ctx) => await config.storageAdapter.stat(ctx, payload.workspace, uuid) ) diff --git a/server/s3/src/index.ts b/server/s3/src/index.ts index b98545dbbe..c76680c5f4 100644 --- a/server/s3/src/index.ts +++ b/server/s3/src/index.ts @@ -29,6 +29,7 @@ import core, { } from '@hcengineering/core' import { getMetadata } from '@hcengineering/platform' import serverCore, { + NoSuchKeyError, type BlobStorageIterator, type ListBlobResult, type StorageAdapter, @@ -316,21 +317,26 @@ export class S3Service implements StorageAdapter { } async doGet (ctx: MeasureContext, workspaceId: WorkspaceId, objectName: string, range?: string): Promise { - const res = await this.client.getObject({ - Bucket: this.getBucketId(workspaceId), - Key: this.getDocumentKey(workspaceId, objectName), - Range: range - }) + try { + const res = await this.client.getObject({ + Bucket: this.getBucketId(workspaceId), + Key: this.getDocumentKey(workspaceId, objectName), + Range: range + }) - const stream = res.Body?.transformToWebStream() + const stream = res.Body?.transformToWebStream() - if (stream !== undefined) { - return Readable.fromWeb(stream as ReadableStream) - } else { - const readable = new Readable() - readable._read = () => {} - readable.push(null) - return readable + if (stream !== undefined) { + return Readable.fromWeb(stream as ReadableStream) + } else { + const readable = new Readable() + readable._read = () => {} + readable.push(null) + return readable + } + } catch (err: any) { + // In case of error return undefined + throw new NoSuchKeyError(`${workspaceId.name} missing ${objectName}`, err) } } diff --git a/server/server-storage/src/fallback.ts b/server/server-storage/src/fallback.ts index 5ba873e60a..264dcad578 100644 --- a/server/server-storage/src/fallback.ts +++ b/server/server-storage/src/fallback.ts @@ -144,7 +144,7 @@ export class FallbackStorageAdapter implements StorageAdapter, StorageAdapterEx return result } - @withContext('aggregator-delete', {}) + @withContext('fallback-delete', {}) async delete (ctx: MeasureContext, workspaceId: WorkspaceId): Promise { for (const { adapter } of this.adapters) { if (await adapter.exists(ctx, workspaceId)) { @@ -153,7 +153,7 @@ export class FallbackStorageAdapter implements StorageAdapter, StorageAdapterEx } } - @withContext('aggregator-remove', {}) + @withContext('fallback-remove', {}) async remove (ctx: MeasureContext, workspaceId: WorkspaceId, objectNames: string[]): Promise { // Group by provider and delegate into it. for (const { adapter } of this.adapters) { @@ -173,40 +173,30 @@ export class FallbackStorageAdapter implements StorageAdapter, StorageAdapterEx } } - @withContext('aggregator-stat', {}) - async stat (ctx: MeasureContext, workspaceId: WorkspaceId, name: string): Promise { - const result = await this.findProvider(ctx, workspaceId, name) - if (result !== undefined) { - result.stat.provider = result.name - } - return result?.stat - } - - @withContext('aggregator-get', {}) - async get (ctx: MeasureContext, workspaceId: WorkspaceId, name: string): Promise { - const result = await this.findProvider(ctx, workspaceId, name) - if (result === undefined) { - throw new NoSuchKeyError(`${workspaceId.name} missing ${name}`) - } - return await result.adapter.get(ctx, workspaceId, result.stat._id) - } - - @withContext('find-provider', {}) - private async findProvider ( - ctx: MeasureContext, - workspaceId: WorkspaceId, - objectName: string - ): Promise<{ name: string, adapter: StorageAdapter, stat: Blob } | undefined> { - // Group by provider and delegate into it. + @withContext('fallback-stat', {}) + async stat (ctx: MeasureContext, workspaceId: WorkspaceId, objectName: string): Promise { for (const { name, adapter } of this.adapters) { const stat = await adapter.stat(ctx, workspaceId, objectName) if (stat !== undefined) { - return { name, adapter, stat } + stat.provider = name + return stat } } } - @withContext('aggregator-partial', {}) + @withContext('fallback-get', {}) + async get (ctx: MeasureContext, workspaceId: WorkspaceId, objectName: string): Promise { + for (const { adapter } of this.adapters) { + try { + return await adapter.get(ctx, workspaceId, objectName) + } catch (err: any) { + // ignore + } + } + throw new NoSuchKeyError(`${workspaceId.name} missing ${objectName}`) + } + + @withContext('fallback-partial', {}) async partial ( ctx: MeasureContext, workspaceId: WorkspaceId, @@ -214,20 +204,26 @@ export class FallbackStorageAdapter implements StorageAdapter, StorageAdapterEx offset: number, length?: number | undefined ): Promise { - const result = await this.findProvider(ctx, workspaceId, objectName) - if (result === undefined) { - throw new NoSuchKeyError(`${workspaceId.name} missing ${objectName}`) + for (const { adapter } of this.adapters) { + try { + return await adapter.partial(ctx, workspaceId, objectName, offset, length) + } catch (err: any) { + // ignore + } } - return await result.adapter.partial(ctx, workspaceId, result.stat._id, offset, length) + throw new NoSuchKeyError(`${workspaceId.name} missing ${objectName}`) } - @withContext('aggregator-read', {}) + @withContext('fallback-read', {}) async read (ctx: MeasureContext, workspaceId: WorkspaceId, objectName: string): Promise { - const result = await this.findProvider(ctx, workspaceId, objectName) - if (result === undefined) { - throw new NoSuchKeyError(`${workspaceId.name} missing ${objectName}`) + for (const { adapter } of this.adapters) { + try { + return await adapter.read(ctx, workspaceId, objectName) + } catch (err: any) { + // Ignore + } } - return await result.adapter.read(ctx, workspaceId, result.stat._id) + throw new NoSuchKeyError(`${workspaceId.name} missing ${objectName}`) } @withContext('aggregator-put', {})