Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes how Sync Gateway generates and persists HLV “current version” values for SG-owned actor writes: it introduces an SG-owned Hybrid Logical Clock (HLC) to generate a literal cv.ver up front (instead of relying on CAS macro expansion), removes per-channel revocation macro-expansion paths, and adds a bounded corrective re-stamp for the rare cv.ver > cas case to satisfy XDCR constraints.
Changes:
- Add
base.HybridLogicalClockand wire a per-DatabaseContextclock to generate HLV version values ahead of the write CAS. - Update HLV write/merge logic to use generated literal versions (leaving
cvCASmacro-expanded) and remove channel-removal macro-expansion plumbing. - Expand/adjust unit and integration tests for the new version semantics and add a new internal stat for corrective CAS re-stamps.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
base/hlc.go |
Introduces the SG-owned Hybrid Logical Clock implementation used to generate CAS-comparable version values. |
base/hlc_test.go |
Adds unit tests validating monotonicity, wall-clock tracking, floor behavior, and concurrency safety of the HLC. |
base/stats.go |
Adds hlv_version_cas_retry_count stat for tracking corrective re-stamps. |
base/stats_descriptions.go |
Adds description text for the new HLV/CAS correction stat. |
db/database.go |
Stores a per-DatabaseContext HLC instance (hlc) initialized at context creation time. |
db/crud.go |
Generates literal HLV versions via HLC, updates merge resolution to use HLC, removes revocation macro expansion, and adds CAS re-stamp correction logic. |
db/document.go |
Simplifies updateChannels signature by removing revoked-channel macro-expansion return values. |
db/hybrid_logical_vector.go |
Adds maxValueForSource helper and removes version macro-expansion behavior from computeMacroExpansions. |
db/utilities_hlv_testing.go |
Updates test helper macro-expansion behavior to model “external peers” explicitly; adjusts AlterHLVForTest; adds a DB helper to fetch HLC values in tests. |
db/hybrid_logical_vector_test.go |
Adds tests for CAS-ahead correction behavior and maxValueForSource. |
db/revision_cache_test.go |
Updates call site for changed updateChannels signature and aligns CV expectations with HLV version. |
db/database_test.go |
Updates resync expectations to compare against HLV version instead of CAS. |
db/crud_test.go |
Updates tests to stop assuming HLV version equals document CAS; removes now-dead subdoc-path tests. |
db/changes_test.go |
Updates CV population assertions to use HLV version semantics. |
rest/api_test.go |
Updates REST API tests to assert cv.ver <= cas and adds coverage for many channel removals. |
rest/changes_test.go |
Updates change feed CV assertions to match HLV version rather than CAS. |
rest/blip_api_crud_test.go |
Adds BLIP test ensuring large channel-removal updates persist correct removal metadata without per-channel macro-expansion paths. |
rest/replicatortest/replicator_test.go |
Updates replicator conflict tests to expect non-CAS HLV version semantics. |
rest/replicatortest/replicator_conflict_test.go |
Updates conflict-setup helpers to generate explicit HLC-backed version values rather than CAS macro placeholders. |
| if err != nil { | ||
| if base.IsCasMismatch(err) { | ||
| // A concurrent writer beat us to it; their mutation carries a later CAS, so the version is no | ||
| // longer ahead, and it's that writer's responsibility to satisfy the invariant. |
There was a problem hiding this comment.
Is it necessarily the case that their mutation with a later CAS is greater than the version, if it occurred quickly enough after our write? Might want to do a comparison and modify the log message accordingly.
May also want to include cas2 in the log message if it's returned on Cas mismatch error?
There was a problem hiding this comment.
modified the comment and log message, we don't return the cas on failure so haven't included that in the log message
| } | ||
| return db.dataStore.UpdateXattrs(ctx, key, 0, cas, map[string][]byte{ | ||
| base.SyncXattrName: syncXattr, | ||
| base.VvXattrName: vvXattr, |
There was a problem hiding this comment.
Can we populate _mou here, to avoid having eventing functions fire again on the restamped version? Or does this result in undesirable XDCR behaviour?
There was a problem hiding this comment.
I originally thought we didn't want mou populated, but I think after thinking about it more we do. I'm going to add that now
CBG-5436
Generate the HLV current version from a new SG-owned Hybrid Logical Clock before the write, so the value is known up front and written as a literal everywhere it's needed (cv, _sync.rev, and each channel removal). This removes the per-channel macro paths entirely, making subdoc path count independent of how many channels are removed. cvCAS stays macro-expanded to the committed CAS.
HybridLogicalClock(Now(floor) generates a monotonic, CAS-comparable value: 48-bit ns + 16-bit logical) — one perDatabaseContext.updateHLV/resolveDocMergeHLV— both now generate the version via the HLC; the version is generated early indocumentUpdateFunc(before the sync function) to widen the gap to the commit CAS.appendRevocationMacroExpansions, the per-channel xattr-path helpers, therevokedChannelsRequiringExpansionplumbing).cv.ver ≤ cas invariant (goxdcr)
A generated version must not exceed the document CAS (goxdcr rejects cv.ver > cas). The version is generated as early as possible so the later server CAS almost always exceeds it. As a safety net,
correctVersionAheadOfCASdetects cv.ver > cas for SG-generated versions, waits for the clock to catch up (bounded at 1s), and re-stamps the CAS while preserving the version — so the version is never moved backwards. Gaps over 1s are logged and left for a later mutation. This will also only run for our sourceID (writes coming in from clients will not have this correction behaviour).Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiDependencies (if applicable)
Integration Tests