CBG-4780: basic ISGR push and pull for 4.0 #7668

gregns1 · 2025-08-07T09:40:36Z

CBG-4780

PR does the minimum to get ISGR running in 4.0 replication. Conflicts are not handled yet.
Some test are skipped due to conflict issues (not implemented)
Some test assertions are changed given we don't currently keep rev IDs in sync each side of the replication (diff revIDs being generated each side even on new doc writes) so CV is only asserted at this time. I have linked tickets to change these when work takes place in other tickets.
I have also linked work in the write REST API work to change assertions based on that work too
One particular test is forced to run in 3.0 mode due to asserting docs created with same revIDs and bodies each each side get marked as peer doesn't want and asserts some check pointing behaviour around this. This is not applicable to 4.0 replication. Given we don’t generate same versions for docs with same id's and bodies like we do with revIDs.
New function to check incoming changes on changes message. I have tried to keep same sort of structure as RevsDiff given I think we can use known revs property for the rev tree property added later.
I will enhance conflict detecting for push replications on changes message in ticket CBG-4792 (similar behaviour to what proposeChanges does)
I have moved assertion changes from CBG-4288: some assertion cleanups for active replicator tests #7665 to here and will close it

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

Dependencies (if applicable)

Link upstream PRs
Update Go module dependencies when merged

Integration Tests

GSI=true,xattrs=true https://jenkins.sgwdev.com/job/SyncGateway-Integration/3242/

Copilot

Pull Request Overview

This PR implements basic Inter-Sync Gateway Replication (ISGR) push and pull functionality for version 4.0 protocol. It focuses on getting ISGR operational with minimal conflict handling support.

Key changes include:

Addition of new version checking functions for ISGR changes message handling
Updated test assertions to accommodate different revision ID generation between active and passive nodes
Temporary skipping of conflict resolution tests until proper ISGR conflict handling is implemented

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`rest/utilities_testing.go`	Adds new test utility functions for version comparison and changes assertion logic for ISGR
`rest/replicatortest/replicator_test_helper.go`	Removes unused test helper function
`rest/replicatortest/replicator_test.go`	Updates test assertions and skips conflict-related tests for ISGR compatibility
`rest/blip_api_crud_test.go`	Skips test due to revision tree differences between active and passive nodes
`rest/attachment_test.go`	Removes test function related to attachment handling
`db/utilities_hlv_testing.go`	Adds new function for revision tree equality checking
`db/document.go`	Adds method to extract document version from Document struct
`db/crud.go`	Implements CheckChangeVersion function for ISGR change handling
`db/blip_handler.go`	Updates changes message handling to support version vectors and removes conflict resolver restriction
`db/active_replicator_common.go`	Removes hardcoded BLIP protocol version restriction

Copilot · 2025-08-07T09:41:09Z

