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.

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

Bug fix: Send commit timestamps in order #2687

Merged
merged 7 commits into from
Oct 23, 2018
Merged

Conversation

manishrjain
Copy link
Contributor

@manishrjain manishrjain commented Oct 22, 2018

Zero had a race condition, where it could send commit timestamps out of order. This can cause invalid reads anyway. But, even more serious than that, Alpha was recently changed to ensure that it does not write over a newer version of the key (a4bd06f2f). If commits come out of order, some of these commits can be dropped on the floor.

This change fixes that by ensuring that Zero would send all the commits in the right order.


This change is Reviewable

dgraph/cmd/root.go Outdated Show resolved Hide resolved
@manishrjain manishrjain merged commit af08300 into master Oct 23, 2018
@manishrjain manishrjain deleted the mrjn/send-cts-in-order branch October 23, 2018 00:03
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
Zero had a race condition, where it could send commit timestamps out of order. This can cause invalid reads anyway. But, even more serious than that, Alpha was recently changed to ensure that it does not write over a newer version of the key (dgraph-io@a4bd06f2f). If commits come out of order, some of these commits can be dropped on the floor.

This change fixes that by ensuring that Zero would send all the commits in the right order.

P.S. Also did a change for @danielmai , which sets `stderrthreshold=0` always, and doesn't let a user overwrite it.

Changelog:

* Fix to ensure that Zero sends out all commit timestamps in order.
* Do the sorting of txn status in Alpha, because Alpha also batches multiple updates from Zero.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants