Skip to content

Conversation

gregns1
Copy link
Contributor

@gregns1 gregns1 commented Oct 6, 2025

CBG-4894

  • Adds logic to have local or remote wins from the default resolver if the local or remote document is a tombstone
  • This was a bug in 4.0.0 implementation and aligns with what CBL does
  • I did think about making this change to blip tester client but topology tests do not seem to like this at all so if we want to make that change I feel it needs its separate ticket given the debugging effort involved to understand why this is happening

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • 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

Integration Tests

@Copilot Copilot AI review requested due to automatic review settings October 6, 2025 11:26
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the default conflict resolver for ISGR by adding logic to handle local or remote tombstones, aligning the behavior with Couchbase Lite. The change ensures that when one document is a tombstone and the other is not, the tombstone wins in the conflict resolution.

  • Adds tombstone consideration logic to the default conflict resolver
  • Removes outdated test comments related to CBG-4799
  • Adds comprehensive test coverage for both local and remote tombstone scenarios

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
rest/replicatortest/replicator_test.go Removes outdated comments about CBG-4799 refactoring
rest/replicatortest/replicator_conflict_test.go Adds new test cases for default resolver with local and remote tombstones
db/hybrid_logical_vector.go Implements the core logic to handle tombstones in conflict resolution

Copy link
Collaborator

@torcolvin torcolvin left a comment

Choose a reason for hiding this comment

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

LGTM, I wonder if you want to look if we can shorten the boilerplate for the tests to make them easier to read and this is a good time in the release cycle to think about this.

@gregns1 gregns1 self-assigned this Oct 6, 2025
@gregns1 gregns1 assigned torcolvin and unassigned gregns1 Oct 7, 2025
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.

2 participants