rest/utilities_testing.go

 }

 // RequireDocVersionNotEqual calls t.Fail if two document versions are equal.
 func RequireDocVersionNotEqual(t *testing.T, expected, actual DocVersion) {
-	require.False(t, expected.Equal(actual), "Versions match. Version should not be %s", expected)
+	// CBG-4751: should be able to uncomment this line once cv is included in write response
+	//require.NotEqual(t, expected.CV.String(), actual.CV.String(), "Versions mismatch.  Expected: %v, Actual: %v", expected, actual)


This commented-out code contains dev-time logging that should be removed rather than commented out. Either remove the line entirely or replace with appropriate Sync Gateway logging if needed.

Copilot · 2025-08-07T09:41:09Z

rest/blip_api_crud_test.go

@@ -2752,6 +2752,8 @@ func TestBlipInternalPropertiesHandling(t *testing.T) {
 // the stat mapping (processRevStats)
 func TestProcessRevIncrementsStat(t *testing.T) {
 	base.RequireNumTestBuckets(t, 2)
+	base.SetUpTestLogging(t, base.LevelDebug, base.KeyAll)


Debug logging setup in tests should be removed before production. This appears to be dev-time logging that was left in.

Copilot · 2025-08-07T09:41:10Z

rest/replicatortest/replicator_test.go

@@ -5921,6 +5959,8 @@ func TestActiveReplicatorPullConflictReadWriteIntlProps(t *testing.T) {
 func TestSGR2TombstoneConflictHandling(t *testing.T) {
 	base.LongRunningTest(t)
 	base.RequireNumTestBuckets(t, 2)
+	base.SetUpTestLogging(t, base.LevelDebug, base.KeyAll)


Debug logging setup in tests should be removed before production. This appears to be dev-time logging that was left in.

db/crud.go

Copilot · 2025-08-07T09:41:10Z

db/blip_handler.go

@@ -740,7 +753,7 @@ func (bh *blipHandler) handleChanges(rq *blip.Message) error {
 			if collectionCtx.sgr2PullAlreadyKnownSeqsCallback != nil {
 				seq, err := ParseJSONSequenceID(seqStr(bh.loggingCtx, change[0]))
 				if err != nil {
-					base.WarnfCtx(bh.loggingCtx, "Unable to parse known sequence %q for %q / %q: %v", change[0], base.UD(docID), revID, err)
+					base.WarnfCtx(bh.loggingCtx, "Unable to parse known sequence %q for %q / %q: %v", change[0], base.UD(docID), rev, err)


Good use of base.UD() to wrap the docID for user data redaction in logging.

Copilot · 2025-08-07T09:41:10Z

db/blip_handler.go

@@ -763,9 +776,9 @@ func (bh *blipHandler) handleChanges(rq *blip.Message) error {
 				seq, err := ParseJSONSequenceID(seqStr(bh.loggingCtx, change[0]))
 				if err != nil {
 					// We've already asked for the doc/rev for the sequence so assume we're going to receive it... Just log this and carry on
-					base.WarnfCtx(bh.loggingCtx, "Unable to parse expected sequence %q for %q / %q: %v", change[0], base.UD(docID), revID, err)
+					base.WarnfCtx(bh.loggingCtx, "Unable to parse expected sequence %q for %q / %q: %v", change[0], base.UD(docID), rev, err)


Good use of base.UD() to wrap the docID for user data redaction in logging.

bbrks · 2025-08-08T10:46:01Z

rest/replicatortest/replicator_test.go

-//   - Put a doc on a active rest tester
-//   - Create replication and wait for the doc to be replicated to passive node
-//   - Assert on the HLV in the metadata of the replicated document
-func TestReplicatorUpdateHLVOnPut(t *testing.T) {


Try a REST API PUT with new_edits=false to ensure we're generating a CV for a rev-ID write.

gregns1 self-assigned this Aug 7, 2025

Copilot AI review requested due to automatic review settings August 7, 2025 09:40

Copilot AI reviewed Aug 7, 2025

View reviewed changes

gregns1 added 3 commits August 7, 2025 10:45

CBG-4780: basic ISGR push and pull for 4.0

67fc8a0

tidy up

db1cd3b

lint

baa881f

gregns1 force-pushed the CBG-4780 branch from 6526e9a to baa881f Compare August 7, 2025 09:45

gregns1 assigned bbrks and unassigned gregns1 Aug 7, 2025

gregns1 mentioned this pull request Aug 7, 2025

CBG-4782: new rev tree property for ISGR replications #7671

Merged

5 tasks

bbrks approved these changes Aug 8, 2025

View reviewed changes

bbrks merged commit 972d34b into main Aug 8, 2025
44 checks passed

bbrks deleted the CBG-4780 branch August 8, 2025 11:23

gregns1 mentioned this pull request Aug 8, 2025

CBG-4798: add test for revID replicator write HLV generation #7674

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CBG-4780: basic ISGR push and pull for 4.0 #7668

CBG-4780: basic ISGR push and pull for 4.0 #7668

Uh oh!

gregns1 commented Aug 7, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 7, 2025

Uh oh!

Copilot AI Aug 7, 2025

Uh oh!

Copilot AI Aug 7, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI Aug 7, 2025

Uh oh!

Copilot AI Aug 7, 2025

Uh oh!

bbrks Aug 8, 2025

Uh oh!

Uh oh!

Uh oh!

CBG-4780: basic ISGR push and pull for 4.0 #7668

CBG-4780: basic ISGR push and pull for 4.0 #7668

Uh oh!

Conversation

gregns1 commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-review checklist

Dependencies (if applicable)

Integration Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

bbrks Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gregns1 commented Aug 7, 2025 •

edited

Loading