perf(sync): add WithMinCheckpointInterval syncer option#865
Open
arreyder wants to merge 1 commit into
Open
Conversation
Checkpoints compress and persist the entire c1z SQLite file. For large
tenants (Eli Lilly: 72 GB c1z), this dominates CPU — profile shows 60%
of Lilly's sync time in saveC1z → zstd.Encode → io.Copy on every step.
The existing 10s throttle (minCheckpointInterval) is hardcoded. Add
WithMinCheckpointInterval(d) as a SyncOpt so callers (c1's sync
activity) can tune per-tenant or per-connector-size.
Higher values reduce checkpoint I/O at the cost of more lost work on
a hard crash (not a clean activity-window timeout, which always
checkpoints via force=true). Lilly's GLB connector has had zero hard
crashes in 15 days of data — only clean timeouts.
Example usage from c1:
syncer.New(connector, opts...,
sync.WithMinCheckpointInterval(2 * time.Minute),
)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ghost
reviewed
May 24, 2026
Comment on lines
+2682
to
+2693
| // WithMinCheckpointInterval sets the minimum time between non-forced | ||
| // checkpoints. Checkpoints compress and persist the c1z file; for large | ||
| // tenants this dominates CPU. Higher values reduce checkpoint overhead at | ||
| // the cost of more lost work on a hard crash. Default 10s. | ||
| func WithMinCheckpointInterval(d time.Duration) SyncOpt { | ||
| return func(s *syncer) { | ||
| if d > 0 { | ||
| s.minCheckpointInterval = d | ||
| } | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🟡 Suggestion: No tests were added for the new WithMinCheckpointInterval option. A small unit test verifying the option sets the field, rejects zero/negative durations, and that Checkpoint respects the configured interval would guard against regressions — especially since this is an exported SDK API that downstream callers will depend on.
Contributor
General PR Review: perf(sync): add WithMinCheckpointInterval syncer optionBlocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0 Review SummaryThis PR adds a new Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
WithMinCheckpointInterval(d time.Duration)as aSyncOptso callers can tune how often the syncer persists the c1z file during non-forced checkpoints.Problem
Profile of be-temporal-sync for a large-tenant connector (72 GB c1z, 180K users, 139K groups) shows 60% of sync CPU in
CheckpointSync→saveC1z→ zstd compression → io.Copy. The current hardcodedminCheckpointInterval = 10smeans the file is compressed and written to disk every 10 seconds.For context:
Fix
minCheckpointIntervalconst todefaultMinCheckpointIntervalminCheckpointInterval time.Durationfield onsyncerstructWithMinCheckpointInterval(d)optionCheckpoint(ctx, force=false)uses the configured interval (falls back to default 10s)Checkpoint(ctx, force=true)always checkpoints (unchanged — clean exits still save)Usage
Setting 2 minutes reduces checkpoint overhead by ~12x (from every 10s to every 120s). The tradeoff: up to 2 minutes of lost work on a hard crash instead of 10 seconds.
Test plan
go vet ./pkg/sync/...— cleango test ./pkg/sync/...— all pass🤖 Generated with Claude Code