fix(nbstore): fix download blob return too early (#10875)

This commit is contained in:
EYHN
2025-03-14 16:41:31 +00:00
parent 0d7f032238
commit 5915e6a6f1
2 changed files with 47 additions and 11 deletions

View File

@@ -102,16 +102,42 @@ export class BlobSyncImpl implements BlobSync {
readonly blobSync: BlobSyncStorage readonly blobSync: BlobSyncStorage
) {} ) {}
downloadBlob(blobId: string) { downloadBlob(blobId: string): Promise<void> {
const signal = this.abortController.signal; const signal = this.abortController.signal;
return Promise.race(
this.peers.map(p => p.downloadBlob(blobId, signal)) return new Promise<void>((resolve, reject) => {
).catch(err => { let completed = 0;
if (err === MANUALLY_STOP) { const totalPeers = this.peers.length;
if (totalPeers === 0) {
resolve();
return; return;
} }
// should never reach here, `downloadBlob()` should never throw
console.error(err); // download from all peers concurrently
// resolve if any peer has success
this.peers.forEach(peer => {
peer
.downloadBlob(blobId, signal)
.then(result => {
if (result === true) {
// resolve if the peer has success
resolve();
}
})
.catch(err => {
// should never throw
// unless the signal is aborted
reject(err);
})
.finally(() => {
completed++;
if (completed === totalPeers) {
// resolve if all peers finish
resolve();
}
});
});
}); });
} }

View File

@@ -38,9 +38,15 @@ export class BlobSyncPeer {
readonly blobSync: BlobSyncStorage readonly blobSync: BlobSyncStorage
) {} ) {}
private readonly downloadingPromise = new Map<string, Promise<void>>(); private readonly downloadingPromise = new Map<string, Promise<boolean>>();
downloadBlob(blobId: string, signal?: AbortSignal): Promise<void> { /**
* Downloads a blob from the peer with retry logic
* @returns true if the blob is downloaded successfully, false if the blob is not found or encounters an error
*
* @throws This method will never throw (errors are saved to the sync status) unless the signal is aborted
*/
downloadBlob(blobId: string, signal?: AbortSignal): Promise<boolean> {
// if the blob is already downloading, return the existing promise // if the blob is already downloading, return the existing promise
const existing = this.downloadingPromise.get(blobId); const existing = this.downloadingPromise.get(blobId);
if (existing) { if (existing) {
@@ -53,7 +59,7 @@ export class BlobSyncPeer {
count: this.remote.isReadonly ? 1 : 5, // readonly remote storage will not retry count: this.remote.isReadonly ? 1 : 5, // readonly remote storage will not retry
}; };
const promise = new Promise<void>((resolve, reject) => { const promise = new Promise<boolean>((resolve, reject) => {
// mark the blob as downloading // mark the blob as downloading
this.status.blobDownloading(blobId); this.status.blobDownloading(blobId);
@@ -72,6 +78,7 @@ export class BlobSyncPeer {
new Date() new Date()
); );
await this.local.set(data, signal); await this.local.set(data, signal);
resolve(true);
} else { } else {
// if the blob is not found, maybe the uploader have't uploaded the blob yet, we will retry several times // if the blob is not found, maybe the uploader have't uploaded the blob yet, we will retry several times
attempts++; attempts++;
@@ -82,9 +89,11 @@ export class BlobSyncPeer {
); );
// eslint-disable-next-line @typescript-eslint/no-misused-promises // eslint-disable-next-line @typescript-eslint/no-misused-promises
setTimeout(attempt, waitTime); setTimeout(attempt, waitTime);
} else {
// reach the max retry times, resolve the promise with false
resolve(false);
} }
} }
resolve();
} catch (error) { } catch (error) {
// if we encounter any error, reject without retry // if we encounter any error, reject without retry
reject(error); reject(error);
@@ -102,6 +111,7 @@ export class BlobSyncPeer {
blobId, blobId,
error instanceof Error ? error.message : String(error) error instanceof Error ? error.message : String(error)
); );
return false;
}) })
.finally(() => { .finally(() => {
this.status.blobDownloadFinish(blobId); this.status.blobDownloadFinish(blobId);