Skip to content

CBG-4796 Match Rosmar xattr and tombstone behaviour to Couchbase Server - #8832

Merged
bbrks merged 29 commits into
mainfrom
CBG-4796
Oct 14, 2026
Merged

bbrks merged 29 commits into
mainfrom
CBG-4796

Conversation

@torcolvin

@torcolvin torcolvin commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

CBG-4796

Adds tests that compare Rosmar with Couchbase Server, and updates rosmar so that it matches Couchbase Server.

  • Multi-xattr operations
  • Results of delete and remove
  • DeleteWithXattrs
  • Writes to tombstones

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)

Add tests comparing Rosmar and Couchbase Server for multi-xattr ops,
delete/remove results, DeleteWithXattrs, and writes to tombstones, and
update rosmar to match Couchbase Server behaviour.

Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@torcolvin
torcolvin requested a review from bbrks September 28, 2026 17:18
@torcolvin
torcolvin marked this pull request as ready for review September 28, 2026 17:19
@torcolvin
torcolvin requested review from a team and a lite review from Copilot September 28, 2026 17:19
@factory-droid

factory-droid Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Droid finished @torcolvin's task —— View job


The change set looks directionally correct and the new cross-backend coverage is valuable. The two proposed tweaks focus on making the tests robust: avoid accidentally using CAS=0 in the “stale CAS” case, and tolerate partial xattr lookups (plus stable key comparisons) after deleting subsets of xattrs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

A moderate xattr test assertion issue remains unresolved.

Review effort: Lite
Findings: None

What changed in this PR

Updates Rosmar and related dependencies while expanding Couchbase Server parity coverage for xattrs, deletions, CAS behavior, and tombstones.

Changes:

  • Upgrades Rosmar and SQLite dependencies.
  • Expands xattr deletion and tombstone tests.
  • Adds datastore operation and compatibility coverage.
File Summary
go.sum Refreshes dependency checksums.
go.mod Updates Rosmar and SQLite versions.
base/​collection_xattr_test.go Expands xattr tombstone and deletion assertions.
base/​bucket_gocb_test.go Adds datastore operation and tombstone behavior tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread base/bucket_gocb_test.go
Comment thread base/collection_xattr_test.go
@bbrks bbrks assigned torcolvin and unassigned bbrks Oct 8, 2026
Comment thread go.mod Outdated
torcolvin and others added 2 commits October 8, 2026 10:26
Update rosmar to b1384a8, which includes the merged tombstone, expiry and nil
write changes. Add the matching tests from rosmar as subtests of
TestTombstonesAndNilWrites, which share one bucket, and add the RemoveXattrs,
DeleteSubDocPaths, Update and missing system xattr cases to the existing tests.

Move SafeDocumentName to base so that base tests can use it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…d events

Update rosmar to couchbaselabs/rosmar#103, which treats a path through a scalar
xattr in DeleteWithXattrs like a missing xattr, as Couchbase Server does. Add
the matching DeleteWithXattrsPathMismatch subtest.

TestXattrOnlyWriteKeepsTombstone now checks that the DCP feed reports each
xattr-only write to a tombstone as a deletion.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@torcolvin

Copy link
Copy Markdown
Collaborator Author

I ported all the rosmar tests to Sync Gateway after couchbaselabs/rosmar#99 and couchbaselabs/rosmar#101 were merged and I found couchbaselabs/rosmar#103.

This PR now contains the tests that match this behavior.

@torcolvin torcolvin assigned bbrks and unassigned torcolvin Oct 9, 2026
torcolvin and others added 9 commits October 12, 2026 10:17
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nentials (#8844)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…#8855)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
torcolvin and others added 13 commits October 12, 2026 10:17
…h user xattr only change (#8860)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…8858)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread db/util_testing.go Outdated
Comment thread rest/utilities_testing.go Outdated
@bbrks bbrks assigned torcolvin and unassigned bbrks Oct 14, 2026
@torcolvin torcolvin assigned bbrks and unassigned bbrks and torcolvin Oct 14, 2026
@bbrks
bbrks merged commit dd74f18 into main Oct 14, 2026
48 checks passed
@bbrks
bbrks deleted the CBG-4796 branch October 14, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants