Skip to content

Comments

Improves situation on LiteCore #437#3349

Merged
adamcfraser merged 2 commits intomasterfrom
feature/litecore_issue_437_unable_push_revs
Mar 2, 2018
Merged

Improves situation on LiteCore #437#3349
adamcfraser merged 2 commits intomasterfrom
feature/litecore_issue_437_unable_push_revs

Conversation

@tleyden
Copy link
Contributor

@tleyden tleyden commented Mar 2, 2018

After this Sync Gateway change, the situation for https://github.com/couchbase/couchbase-lite-core/issues/437 incrementally improved as described in: https://github.com/couchbase/couchbase-lite-core/issues/437#issuecomment-370039341

Before

  1. The rev tree branch that cblite considered a winner was determined as a winner was non-deterministic
  2. In the case where the shorter rev tree branch was chosen as the winning branch by cblite, all subsequent updates to the cblite doc failed to push, as they were rejected by the SG proposeChanges endpoint

After

  1. Both of those issues appear to be resolved
  2. Still some caveats mentioned in https://github.com/couchbase/couchbase-lite-core/issues/437#issuecomment-370045740 -- this probably requires more discussion to figure out how to handle these cases.


// Don't send conflicting rev tree branches, just send the winning rev tree branch leaf revision + history.
// Otherwise, Couchbase Lite will non-deterministically choose an arbitrary branch as the winner and
// tombstone the other branch. See LiteCore #437.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the non-deterministic branch choosing is really the reason we want to set sendConflicts=false. It's more correct to say here that CBL 2.0 (and blip_sync) doesn't support branched revision trees, and so we only want to send the winning revision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess the angle I was taking was to try to scare anyone who decided to flip that to true on a whim to fix some other bug. At least, let them know the consequences. But yeah, I agree that's not the primary motivation, so I'll reword that and leave the link to the issue for further backstory.

@adamcfraser adamcfraser merged commit a60c2d9 into master Mar 2, 2018
@adamcfraser adamcfraser removed the review label Mar 2, 2018
@adamcfraser adamcfraser deleted the feature/litecore_issue_437_unable_push_revs branch March 2, 2018 23:32
jamesnocentini pushed a commit that referenced this pull request Mar 23, 2018
* Improves situation on LiteCore #437

* Reword comment
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