Issue 359 support scaleway #577

fflorent · 2023-07-18T10:14:14Z

Context

ANCT Données et Territoire deploys Grist instances whose external storage are S3 and use the Minio connector. The S3 provider used is not MinIO but Scaleway.

Despite having enabled the versioning feature of the bucket, the snapshots feature had erratic behavior and mostly failed to work when importing documents.

The reason appears to be quite simple: that's because the versionId returned by Scaleway's API is not a string (as MinIO and AWS do), but is an integer.

Steps to reproduce

NB: Scaleway provides Free buckets with a limitation of 75Gb of storage.

Create some document and export it in the .grist format;
Make an instance of Grist and setup the env variables (GRIST_DOCS_MINIO_*) so it uses Scaleway bucket and some Redis instance;
Start the instance;
Log in;
Import the document;

You get this kind of error message:

operation failed to become consistent: versions - 238,not ready,3360,not ready,8698,not ready,17486,not ready

And this exception:

2023-07-13 10:13:34.086 - error: HostedStorageManager error pushing 1w663WJyaYMxHhaPGNVDkd (1): Error: operation failed to become consistent: versions - 119,not ready,3250,not ready,8461,not ready,17240,not ready
    at ChecksummedExternalStorage._retry (/grist/_build/app/server/lib/ExternalStorage.js:286:15)
    at async DocSnapshotInventory._reconstruct (/grist/_build/app/server/lib/DocSnapshots.js:287:27)
    at async DocSnapshotInventory._reconstruct (/grist/_build/app/server/lib/DocSnapshots.js:287:27)
    at async DocSnapshotInventory._getSnapshots (/grist/_build/app/server/lib/DocSnapshots.js:253:20)
    at async DocSnapshotInventory._getSnapshots (/grist/_build/app/server/lib/DocSnapshots.js:253:20)
    at async /grist/_build/app/server/lib/DocSnapshots.js:173:31
    at async /grist/_build/app/server/lib/DocSnapshots.js:173:31
    at async KeyedMutex.runExclusive (/grist/_build/app/common/KeyedMutex.js:32:20)
    at async KeyedMutex.runExclusive (/grist/_build/app/common/KeyedMutex.js:32:20)
    at async DocSnapshotInventory.uploadAndAdd (/grist/_build/app/server/lib/DocSnapshots.js:167:9)
    at async DocSnapshotInventory.uploadAndAdd (/grist/_build/app/server/lib/DocSnapshots.js:167:9)
    at async HostedStorageManager._pushToS3 (/grist/_build/app/server/lib/HostedStorageManager.js:692:13) docId=null
    at async HostedStorageManager._pushToS3 (/grist/_build/app/server/lib/HostedStorageManager.js:692:13) docId=null

The solution

I just cast the versionId to a string

Unit tests

I added some unit tests to cover most of the cases that could occur while using the versions() function.

Misc

I also fixed this error which occurred while Grist was waiting a long time for response from the S3 storage:

2023-07-12 11:14:39.717 - error: ActiveDoc failed to update data size Error: Tried to use DocStorage database before it was opened
    at DocStorage._getDB (/home/florentpro/projects/grist-core-scaleway/_build/app/server/lib/DocStorage.js:1253:19)
    at DocStorage.get (/home/florentpro/projects/grist-core-scaleway/_build/app/server/lib/DocStorage.js:1010:21)
    at DocStorage.getDataSizeUncached (/home/florentpro/projects/grist-core-scaleway/_build/app/server/lib/DocStorage.js:1098:35)
    at DocStorage.getDataSize (/home/florentpro/projects/grist-core-scaleway/_build/app/server/lib/DocStorage.js:1091:112)
    at ActiveDoc._updateDataSize (/home/florentpro/projects/grist-core-scaleway/_build/app/server/lib/ActiveDoc.js:1859:53)
    at ActiveDoc._checkDataSizeLimitRatio (/home/florentpro/projects/grist-core-scaleway/_build/app/server/lib/ActiveDoc.js:1846:42)
    at Interval._callback (/home/florentpro/projects/grist-core-scaleway/_build/app/server/lib/ActiveDoc.js:185:48)
    at Interval._onTimeoutTriggered (/home/florentpro/projects/grist-core-scaleway/_build/app/common/Interval.js:76:49)
    at Timeout.<anonymous> (/home/florentpro/projects/grist-core-scaleway/_build/app/common/Interval.js:60:47)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7) docId=pfrVp1tA77Sm5ZdRx5JAVq

app/server/lib/ActiveDoc.ts

app/server/lib/MinIOExternalStorage.ts

paulfitz

Looks good, glad you tracked this subtle problem down. I see other uses of versionId such as in the head() method, does the MinIO client do a cast to string in statObject?

