Skip to content

Commit

Permalink
fix(replicache): Wrap persist in a lock. (#3577)
Browse files Browse the repository at this point in the history
This was triggered in the zero perf benchmark which calls persist.
Sometimes this call got overlapped with the scheduled persist call which
raised the assert.

Instead of asserting in this condition we wait until the current persist
operation is done.
  • Loading branch information
arv authored Jan 21, 2025
1 parent 936ac9b commit cc36b3f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 15 deletions.
23 changes: 10 additions & 13 deletions packages/replicache/src/replicache-impl.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {Lock} from '@rocicorp/lock';
import {consoleLogSink, LogContext} from '@rocicorp/logger';
import {resolver} from '@rocicorp/resolver';
import {AbortError} from '../../shared/src/abort-error.js';
Expand Down Expand Up @@ -291,7 +292,7 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {

readonly #closeAbortController = new AbortController();

#persistIsRunning = false;
readonly #persistLock = new Lock();
readonly #enableScheduledPersist: boolean;
readonly #enableScheduledRefresh: boolean;
readonly #enablePullAndPushInOpen: boolean;
Expand Down Expand Up @@ -1131,10 +1132,9 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
return {requestID, syncHead, ok: httpRequestInfo.httpStatusCode === 200};
}

async persist(): Promise<void> {
assert(!this.#persistIsRunning);
this.#persistIsRunning = true;
try {
persist(): Promise<void> {
// Prevent multiple persist calls from running at the same time.
return this.#persistLock.withLock(async () => {
const {clientID} = this;
await this.#ready;
if (this.#closed) {
Expand All @@ -1147,7 +1147,7 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
this.memdag,
this.perdag,
this.#mutatorRegistry,
() => this.closed,
() => this.#closed,
FormatVersion.Latest,
);
} catch (e) {
Expand All @@ -1159,14 +1159,11 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
throw e;
}
}
} finally {
this.#persistIsRunning = false;
}

const {clientID} = this;
const clientGroupID = await this.#clientGroupIDPromise;
assert(clientGroupID);
this.#onPersist({clientID, clientGroupID});
const clientGroupID = await this.#clientGroupIDPromise;
assert(clientGroupID);
this.#onPersist({clientID, clientGroupID});
});
}

async refresh(): Promise<void> {
Expand Down
2 changes: 0 additions & 2 deletions packages/zero-client/src/client/zero.bench.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {resolver} from '@rocicorp/resolver';
import {bench, describe, expect} from 'vitest';
import {sleep} from '../../../shared/src/sleep.js';
import type {Row} from '../../../zql/src/query/query.js';
import {getInternalReplicacheImplForTesting, Zero} from './zero.js';

Expand Down Expand Up @@ -44,7 +43,6 @@ async function withZero(
});
await f(z);
if (persist) {
await sleep(500);
await getInternalReplicacheImplForTesting(z).persist();
}
await z.close();
Expand Down

0 comments on commit cc36b3f

Please sign in to comment.