Remove GRIST_SKIP_REDIS_CHECKSUM_MISMATCH (#1098)

Skipping the redis checksum mismatch is now generalized. A warning is
logged when we see a mismatch.
This commit is contained in:
Florent 2024-07-10 20:28:20 +02:00 committed by GitHub
parent d33629366b
commit 39eb042ff1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 5 additions and 24 deletions

View File

@ -316,7 +316,6 @@ Grist can be configured in many ways. Here are the main environment variables it
| HOME_PORT | port number to listen on for REST API server; if set to "share", add API endpoints to regular grist port. | | HOME_PORT | port number to listen on for REST API server; if set to "share", add API endpoints to regular grist port. |
| PORT | port number to listen on for Grist server | | PORT | port number to listen on for Grist server |
| REDIS_URL | optional redis server for browser sessions and db query caching | | REDIS_URL | optional redis server for browser sessions and db query caching |
| GRIST_SKIP_REDIS_CHECKSUM_MISMATCH | Experimental. If set, only warn if the checksum in Redis differs with the one in your S3 backend storage. You may turn it on if your backend storage implements the [read-after-write consistency](https://aws.amazon.com/fr/blogs/aws/amazon-s3-update-strong-read-after-write-consistency/). Defaults to false. |
| GRIST_SNAPSHOT_TIME_CAP | optional. Define the caps for tracking buckets. Usage: {"hour": 25, "day": 32, "isoWeek": 12, "month": 96, "year": 1000} | | GRIST_SNAPSHOT_TIME_CAP | optional. Define the caps for tracking buckets. Usage: {"hour": 25, "day": 32, "isoWeek": 12, "month": 96, "year": 1000} |
| GRIST_SNAPSHOT_KEEP | optional. Number of recent snapshots to retain unconditionally for a document, regardless of when they were made | | GRIST_SNAPSHOT_KEEP | optional. Number of recent snapshots to retain unconditionally for a document, regardless of when they were made |
| GRIST_PROMCLIENT_PORT | optional. If set, serve the Prometheus metrics on the specified port number. ⚠️ Be sure to use a port which is not publicly exposed ⚠️. | | GRIST_PROMCLIENT_PORT | optional. If set, serve the Prometheus metrics on the specified port number. ⚠️ Be sure to use a port which is not publicly exposed ⚠️. |

View File

@ -1,5 +1,4 @@
import {ObjMetadata, ObjSnapshot, ObjSnapshotWithMetadata} from 'app/common/DocSnapshot'; import {ObjMetadata, ObjSnapshot, ObjSnapshotWithMetadata} from 'app/common/DocSnapshot';
import {isAffirmative} from 'app/common/gutil';
import log from 'app/server/lib/log'; import log from 'app/server/lib/log';
import {createTmpDir} from 'app/server/lib/uploads'; import {createTmpDir} from 'app/server/lib/uploads';
@ -236,19 +235,8 @@ export class ChecksummedExternalStorage implements ExternalStorage {
// We are confident this should not be the case anymore, though this has to be studied carefully. // We are confident this should not be the case anymore, though this has to be studied carefully.
// If a snapshotId was specified, we can skip this check. // If a snapshotId was specified, we can skip this check.
if (expectedChecksum && expectedChecksum !== checksum) { if (expectedChecksum && expectedChecksum !== checksum) {
const message = `ext ${this.label} download: data for ${fromKey} has wrong checksum:` + log.warn(`ext ${this.label} download: data for ${fromKey} has wrong checksum:` +
` ${checksum} (expected ${expectedChecksum})`; ` ${checksum} (expected ${expectedChecksum})`);
// If GRIST_SKIP_REDIS_CHECKSUM_MISMATCH is set, issue a warning only and continue,
// rather than issuing an error and failing.
// This flag is experimental and should be removed once we are
// confident that the checksums verification is useless.
if (isAffirmative(process.env.GRIST_SKIP_REDIS_CHECKSUM_MISMATCH)) {
log.warn(message);
} else {
log.error(message);
return undefined;
}
} }
} }

View File

@ -494,18 +494,12 @@ describe('HostedStorageManager', function() {
await setRedisChecksum(docId, 'nobble'); await setRedisChecksum(docId, 'nobble');
await store.removeAll(); await store.removeAll();
// With GRIST_SKIP_REDIS_CHECKSUM_MISMATCH set, the fetch should work const warnSpy = sandbox.spy(log, 'warn');
process.env.GRIST_SKIP_REDIS_CHECKSUM_MISMATCH = 'true';
await store.run(async () => { await store.run(async () => {
await assert.isFulfilled(store.docManager.fetchDoc(docSession, docId)); await assert.isFulfilled(store.docManager.fetchDoc(docSession, docId));
assert.isTrue(warnSpy.calledWithMatch('has wrong checksum'), 'a warning should have been logged');
}); });
warnSpy.restore();
// By default, the fetch should eventually errors.
delete process.env.GRIST_SKIP_REDIS_CHECKSUM_MISMATCH;
await store.run(async () => {
await assert.isRejected(store.docManager.fetchDoc(docSession, docId),
/operation failed to become consistent/);
});
// Check we get the document back on fresh start if checksum is correct. // Check we get the document back on fresh start if checksum is correct.
await setRedisChecksum(docId, checksum); await setRedisChecksum(docId, checksum);