Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the default conflict resolver for the BLIP tester to handle tombstones when resolving conflicts. The changes ensure that when either the local or remote document is a tombstone, the conflict resolution favors the non-deleted document.
- Updated
_resolveConflictLWWto prioritize non-deleted documents over tombstones during conflict resolution - Added helper functions to determine when conflicts are expected in CBL topologies
- Modified resurrection test logic to handle tombstone convergence when CBL resolves in favor of its own tombstone
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| rest/utilities_testing_blip_client.go | Enhanced conflict resolver to check for tombstones and prefer non-deleted documents |
| topologytest/peer_test.go | Added helper functions to identify server-side peers and determine expected conflicts in CBL topologies |
| topologytest/multi_actor_no_conflict_test.go | Updated resurrection test to wait for tombstone convergence when CBL conflicts are expected |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
torcolvin
left a comment
There was a problem hiding this comment.
Make sure to run topologytests with CBS (you can do this locally with SG_TEST_TOPOLOGY_TESTS=true or with jenkins job, https://jenkins.sgwdev.com/job/TopologyTests/)
| newBody, updatedHLV = btcc._resolveConflict(opts.incomingHLV, opts.body, doc._latestRev(btcc.TB())) | ||
| newBody, updatedHLV, isTombstone = btcc._resolveConflict(opts.incomingHLV, opts.body, opts.isDelete, doc._latestRev(btcc.TB())) | ||
| if isTombstone { | ||
| opts.isDelete = true |
There was a problem hiding this comment.
one tweak and I think I'm fine with this, I think I declare:
isDelete := opts.isDelete
And then use that variable to use a local variable. The reason for this is to match newBody and newVersion behavior and to make the options struct effectively immutable to make this easier to do debugging.
There was a problem hiding this comment.
Pushed update for this but I can't get onto Jenkins with this netspoke stuff. Going to bring this up to IT soon.
CBG-4905
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiDependencies (if applicable)
Integration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGatewayIntegration/183/