Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) serialize document uploads and DocSnapshots.versions() to reduce surprises

Browse Source 
Summary:
Occasionally, while the versions of a document are being enumerated,
a new version of the document will be created. This is detected and
triggers re-enumeration and a "surprise" log message. This diff
tweaks uploads to be run in series with DocSnapshots operations.
This means that listing versions would be blocked on an upload, or
vice versa, rather than overlapping. This is simpler and more deterministic.
I'm not sure how the user experience will feel if the operations
are slow.

Test Plan: existing tests pass; will see if surprises are reduced

Reviewers: alexmojaki

Reviewed By: alexmojaki

Subscribers: alexmojaki

Differential Revision: https://phab.getgrist.com/D3551
This commit is contained in:
Paul Fitzpatrick 2022-07-28 13:50:29 -04:00
parent d12b05abcb
commit b6890bed4b
2 changed files with 44 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
app/server/lib/DocSnapshots.ts
View File

@ -148,7 +148,29 @@ export class DocSnapshotInventory implements IInventory {
* The snapshot supplied will be modified in place to a normalized form.
*/
public async add(key: string, snapshot: ObjSnapshotWithMetadata, prevSnapshotId: string|null) {
await this.uploadAndAdd(key, async () => {
return { snapshot, prevSnapshotId };
});
}
/**
* Like add(), but takes an "upload" callback that allows
* preparing snapshot and prevSnapshotId atomically with the
* rest of the add operation, and thus serialized with any
* other operations such as versions(). This is important since an
* upload changes the list of versions as far as the external store
* is concerned, which could trigger a "surprise" and a full reload
* of the version list.
*/
public async uploadAndAdd(key: string,
upload: () => Promise<{snapshot?: ObjSnapshotWithMetadata,
prevSnapshotId: string|null}>) {
await this._mutex.runExclusive(key, async() => {
const {snapshot, prevSnapshotId} = await upload();
if (!snapshot) {
// the upload generated no snapshot, so there is nothing to do.
return;
}
const snapshots = await this._getSnapshots(key, prevSnapshotId);
// Could be already added if reconstruction happened.
if (snapshots[0] && snapshots[0].snapshotId === snapshot.snapshotId) { return; }

38
app/server/lib/HostedStorageManager.ts
View File

@ -739,23 +739,29 @@ export class HostedStorageManager implements IDocStorageManager {
...label && {label},
t,
};
const prevSnapshotId = this._latestVersions.get(docId) || null;
const newSnapshotId = await this._ext.upload(docId, tmpPath, metadata);
if (newSnapshotId === Unchanged) {
// Nothing uploaded because nothing changed
return;
let changeMade: boolean = false;
await this._inventory.uploadAndAdd(docId, async () => {
const prevSnapshotId = this._latestVersions.get(docId) || null;
const newSnapshotId = await this._ext.upload(docId, tmpPath as string, metadata);
if (newSnapshotId === Unchanged) {
// Nothing uploaded because nothing changed
return { prevSnapshotId };
}
if (!newSnapshotId) {
// This is unexpected.
throw new Error('No snapshotId allocated after upload');
}
const snapshot = {
lastModified: t,
snapshotId: newSnapshotId,
metadata
};
changeMade = true;
return { snapshot, prevSnapshotId };
});
if (changeMade) {
await this._onInventoryChange(docId);
}
if (!newSnapshotId) {
// This is unexpected.
throw new Error('No snapshotId allocated after upload');
}
const snapshot = {
lastModified: t,
snapshotId: newSnapshotId,
metadata
};
await this._inventory.add(docId, snapshot, prevSnapshotId);
await this._onInventoryChange(docId);
} finally {
// Clean up backup.
// NOTE: fse.remove succeeds also when the file does not exist.