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

Fix bugs related to best-effort queries #3125

Merged
merged 8 commits into from
Mar 13, 2019
Merged

Conversation

manishrjain
Copy link
Contributor

@manishrjain manishrjain commented Mar 12, 2019

  • Don't use a pending txn cache when running best effort queries to avoid returning uncommitted data. If it gets used, a best-effort query ends up reading uncommitted data from another txn's cache who owns the start ts that best-effort query is "borrowing."

  • If no txns are going on in the system and we create new predicates by asking for them (case in v1.0 series), a best-effort query would block forever due to predicate checksum mismatch between OracleDelta and MembershipState. Fix that by sending out an empty OracleDelta from Zero after a tablet update.

  • Add tests in counter tool to check for these edge cases.


This change is Reviewable

@manishrjain manishrjain requested a review from martinmr March 12, 2019 21:39
Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

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

Just a question about the choice of int over bool but otherwise LGTM.

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manishrjain)


protos/pb.proto, line 72 at r1 (raw file):

	uint64 read_ts = 13;
	int32 cache = 14;

Why not a bool instead of an int. There's only two choices so far. Do you anticipate more choices in the future?

… to the system without doing any transactions. This causes the group checksum to mismatch and stay that way. The fix would push something to Oracle update channel so a new OracleDelta would be produced and sent out.
@manishrjain manishrjain changed the title Don't use txn cache in best effort queries Fix bugs related to best-effort queries Mar 13, 2019
@manishrjain manishrjain merged commit 6398f85 into master Mar 13, 2019
@manishrjain manishrjain deleted the mrjn/no-txn-cache branch March 13, 2019 02:50
manishrjain added a commit that referenced this pull request Mar 13, 2019
- Don't use a pending txn cache when running best effort queries to avoid returning uncommitted data. If it gets used, a best-effort query ends up reading uncommitted data from another txn's cache who owns the start ts that best-effort query is "borrowing."

- If no txns are going on in the system and we create new predicates by asking for them (case in v1.0 series), a best-effort query would block forever due to predicate checksum mismatch between `OracleDelta` and `MembershipState`. Fix that by sending out an empty `OracleDelta` from Zero after a tablet update.

- Add tests in counter tool to check for these edge cases.

Changes:

* Don't use a pending txn cache when running best effort queries to avoid returning uncommitted data.
* Fix a bug caused by best effort queries, where we add a new predicate to the system without doing any transactions. This causes the group checksum to mismatch and stay that way. The fix would push something to Oracle update channel so a new OracleDelta would be produced and sent out.
* Add tests to ensure that best effort queries work as expected.
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
- Don't use a pending txn cache when running best effort queries to avoid returning uncommitted data. If it gets used, a best-effort query ends up reading uncommitted data from another txn's cache who owns the start ts that best-effort query is "borrowing."

- If no txns are going on in the system and we create new predicates by asking for them (case in v1.0 series), a best-effort query would block forever due to predicate checksum mismatch between `OracleDelta` and `MembershipState`. Fix that by sending out an empty `OracleDelta` from Zero after a tablet update.

- Add tests in counter tool to check for these edge cases.

Changes:

* Don't use a pending txn cache when running best effort queries to avoid returning uncommitted data.
* Fix a bug caused by best effort queries, where we add a new predicate to the system without doing any transactions. This causes the group checksum to mismatch and stay that way. The fix would push something to Oracle update channel so a new OracleDelta would be produced and sent out.
* Add tests to ensure that best effort queries work as expected.
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