My comments are all bureaucratic in nature, nothing of substance.

test/server/lib/MinIOExternalStorage.ts

fflorent · 2023-07-19T18:35:06Z

I see other uses of versionId such as in the head() method, does the MinIO client do a cast to string in statObject?

Thanks a lot @paulfitz for this valuable remark! 🙏

I checked the type of versionId for the results of stat() and putObject().

And that's strange that in these case, versionId is a string.

I realized that it might not be a bug of Grist but instead a minio-js inconsistency:

statObject returns the response header value, hence it is a string;
same for putObject;
However, listObjects gets its result from an XML parser, without any casting;

Therefore, I tend to think it's a bug in minio-js. We may keep the current patch as provided (maybe change the comment), but I would send a patch to the MinIO client.

paulfitz

Thanks @fflorent

paulfitz · 2023-07-19T21:34:26Z

Therefore, I tend to think it's a bug in minio-js. We may keep the current patch as provided (maybe change the comment), but I would send a patch to the MinIO client.

It don't know if I'd classify it as a bug exactly, I think I remember AWS S3 docs being a little vague on when exactly VersionId is included in results, and needing to probe them empirically. Still, bug or not, to be a good drop-in S3 replacement, making a patch to clean up this corner of the MinIO client (when used with Scaleway) would be helpful.

fflorent · 2023-07-20T09:05:52Z

making a patch to clean up this corner of the MinIO client (when used with Scaleway) would be helpful.

Done :) : https://github.com/minio/minio-js/pull/1193/files

I am changing some comments and test descriptions of the PR to reflect that.

While MinIO and AWS return versionId as strings, other S3 API implementations return versionId as integers. We must carefully convert the versionId as string in order to cover these various behaviors. Also ensure that docStorage is initialized before attempting to calculate the data size in order to avoid an exception.

Introduced some unit tests to : - ensure listObjects is called with the right arguments; - cover the case when a S3 bucket implementation does not return the versionId as a string but rather as an integer (like Scaleway): in such a case, ensure that the returned snapshotId is a string; - cover the case when the listObjects function emits an error, ensure the versions() call rejets with the error emitted; - that the deleteMarkers are only returned when the includeDeleteMarkers is passed;

paulfitz · 2023-07-20T10:17:35Z

making a patch to clean up this corner of the MinIO client (when used with Scaleway) would be helpful.

Done :) : https://github.com/minio/minio-js/pull/1193/files

Great! Thanks a lot @fflorent.

fflorent commented Jul 18, 2023

View reviewed changes

app/server/lib/ActiveDoc.ts Show resolved Hide resolved

app/server/lib/MinIOExternalStorage.ts Show resolved Hide resolved

fflorent mentioned this pull request Jul 18, 2023

How to make snapshots work on self-hosted ? #359

Closed

fflorent force-pushed the issue-359-support-scaleway branch from a071399 to 11e401b Compare July 19, 2023 14:13

paulfitz reviewed Jul 19, 2023

View reviewed changes

test/server/lib/MinIOExternalStorage.ts Show resolved Hide resolved

test/server/lib/MinIOExternalStorage.ts Outdated Show resolved Hide resolved

test/server/lib/MinIOExternalStorage.ts Show resolved Hide resolved

fflorent force-pushed the issue-359-support-scaleway branch from 11e401b to 48e5f64 Compare July 19, 2023 14:27

paulfitz approved these changes Jul 19, 2023

View reviewed changes

Florent FAYOLLE and others added 2 commits July 20, 2023 11:26

fflorent force-pushed the issue-359-support-scaleway branch from 48e5f64 to 56288f6 Compare July 20, 2023 09:26

paulfitz approved these changes Jul 20, 2023

View reviewed changes

paulfitz merged commit 5e33b68 into gristlabs:main Jul 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 359 support scaleway #577

Issue 359 support scaleway #577

fflorent commented Jul 18, 2023

paulfitz left a comment

fflorent commented Jul 19, 2023 •

edited

Loading

paulfitz left a comment

paulfitz commented Jul 19, 2023

fflorent commented Jul 20, 2023

paulfitz commented Jul 20, 2023

Issue 359 support scaleway #577

Issue 359 support scaleway #577

Conversation

fflorent commented Jul 18, 2023

Context

Steps to reproduce

The solution

Unit tests

Misc

paulfitz left a comment

Choose a reason for hiding this comment

fflorent commented Jul 19, 2023 • edited Loading

paulfitz left a comment

Choose a reason for hiding this comment

paulfitz commented Jul 19, 2023

fflorent commented Jul 20, 2023

paulfitz commented Jul 20, 2023

fflorent commented Jul 19, 2023 •

edited

Loading