Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CBG-4331: legacy rev handling for version 4 replication protocol #7239

Merged
merged 5 commits into from
Dec 13, 2024
Merged

CBG-4331: legacy rev handling for version 4 replication protocol #7239

merged 5 commits into from
Dec 13, 2024
+474 −17

Conversation

gregns1
Copy link
Contributor

@gregns1 gregns1 commented Dec 11, 2024 •
edited
Loading

CBG-4331

  • Add check for vector rev in propose changes handling, if no vector present fall into legacy rev handling
  • When failing to parse hlv out of known revs AND we're in protocol version 4 then we can assume we have received a legacy rev, then i'm passing down boolean into sendRevision to build out the history for a legacy rev message
  • Handling for legacy rev messages in processRev when in protocol version 4.
  • Test coverage using blip tester, adding custom handlers when necessary to force a legacy rev response
  • CBG-4409 will be the ticket to add new BlipTesterClient tests for each test case in the document

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

@adamcfraser adamcfraser self-assigned this Dec 11, 2024
adamcfraser
adamcfraser requested changes Dec 12, 2024
View reviewed changes
Copy link
Collaborator

@adamcfraser adamcfraser left a comment

Choose a reason for hiding this comment

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

Changes look good - made a comment about one use case that I thought the tests could include.

rest/blip_legacy_revid_test.go Show resolved Hide resolved
@adamcfraser adamcfraser assigned gregns1 and unassigned adamcfraser Dec 12, 2024
@gregns1 gregns1 assigned adamcfraser and unassigned gregns1 Dec 12, 2024
adamcfraser
adamcfraser previously requested changes Dec 13, 2024
View reviewed changes
db/blip_sync_context.go Outdated Show resolved Hide resolved
@adamcfraser adamcfraser assigned gregns1 and unassigned adamcfraser Dec 13, 2024
@gregns1 gregns1 assigned bbrks and unassigned gregns1 Dec 13, 2024
@gregns1 gregns1 merged commit dcc98f1 into release/anemone Dec 13, 2024
36 checks passed
@gregns1 gregns1 deleted the CBG-4331 branch December 13, 2024 11:08
bbrks pushed a commit that referenced this pull request Dec 17, 2024
@gregns1 @bbrks
CBG-4331: legacy rev handling for version 4 replication protocol (#7239)
8c07c37 
* CBG-4331: legacy rev handling for version 4 replication protocol

* tidy up + fix for test flake

* update comment

* updated to address comments

* fix incorrect redaction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbrks bbrks bbrks approved these changes

@adamcfraser adamcfraser adamcfraser left review comments

Assignees

@bbrks bbrks

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gregns1 @bbrks @adamcfraser