From ce4070412faa9e08b7e93f3c1f3f9091cea6ba3a Mon Sep 17 00:00:00 2001 From: Alexander Onnikov Date: Thu, 13 Jun 2024 23:27:35 +0700 Subject: [PATCH] UBERF-7260 Handle storage errors in collaborator (#5806) Signed-off-by: Alexander Onnikov --- server/collaboration/src/index.ts | 2 +- .../src/utils/collaborative-doc.ts | 2 +- .../src/utils/{minio.ts => storage.ts} | 17 ++- server/collaborator/src/extensions/storage.ts | 8 +- server/collaborator/src/storage/platform.ts | 129 +++++++++++------- 5 files changed, 90 insertions(+), 68 deletions(-) rename server/collaboration/src/utils/{minio.ts => storage.ts} (81%) diff --git a/server/collaboration/src/index.ts b/server/collaboration/src/index.ts index 5999004357..223512db04 100644 --- a/server/collaboration/src/index.ts +++ b/server/collaboration/src/index.ts @@ -17,5 +17,5 @@ export * from './history/branch' export * from './history/history' export * from './history/snapshot' export * from './utils/collaborative-doc' -export * from './utils/minio' +export * from './utils/storage' export * from './utils/ydoc' diff --git a/server/collaboration/src/utils/collaborative-doc.ts b/server/collaboration/src/utils/collaborative-doc.ts index debe69b8f8..16f57bb669 100644 --- a/server/collaboration/src/utils/collaborative-doc.ts +++ b/server/collaboration/src/utils/collaborative-doc.ts @@ -28,7 +28,7 @@ import { StorageAdapter } from '@hcengineering/server-core' import { yDocBranch } from '../history/branch' import { YDocVersion } from '../history/history' import { createYdocSnapshot, restoreYdocSnapshot } from '../history/snapshot' -import { yDocFromStorage, yDocToStorage } from './minio' +import { yDocFromStorage, yDocToStorage } from './storage' /** @public */ export function collaborativeHistoryDocId (id: string): string { diff --git a/server/collaboration/src/utils/minio.ts b/server/collaboration/src/utils/storage.ts similarity index 81% rename from server/collaboration/src/utils/minio.ts rename to server/collaboration/src/utils/storage.ts index 7b1eec5630..f68610cf99 100644 --- a/server/collaboration/src/utils/minio.ts +++ b/server/collaboration/src/utils/storage.ts @@ -27,19 +27,18 @@ export async function yDocFromStorage ( minioDocumentId: string, 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, minioDocumentId) + if (blob === undefined) { + return undefined + } + // 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({ gc: false }) - try { - const buffer = await storageAdapter.read(ctx, workspace, minioDocumentId) - return yDocFromBuffer(Buffer.concat(buffer), ydoc) - } catch (err: any) { - if (err?.code === 'NoSuchKey' || err?.code === 'NotFound') { - return undefined - } - throw err - } + const buffer = await storageAdapter.read(ctx, workspace, minioDocumentId) + return yDocFromBuffer(Buffer.concat(buffer), ydoc) } /** @public */ diff --git a/server/collaborator/src/extensions/storage.ts b/server/collaborator/src/extensions/storage.ts index b3f845a5d0..40680c7243 100644 --- a/server/collaborator/src/extensions/storage.ts +++ b/server/collaborator/src/extensions/storage.ts @@ -107,8 +107,8 @@ export class StorageExtension implements Extension { return await adapter.loadDocument(ctx, documentId, context) }) } catch (err) { - ctx.error('failed to load document content', { documentId, error: err }) - return undefined + ctx.error('failed to load document', { documentId, error: err }) + throw new Error('Failed to load document') } } @@ -120,8 +120,8 @@ export class StorageExtension implements Extension { await adapter.saveDocument(ctx, documentId, document, context) }) } catch (err) { - ctx.error('failed to save document content', { documentId, error: err }) - return undefined + ctx.error('failed to save document', { documentId, error: err }) + throw new Error('Failed to save document') } } } diff --git a/server/collaborator/src/storage/platform.ts b/server/collaborator/src/storage/platform.ts index 54f0474900..e5ba7bae1f 100644 --- a/server/collaborator/src/storage/platform.ts +++ b/server/collaborator/src/storage/platform.ts @@ -52,64 +52,63 @@ export class PlatformStorageAdapter implements CollabStorageAdapter { ) {} async loadDocument (ctx: MeasureContext, documentId: DocumentId, context: Context): Promise { + // try to load document content try { - // try to load document content + ctx.info('load document content', { documentId }) + const ydoc = await this.loadDocumentFromStorage(ctx, documentId, context) + + if (ydoc !== undefined) { + return ydoc + } + } catch (err) { + ctx.error('failed to load document content', { documentId, error: err }) + throw err + } + + // then try to load from inital content + const { initialContentId } = context + if (initialContentId !== undefined && initialContentId.length > 0) { try { - ctx.info('load document content', { documentId }) - const ydoc = await this.loadDocumentFromStorage(ctx, documentId, context) - - if (ydoc !== undefined) { - return ydoc - } - } catch (err) { - ctx.error('failed to load document content', { documentId, error: err }) - } - - // then try to load from inital content - const { initialContentId } = context - if (initialContentId !== undefined && initialContentId.length > 0) { - try { - ctx.info('load document initial content', { documentId, initialContentId }) - const ydoc = await this.loadDocumentFromStorage(ctx, initialContentId, context) - - // if document was loaded from the initial content or storage we need to save - // it to ensure the next time we load it from the ydoc document - if (ydoc !== undefined) { - ctx.info('save document content', { documentId, initialContentId }) - await this.saveDocumentToStorage(ctx, documentId, ydoc, context) - return ydoc - } - } catch (err) { - ctx.error('failed to load initial document content', { documentId, initialContentId, error: err }) - } - } - - // finally try to load from the platform - const { platformDocumentId } = context - if (platformDocumentId !== undefined) { - ctx.info('load document platform content', { documentId, platformDocumentId }) - const ydoc = await ctx.with('load-from-platform', {}, async (ctx) => { - try { - return await this.loadDocumentFromPlatform(ctx, platformDocumentId, context) - } catch (err) { - ctx.error('failed to load platform document', { documentId, platformDocumentId, error: err }) - } - }) + ctx.info('load document initial content', { documentId, initialContentId }) + const ydoc = await this.loadDocumentFromStorage(ctx, initialContentId, context) // if document was loaded from the initial content or storage we need to save // it to ensure the next time we load it from the ydoc document if (ydoc !== undefined) { - ctx.info('save document content', { documentId, platformDocumentId }) + ctx.info('save document content', { documentId, initialContentId }) await this.saveDocumentToStorage(ctx, documentId, ydoc, context) return ydoc } + } catch (err) { + ctx.error('failed to load initial document content', { documentId, initialContentId, error: err }) + throw err } - - // nothing found - return undefined - } catch (err) { - ctx.error('failed to load document', { documentId, error: err }) } + + // finally try to load from the platform + const { platformDocumentId } = context + if (platformDocumentId !== undefined) { + ctx.info('load document platform content', { documentId, platformDocumentId }) + const ydoc = await ctx.with('load-from-platform', {}, async (ctx) => { + try { + return await this.loadDocumentFromPlatform(ctx, platformDocumentId, context) + } catch (err) { + ctx.error('failed to load platform document', { documentId, platformDocumentId, error: err }) + throw err + } + }) + + // if document was loaded from the initial content or storage we need to save + // it to ensure the next time we load it from the ydoc document + if (ydoc !== undefined) { + ctx.info('save document content', { documentId, platformDocumentId }) + await this.saveDocumentToStorage(ctx, documentId, ydoc, context) + return ydoc + } + } + + // nothing found + return undefined } async saveDocument (ctx: MeasureContext, documentId: DocumentId, document: YDoc, context: Context): Promise { @@ -133,6 +132,9 @@ export class PlatformStorageAdapter implements CollabStorageAdapter { await this.saveDocumentToStorage(ctx, documentId, document, context) } catch (err) { ctx.error('failed to save document', { documentId, error: err }) + // raise an error if failed to save document to storage + // this will prevent document from being unloaded from memory + throw err } const { platformDocumentId } = context @@ -166,12 +168,9 @@ export class PlatformStorageAdapter implements CollabStorageAdapter { const adapter = this.getStorageAdapter(storage) return await ctx.with('load-document', { storage }, async (ctx) => { - try { + return await withRetry(ctx, 5, async () => { return await loadCollaborativeDoc(adapter, context.workspaceId, collaborativeDoc, ctx) - } catch (err) { - ctx.error('failed to load storage document', { documentId, collaborativeDoc, error: err }) - return undefined - } + }) }) } @@ -185,7 +184,9 @@ export class PlatformStorageAdapter implements CollabStorageAdapter { const adapter = this.getStorageAdapter(storage) await ctx.with('save-document', {}, async (ctx) => { - await saveCollaborativeDoc(adapter, context.workspaceId, collaborativeDoc, document, ctx) + await withRetry(ctx, 5, async () => { + await saveCollaborativeDoc(adapter, context.workspaceId, collaborativeDoc, document, ctx) + }) }) } @@ -291,3 +292,25 @@ export class PlatformStorageAdapter implements CollabStorageAdapter { } } } + +async function withRetry ( + ctx: MeasureContext, + retries: number, + op: () => Promise, + delay: number = 100 +): Promise { + let error: any + while (retries > 0) { + retries-- + try { + return await op() + } catch (err: any) { + error = err + ctx.error('error', err) + if (retries !== 0) { + await new Promise((resolve) => setTimeout(resolve, delay)) + } + } + } + throw error +}