CBG-4365 rosmar xdcr, use _mou.cas for conflict resolution #7206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gregns1
reviewed
Nov 22, 2024
xdcr/rosmar_xdcr.go
Outdated
| @@ -373,3 +388,21 @@ func updateHLV(xattrs map[string][]byte, sourceHLV *db.HybridLogicalVector, sour | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| // getConflictResolutionCas returns cas for conflict resolution. If thethe conflict resolution cas. | |||
There was a problem hiding this comment.
Typo here? I think it would be good to include on the comment the expected interaction between actual cas and mou cas, something like the below:
If we have:
- the _mou.cas == actualCas we will return _vv.cvCAS for conflict resolution,
- if mou.Cas is out of date we will use _vv.ver
- if mou.Cas is not out of date but there is no HLV we return 0 to match XDCR
adamcfraser
requested changes
Nov 25, 2024
xdcr/rosmar_xdcr.go
Outdated
| @@ -342,21 +350,24 @@ func getHLVAndMou(xattrs map[string][]byte) (*db.HybridLogicalVector, *db.Metada | |||
| return hlv, mou, nil | |||
| } | |||
|
|
|||
| func updateHLV(xattrs map[string][]byte, sourceHLV *db.HybridLogicalVector, sourceMou *db.MetadataOnlyUpdate, sourceID string, sourceCas uint64) error { | |||
| func updateHLV(xattrs map[string][]byte, sourceHLV *db.HybridLogicalVector, sourceMou *db.MetadataOnlyUpdate, sourceID string, copiedCas uint64) error { | |||
There was a problem hiding this comment.
I'm not sure what 'copiedCas' implies here (as opposed to source cas) - maybe a comment to explain?
adamcfraser
approved these changes
Nov 25, 2024
adamcfraser
pushed a commit
that referenced
this pull request
Nov 29, 2024
bbrks
pushed a commit
that referenced
this pull request
Dec 2, 2024
bbrks
pushed a commit
that referenced
this pull request
Dec 5, 2024
bbrks
pushed a commit
that referenced
this pull request
Dec 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CBG-4365 rosmar xdcr, use _mou.cas for conflict resolution
I ran topologytests without a skip on
TestHLVUpdateDocumentSingleActorand they now intermittently pass, rather than consistently fail without this patch.The guts of this ticket are to consider the cas for conflict resolution in xdcr as actual cas, unless
_mou.cas == doc cas, in which case, using_hlv.cvCasis correct. This matches the behavior of server XDCR.MetadataOnlyUpdateis one of the only datastructures that we have that stores cas as a string, and that's fine for serialization but problematic for comparison. I decided to write method to convert the data rather than writing yet more comparisons in xdcr code. I don't think it makes sense to change the data on marshal and unmarshal due to the use of string comparison incomputeMetadataOnlyUpdatebut maybe having it convert all the values to hex on marshal/unmarshal would make more sense.Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiIntegration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/2810/