[torcolvin]

CBG-5905, CBG-6057

• CBG-5905: Persist in-memory checkpoints. An empty checkpoint prefix matched every key, so in-memory feeds never recorded seqs, and a reopened stream restarted from its initial metadata.
• CBG-6057: Do not write checkpoints when the FeedEventCallback returns false, to match the behavior of cbgt. A false return still records the seq, and the next persist writes it.
• lastMetaPersistTime is now set, so persistence is throttled to metaPersistFrequency instead of a write on every event. A deferred checkpoint is flushed when the worker closes.
• One-shot checkpoint purge now runs after all workers stop, so a worker's flush on close cannot recreate a purged checkpoint.
• seqnoAdvancedEvent records its seq but does not trigger a persist.

Pre-review checklist

• Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
• Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

• Link upstream PRs
• Update Go module dependencies when merged

Integration Tests

• https://jenkins.sgwdev.com/job/SyncGatewayIntegration-Pipeline/282/

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Non-sharded import shutdown can close the bucket before the deferred checkpoint flush runs, losing pending checkpoint progress and causing processed events to replay after restart.

2 open issues

• Wait for import checkpoint flush before closing the bucket · New
• Add coverage for checkpoint persistence after the throttle interval · New

What changed in this PR

Improves Sync Gateway’s DCP checkpoint handling so feeds retain sequence progress, respect callback decisions, and coordinate checkpoint cleanup with worker shutdown.

Changes:

• Fixes sequence tracking when the checkpoint prefix is empty.
• Honors callback-controlled persistence, throttles writes, and flushes deferred checkpoints on worker close.
• Moves one-shot checkpoint cleanup after worker shutdown and adds persistence tests.

File	Description
base/​gocb_dcp_client.go	Delays one-shot checkpoint purge until all workers stop.
base/​dcp_client_worker.go	Updates sequence tracking, persistence controls, throttling, and close-time flushing.
base/​dcp_client_worker_test.go	Adds unit tests for callback decisions and deferred persistence.
base/​dcp_client_test.go	Adds callback-controlled checkpoint integration coverage and updates resume-test behavior.

_{Review effort: Balanced}

[factory-droid]

Droid finished @torcolvin's task —— View job

---

The DCP checkpoint persistence changes look correct overall, but there’s still a correctness risk if checkpoint writes fail yet the worker treats them as persisted (throttling retries and skipping the Close flush). Also, importListener.Stop’s doc comment says it’s async, but it can now block waiting for shutdown, so that contract should be clarified.

2 inline comments posted

[factory-droid]

[P2] Avoid treating failed checkpoint writes as persisted

DCPWorker.persist() always clears pendingPersist and updates lastMetaPersistTime after calling metadata.Persist(), but DCPMetadataCS.Persist() only logs KV write failures and returns no status. If a checkpoint write fails, this worker will still suppress retries for the throttle interval and Close() will not flush because pendingPersist was cleared, leaving persisted checkpoints behind in-memory seqs and potentially causing replay/backtracking after restart; consider changing the DCPMetadataStore Persist API to return an error (or success bool) so the worker only advances lastMetaPersistTime and clears pendingPersist on success.

[gregns1]

I don't think not updating lastMetaPersistTime on failure will be a good failure mode. This will mean each following event will request writing a checkpoint to a struggling cluster.

I wonder if we should not clear w.pendingPersist when we fail to write a checkpoint though? Then at least if Close is called it will attempt again to write?

[torcolvin]

Agreed. Persist now returns an error.

On failure, the worker still updates lastMetaPersistTime, so a cluster with errors does not get a write on every event, but it keeps pendingPersist set, so Close retries the write.

The worker also keeps sawNonSyncDoc after a failed write, so the retry is not counted as an idle KV op. TestDCPWorkerRetriesFailedCheckpointOnClose covers this.

[gregns1]

This looks good to be but I think i'd like a regression test in place for the situation where we have to wait for workers to close before purging checkpoints or risk workers re-persisting post purge. The existing testing around attachment migration etc will depend on whether the workers that ends last having a deferred persist pending. I think a more deterministic test around this would be good to protect against regression.

[gregns1]

I don't think not updating lastMetaPersistTime on failure will be a good failure mode. This will mean each following event will request writing a checkpoint to a struggling cluster.

I wonder if we should not clear w.pendingPersist when we fail to write a checkpoint though? Then at least if Close is called it will attempt again to write?