Iterator: Always add key to txn.reads #1328
Merged
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
added 3 commits
May 13, 2020 20:44
jarifibrahim
requested review from
ashish-goswami,
manishrjain and
a team
as code owners
added 2 commits
May 14, 2020 17:48
manishrjain
reviewed
May 14, 2020
There was a problem hiding this comment.
Be very careful.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @ashish-goswami and @manishrjain)
|
I have tested your fix from branch Output from test runs; So things look good to me now thank you. |
added 3 commits
May 18, 2020 18:32
manishrjain
approved these changes
May 21, 2020
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @jarifibrahim)
txn.go, line 488 at r2 (raw file):
func (txn *Txn) addReadKey(key []byte) { if txn.update { fp := z.MemHash(key)
avoid changes.
jarifibrahim
commented
May 22, 2020
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)
txn.go, line 488 at r2 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
avoid changes.
Done.
jarifibrahim
pushed a commit
that referenced
this pull request
Jul 8, 2020
This PR adds the following changes from **Master branch to the dgraph-maintenance branch**. ``` 25fd0ef Update ristretto to commit f66de99 (#1391) fac972f Update head while replaying value log (#1372) e5fd05a Rework DB.DropPrefix (#1381) adeb842 Fix assert in background compression and encryption. (#1366) ``` The **master and dgraph-maintenace branch** have following difference `+` (in green) means commit missing on dgraph-maintenance `-` (in red) means the commit exists. ```diff + 079f5ae DefaultOptions: Set KeepL0InMemory to false (#1345) + 7e19cac Add immudb to the project list (#1341) + da80eb9 Iterator: Always add key to txn.reads (#1328) + a7e239e StreamWriter: Close head writer (#1347) + 543f353 Fix build on golang tip (#1355) + fd89894 Compaction: Expired keys and delete markers are never purged (#1354) + 056d859 Support disabling conflict detection (#1344) + b762832 Tests: Do not leave behind state goroutines (#1349) + b2267c2 Restore: Account for value size as well (#1358) + 14386ac GC: Consider size of value while rewriting (#1357) + c45d966 Fix assert in background compression and encryption. (#1366) + dd332b0 Avoid panic in filltables() (#1365) + 3f4761d Force KeepL0InMemory to be true when InMemory is true (#1375) + d37ce36 Tests: Use t.Parallel in TestIteratePrefix tests (#1377) + 158d927 Remove second initialization of writech in Open (#1382) + 675efcd Increase default valueThreshold from 32B to 1KB (#1346) + 3042e37 pre allocate cache key for the block cache and the bloom filter cache (#1371) + e013bfd Rework DB.DropPrefix (#1381) + 509de73 Update head while replaying value log (#1372) + 09dfa66 Update ristretto to commit f66de99 (#1391) + 717b89c Enable cross-compiled 32bit tests on TravisCI (#1392) ``` <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/dgraph-io/badger/1398) <!-- Reviewable:end -->
jarifibrahim
pushed a commit
that referenced
this pull request
Oct 2, 2020
The SSI guarantees provided by badger are respected when multiple concurrent transactions try to update the same key at once using the txn.Get and txn.Set API ``` // txn.Get // if not found; txn.Set and Commit ``` but the same guarantees fail when an `iterator` is used instead of the `txn.Get` API ``` // itr.Seek(key) // if not itr.Valid(); txn.Set and Commit ``` If the code above is executed via multiple concurrent transactions, we might overwrite the first update. The test `TestConflict` added in this PR fails on master but works fine on this PR. Fixes #1289
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.
The SSI guarantees provided by badger are respected when multiple concurrent transactions try to update the same key at once using the txn.Get and txn.Set API
but the same guarantees fail when an
iteratoris used instead of thetxn.GetAPIIf the code about is executed via multiple concurrent transactions, we might lose some updates.
The test
TestConflictadded in this PR fails on master but works fine on this PR.Fixes #1289
This change is