From 3873707085a9e92dc74ebd99129dfc730b9af7ca Mon Sep 17 00:00:00 2001 From: Gregory Newman-Smith Date: Thu, 20 Nov 2025 10:51:47 +0000 Subject: [PATCH 1/6] CBG-4905: update defualt resolver for blip tester --- rest/utilities_testing_blip_client.go | 31 +++++++++++++++----- topologytest/multi_actor_no_conflict_test.go | 12 +++++++- topologytest/peer_test.go | 19 ++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/rest/utilities_testing_blip_client.go b/rest/utilities_testing_blip_client.go index cbe748de0f..f2c04f57c1 100644 --- a/rest/utilities_testing_blip_client.go +++ b/rest/utilities_testing_blip_client.go @@ -339,25 +339,38 @@ func (cd *clientDoc) _hasConflict(t testing.TB, incomingHLV *db.HybridLogicalVec return false } -func (btcc *BlipTesterCollectionClient) _resolveConflict(incomingHLV *db.HybridLogicalVector, incomingBody []byte, localDoc *clientDocRev) (body []byte, hlv db.HybridLogicalVector) { +func (btcc *BlipTesterCollectionClient) _resolveConflict(incomingHLV *db.HybridLogicalVector, incomingBody []byte, incomingIsDelete bool, localDoc *clientDocRev) (body []byte, hlv db.HybridLogicalVector, isTombstone bool) { switch btcc.parent.ConflictResolver { case ConflictResolverLastWriteWins: - return btcc._resolveConflictLWW(incomingHLV, incomingBody, localDoc) + return btcc._resolveConflictLWW(incomingHLV, incomingBody, incomingIsDelete, localDoc) } btcc.TB().Fatalf("Unknown conflict resolver %q - cannot resolve detected conflict", btcc.parent.ConflictResolver) - return nil, db.HybridLogicalVector{} + return nil, db.HybridLogicalVector{}, isTombstone } -func (btcc *BlipTesterCollectionClient) _resolveConflictLWW(incomingHLV *db.HybridLogicalVector, incomingBody []byte, latestLocalRev *clientDocRev) (body []byte, hlv db.HybridLogicalVector) { +func (btcc *BlipTesterCollectionClient) _resolveConflictLWW(incomingHLV *db.HybridLogicalVector, incomingBody []byte, incomingIsDelete bool, latestLocalRev *clientDocRev) (body []byte, hlv db.HybridLogicalVector, isTombstone bool) { + fmt.Println("incoming hlv in resolver", incomingHLV) latestLocalHLV := latestLocalRev.HLV + fmt.Println("local hlv in resolver", latestLocalHLV) updatedHLV := latestLocalRev.HLV.Copy() + localDeleted := latestLocalRev.isDelete + if localDeleted && !incomingIsDelete { + // resolve in favour of local document + incomingHLV.UpdateWithIncomingHLV(updatedHLV) + return latestLocalRev.body, *updatedHLV, true + } + if incomingIsDelete && !localDeleted { + // resolve in favour of remote document + updatedHLV.UpdateWithIncomingHLV(incomingHLV) + return incomingBody, *updatedHLV, true + } // resolve conflict in favor of remote document if incomingHLV.Version > latestLocalHLV.Version { updatedHLV.UpdateWithIncomingHLV(incomingHLV) - return incomingBody, *updatedHLV + return incomingBody, *updatedHLV, incomingIsDelete } incomingHLV.UpdateWithIncomingHLV(updatedHLV) - return latestLocalRev.body, *updatedHLV + return latestLocalRev.body, *updatedHLV, latestLocalRev.isDelete } type BlipTesterCollectionClient struct { @@ -2180,6 +2193,7 @@ func (btcc *BlipTesterCollectionClient) addRev(ctx context.Context, docID string btcc.seqLock.Lock() defer btcc.seqLock.Unlock() newClientSeq := btcc._nextSequence() + isTombstone := false newBody := opts.body newVersion := opts.incomingVersion @@ -2187,7 +2201,10 @@ func (btcc *BlipTesterCollectionClient) addRev(ctx context.Context, docID string updatedHLV := doc._getLatestHLVCopy(btcc.TB()) require.NotNil(btcc.TB(), updatedHLV, "updatedHLV should not be nil for docID %q", docID) if doc._hasConflict(btcc.TB(), opts.incomingHLV) { - 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 + } base.DebugfCtx(ctx, base.KeySGTest, "Resolved conflict for docID %q, incomingHLV:%#v, existingHLV:%#v, updatedHLV:%#v", docID, opts.incomingHLV, doc._latestRev(btcc.TB()).HLV, updatedHLV) } else { base.DebugfCtx(ctx, base.KeySGTest, "No conflict") diff --git a/topologytest/multi_actor_no_conflict_test.go b/topologytest/multi_actor_no_conflict_test.go index ae6ce87870..ed5051f6d1 100644 --- a/topologytest/multi_actor_no_conflict_test.go +++ b/topologytest/multi_actor_no_conflict_test.go @@ -107,8 +107,18 @@ func TestMultiActorResurrect(t *testing.T) { resBody := fmt.Appendf(nil, `{"activePeer": "%s", "createPeer": "%s", "deletePeer": "%s", "resurrectPeer": "%s", "topology": "%s", "action": "resurrect"}`, resurrectPeerName, createPeerName, deletePeer, resurrectPeer, topology.specDescription) resurrectVersion := resurrectPeer.WriteDocument(collectionName, docID, resBody) // in the case of a Couchbase Server resurrection, the hlv is lost since all system xattrs are lost on a resurrection + // if cbs resurrect, if delete AND resurrecting peer is server side peer (cbs or sgw) the all docs will converge for version expected + // if cbs resurrect and delete AND resurrecting peer is NOT server side peer (lite), then need to wait for tombstone convergence first if resurrectPeer.Type() == PeerTypeCouchbaseServer { - waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) + // almost has it, need to think if lite + l := IsServerSidePeer([]Peer{deletePeer, resurrectPeer}) + if !l { //if deletePeer.Type() == PeerTypeCouchbaseLite { //|| createPeer.Type() == PeerTypeCouchbaseLite { //strings.Contains(deletePeer.Type().String(), "Couchbase Lite Peer") { + fmt.Println("waiting for converging tombstones") + waitForConvergingTombstones(t, collectionName, docID, topology) + } else { + waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) + } + //waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) } else { waitForVersionAndBody(t, collectionName, docID, resurrectVersion, topology) } diff --git a/topologytest/peer_test.go b/topologytest/peer_test.go index 3cfbbe564f..21f03616c2 100644 --- a/topologytest/peer_test.go +++ b/topologytest/peer_test.go @@ -127,6 +127,21 @@ func (p Peers) SortedPeers() iter.Seq2[string, Peer] { } } +func (p Peers) PeerList() []string { + peerNames := slices.Collect(maps.Keys(p)) + return peerNames +} + +func IsServerSidePeer(p []Peer) bool { + for _, v := range p { + if v.Type() == PeerTypeCouchbaseServer || v.Type() == PeerTypeSyncGateway { + continue + } + return false + } + return true +} + // NonImportSortedPeers returns a sorted iterator peers that will not cause import operations. For example: // - cbs1 <-> sg1 <-> cbl1 would return sg1 and cbl1, but not cbs1 // - cbs1 <-> cbs2 would return cbs1 and cbs2 @@ -245,6 +260,10 @@ func (to Topology) SortedPeers() iter.Seq2[string, Peer] { return to.peers.SortedPeers() } +func (to Topology) PeersInTopology() []string { + return to.peers.PeerList() +} + // CompareRevTreeOnly is true for a given topology when comparing only revtree is correct. func (to Topology) CompareRevTreeOnly() bool { for _, peer := range to.peers.ActivePeers() { From 4a680279da0aefc26219b9572276d07873fed506 Mon Sep 17 00:00:00 2001 From: Gregory Newman-Smith Date: Mon, 24 Nov 2025 12:55:50 +0000 Subject: [PATCH 2/6] additional changes --- topologytest/multi_actor_no_conflict_test.go | 9 ++++----- topologytest/peer_test.go | 19 ++++++++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/topologytest/multi_actor_no_conflict_test.go b/topologytest/multi_actor_no_conflict_test.go index ed5051f6d1..480276786f 100644 --- a/topologytest/multi_actor_no_conflict_test.go +++ b/topologytest/multi_actor_no_conflict_test.go @@ -110,13 +110,12 @@ func TestMultiActorResurrect(t *testing.T) { // if cbs resurrect, if delete AND resurrecting peer is server side peer (cbs or sgw) the all docs will converge for version expected // if cbs resurrect and delete AND resurrecting peer is NOT server side peer (lite), then need to wait for tombstone convergence first if resurrectPeer.Type() == PeerTypeCouchbaseServer { - // almost has it, need to think if lite - l := IsServerSidePeer([]Peer{deletePeer, resurrectPeer}) - if !l { //if deletePeer.Type() == PeerTypeCouchbaseLite { //|| createPeer.Type() == PeerTypeCouchbaseLite { //strings.Contains(deletePeer.Type().String(), "Couchbase Lite Peer") { + if conflictNotExpectedOnCBL([]Peer{createPeer, deletePeer, resurrectPeer}, []string{deletePeerName, createPeerName}, resurrectPeerName) { //allActorsServerSide([]Peer{createPeer, deletePeer, resurrectPeer}) || conflictNotExpectedOnCBL(deletePeer, []string{deletePeerName, createPeerName}, resurrectPeerName) { + // no cbl conflict? + waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) + } else { fmt.Println("waiting for converging tombstones") waitForConvergingTombstones(t, collectionName, docID, topology) - } else { - waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) } //waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) } else { diff --git a/topologytest/peer_test.go b/topologytest/peer_test.go index 21f03616c2..54bbbea703 100644 --- a/topologytest/peer_test.go +++ b/topologytest/peer_test.go @@ -132,7 +132,7 @@ func (p Peers) PeerList() []string { return peerNames } -func IsServerSidePeer(p []Peer) bool { +func allActorsServerSide(p []Peer) bool { for _, v := range p { if v.Type() == PeerTypeCouchbaseServer || v.Type() == PeerTypeSyncGateway { continue @@ -142,6 +142,23 @@ func IsServerSidePeer(p []Peer) bool { return true } +func deleteHasSGWSource(deletingPeer Peer) bool { + if deletingPeer.Type() == PeerTypeCouchbaseServer || deletingPeer.Type() == PeerTypeSyncGateway { + return true + } + return false +} + +func conflictNotExpectedOnCBL(peers []Peer, createDelPeerNames []string, resPeerName string) bool { + if createDelPeerNames[1] == "sg1" && resPeerName == "cbs1" { + return true + } + if createDelPeerNames[1] == "cbs1" && resPeerName == "cbs1" { + return true + } + return false +} + // NonImportSortedPeers returns a sorted iterator peers that will not cause import operations. For example: // - cbs1 <-> sg1 <-> cbl1 would return sg1 and cbl1, but not cbs1 // - cbs1 <-> cbs2 would return cbs1 and cbs2 From 74a6712703b35e083dda181dfb353cf35ad22c9d Mon Sep 17 00:00:00 2001 From: Gregory Newman-Smith Date: Mon, 1 Dec 2025 15:11:51 +0000 Subject: [PATCH 3/6] fix failing tests and clear up work --- topologytest/multi_actor_no_conflict_test.go | 16 +++++---- topologytest/peer_test.go | 38 +++++++++++--------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/topologytest/multi_actor_no_conflict_test.go b/topologytest/multi_actor_no_conflict_test.go index 480276786f..26f5cee6f3 100644 --- a/topologytest/multi_actor_no_conflict_test.go +++ b/topologytest/multi_actor_no_conflict_test.go @@ -10,6 +10,7 @@ package topologytest import ( "fmt" + "strings" "testing" "github.com/couchbase/sync_gateway/base" @@ -110,14 +111,17 @@ func TestMultiActorResurrect(t *testing.T) { // if cbs resurrect, if delete AND resurrecting peer is server side peer (cbs or sgw) the all docs will converge for version expected // if cbs resurrect and delete AND resurrecting peer is NOT server side peer (lite), then need to wait for tombstone convergence first if resurrectPeer.Type() == PeerTypeCouchbaseServer { - if conflictNotExpectedOnCBL([]Peer{createPeer, deletePeer, resurrectPeer}, []string{deletePeerName, createPeerName}, resurrectPeerName) { //allActorsServerSide([]Peer{createPeer, deletePeer, resurrectPeer}) || conflictNotExpectedOnCBL(deletePeer, []string{deletePeerName, createPeerName}, resurrectPeerName) { - // no cbl conflict? - waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) + if strings.Contains(topologySpec.description, "CBL") { + if conflictNotExpectedOnCBL(deletePeer, resurrectPeer, deletePeerName, resurrectPeerName) { + // if no cbl conflict is expected we can wait on CV and body + waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) + } else { + // if cbl conflict is expected we need to wait for tombstone convergence + waitForConvergingTombstones(t, collectionName, docID, topology) + } } else { - fmt.Println("waiting for converging tombstones") - waitForConvergingTombstones(t, collectionName, docID, topology) + waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) } - //waitForCVAndBody(t, collectionName, docID, resurrectVersion, topology) } else { waitForVersionAndBody(t, collectionName, docID, resurrectVersion, topology) } diff --git a/topologytest/peer_test.go b/topologytest/peer_test.go index 54bbbea703..8747da270a 100644 --- a/topologytest/peer_test.go +++ b/topologytest/peer_test.go @@ -132,30 +132,36 @@ func (p Peers) PeerList() []string { return peerNames } -func allActorsServerSide(p []Peer) bool { - for _, v := range p { - if v.Type() == PeerTypeCouchbaseServer || v.Type() == PeerTypeSyncGateway { - continue - } - return false - } - return true -} - -func deleteHasSGWSource(deletingPeer Peer) bool { - if deletingPeer.Type() == PeerTypeCouchbaseServer || deletingPeer.Type() == PeerTypeSyncGateway { +// peerIsServerSide returns true if the peer is a Couchbase Server or Sync Gateway peer. +func peerIsServerSide(p Peer) bool { + if p.Type() == PeerTypeCouchbaseServer || p.Type() == PeerTypeSyncGateway { return true } return false } -func conflictNotExpectedOnCBL(peers []Peer, createDelPeerNames []string, resPeerName string) bool { - if createDelPeerNames[1] == "sg1" && resPeerName == "cbs1" { - return true +// conflictNotExpectedOnCBL will return true if no conflict is expected for delete and resurrect operations for +// topologies with cbl peer in them and false for expected conflict +func conflictNotExpectedOnCBL(deletePeer Peer, resurrectPeer Peer, delPeerName string, resPeerName string) bool { + if delPeerName == "cbl1" { + // cbl delete will mean cbs resurrect has a conflict + return false } - if createDelPeerNames[1] == "cbs1" && resPeerName == "cbs1" { + if peerIsServerSide(deletePeer) && peerIsServerSide(resurrectPeer) { + if strings.Contains(delPeerName, "1") && strings.Contains(resPeerName, "2") { + fmt.Println("conflict expected due to different backing bucket") + // conflict expected due to different backing bucket (sourceID) + return false + } + if strings.Contains(delPeerName, "2") && strings.Contains(resPeerName, "1") { + fmt.Println("conflict expected due to different backing bucket") + // conflict expected due to different backing bucket (sourceID) + return false + } + // if both actors are server side and same backing bucket, no conflict expected return true } + // conflict expected return false } From ce08c33d2096dd170ddde8ce61c583bf532b9d1b Mon Sep 17 00:00:00 2001 From: Gregory Newman-Smith Date: Mon, 1 Dec 2025 15:57:15 +0000 Subject: [PATCH 4/6] alter comments + remove debugging lines --- rest/utilities_testing_blip_client.go | 2 -- topologytest/multi_actor_no_conflict_test.go | 6 +++--- topologytest/peer_test.go | 11 ----------- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/rest/utilities_testing_blip_client.go b/rest/utilities_testing_blip_client.go index f2c04f57c1..aa890add49 100644 --- a/rest/utilities_testing_blip_client.go +++ b/rest/utilities_testing_blip_client.go @@ -349,9 +349,7 @@ func (btcc *BlipTesterCollectionClient) _resolveConflict(incomingHLV *db.HybridL } func (btcc *BlipTesterCollectionClient) _resolveConflictLWW(incomingHLV *db.HybridLogicalVector, incomingBody []byte, incomingIsDelete bool, latestLocalRev *clientDocRev) (body []byte, hlv db.HybridLogicalVector, isTombstone bool) { - fmt.Println("incoming hlv in resolver", incomingHLV) latestLocalHLV := latestLocalRev.HLV - fmt.Println("local hlv in resolver", latestLocalHLV) updatedHLV := latestLocalRev.HLV.Copy() localDeleted := latestLocalRev.isDelete if localDeleted && !incomingIsDelete { diff --git a/topologytest/multi_actor_no_conflict_test.go b/topologytest/multi_actor_no_conflict_test.go index 26f5cee6f3..49c8160180 100644 --- a/topologytest/multi_actor_no_conflict_test.go +++ b/topologytest/multi_actor_no_conflict_test.go @@ -107,9 +107,9 @@ func TestMultiActorResurrect(t *testing.T) { resBody := fmt.Appendf(nil, `{"activePeer": "%s", "createPeer": "%s", "deletePeer": "%s", "resurrectPeer": "%s", "topology": "%s", "action": "resurrect"}`, resurrectPeerName, createPeerName, deletePeer, resurrectPeer, topology.specDescription) resurrectVersion := resurrectPeer.WriteDocument(collectionName, docID, resBody) - // in the case of a Couchbase Server resurrection, the hlv is lost since all system xattrs are lost on a resurrection - // if cbs resurrect, if delete AND resurrecting peer is server side peer (cbs or sgw) the all docs will converge for version expected - // if cbs resurrect and delete AND resurrecting peer is NOT server side peer (lite), then need to wait for tombstone convergence first + // in the case of a Couchbase Server resurrection, the hlv is lost since all system xattrs are + // lost on a resurrection so the resurrecting version may conflict with a version on cbl + // peer then cbl will resolve in favour if its own tombstone. if resurrectPeer.Type() == PeerTypeCouchbaseServer { if strings.Contains(topologySpec.description, "CBL") { if conflictNotExpectedOnCBL(deletePeer, resurrectPeer, deletePeerName, resurrectPeerName) { diff --git a/topologytest/peer_test.go b/topologytest/peer_test.go index 8747da270a..b2d2edeaee 100644 --- a/topologytest/peer_test.go +++ b/topologytest/peer_test.go @@ -127,11 +127,6 @@ func (p Peers) SortedPeers() iter.Seq2[string, Peer] { } } -func (p Peers) PeerList() []string { - peerNames := slices.Collect(maps.Keys(p)) - return peerNames -} - // peerIsServerSide returns true if the peer is a Couchbase Server or Sync Gateway peer. func peerIsServerSide(p Peer) bool { if p.Type() == PeerTypeCouchbaseServer || p.Type() == PeerTypeSyncGateway { @@ -149,12 +144,10 @@ func conflictNotExpectedOnCBL(deletePeer Peer, resurrectPeer Peer, delPeerName s } if peerIsServerSide(deletePeer) && peerIsServerSide(resurrectPeer) { if strings.Contains(delPeerName, "1") && strings.Contains(resPeerName, "2") { - fmt.Println("conflict expected due to different backing bucket") // conflict expected due to different backing bucket (sourceID) return false } if strings.Contains(delPeerName, "2") && strings.Contains(resPeerName, "1") { - fmt.Println("conflict expected due to different backing bucket") // conflict expected due to different backing bucket (sourceID) return false } @@ -283,10 +276,6 @@ func (to Topology) SortedPeers() iter.Seq2[string, Peer] { return to.peers.SortedPeers() } -func (to Topology) PeersInTopology() []string { - return to.peers.PeerList() -} - // CompareRevTreeOnly is true for a given topology when comparing only revtree is correct. func (to Topology) CompareRevTreeOnly() bool { for _, peer := range to.peers.ActivePeers() { From b50b1cb7965aadab64de8433b0cb547158498a94 Mon Sep 17 00:00:00 2001 From: Gregory Newman-Smith Date: Mon, 1 Dec 2025 16:12:45 +0000 Subject: [PATCH 5/6] address review comment --- topologytest/peer_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/topologytest/peer_test.go b/topologytest/peer_test.go index b2d2edeaee..d7f7f90f02 100644 --- a/topologytest/peer_test.go +++ b/topologytest/peer_test.go @@ -129,10 +129,7 @@ func (p Peers) SortedPeers() iter.Seq2[string, Peer] { // peerIsServerSide returns true if the peer is a Couchbase Server or Sync Gateway peer. func peerIsServerSide(p Peer) bool { - if p.Type() == PeerTypeCouchbaseServer || p.Type() == PeerTypeSyncGateway { - return true - } - return false + return p.Type() == PeerTypeCouchbaseServer || p.Type() == PeerTypeSyncGateway } // conflictNotExpectedOnCBL will return true if no conflict is expected for delete and resurrect operations for From f2099874f0b223d818c3bd4312cd3a86cf5de0cf Mon Sep 17 00:00:00 2001 From: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com> Date: Mon, 1 Dec 2025 16:13:58 +0000 Subject: [PATCH 6/6] Update topologytest/multi_actor_no_conflict_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- topologytest/multi_actor_no_conflict_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/topologytest/multi_actor_no_conflict_test.go b/topologytest/multi_actor_no_conflict_test.go index 49c8160180..81480f61b1 100644 --- a/topologytest/multi_actor_no_conflict_test.go +++ b/topologytest/multi_actor_no_conflict_test.go @@ -109,7 +109,7 @@ func TestMultiActorResurrect(t *testing.T) { resurrectVersion := resurrectPeer.WriteDocument(collectionName, docID, resBody) // in the case of a Couchbase Server resurrection, the hlv is lost since all system xattrs are // lost on a resurrection so the resurrecting version may conflict with a version on cbl - // peer then cbl will resolve in favour if its own tombstone. + // peer then cbl will resolve in favor if its own tombstone. if resurrectPeer.Type() == PeerTypeCouchbaseServer { if strings.Contains(topologySpec.description, "CBL") { if conflictNotExpectedOnCBL(deletePeer, resurrectPeer, deletePeerName, resurrectPeerName) {