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

Support disabling conflict detection #1344

Merged
merged 5 commits into from
Jun 3, 2020

Conversation

jarifibrahim
Copy link
Contributor

@jarifibrahim jarifibrahim commented May 26, 2020

This commit adds support for disabling conflict detection by setting the
option.DetectConflicts=false. When conflict detection is disabled
badger will not store information required to detect the conflict. This
reduces the amount of memory used by transactions running parallely and
also allows transactions to be processed at a faster rate.

fixes BADGER-207


This change is Reviewable

This commit adds support for disabling conflict detection by setting the
`option.DetectConflicts=false`. When conflict detection is disabled
badger will not store information required to detect the conflict. This
reduces the amount of memory used by transactions running parallely and
also allows transactions to be processed at a faster rate.
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.

:lgtm:

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)


txn.go, line 654 at r1 (raw file):

// tree won't be updated, so there's no need for any rollback.
func (txn *Txn) Commit() error {
	if len(txn.pendingWrites) == 0 {

why did this change?

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: A bunch of comments.

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @jarifibrahim, and @martinmr)


txn.go, line 391 at r1 (raw file):

	if txn.db.opt.DetectConflicts {
		fp := z.MemHash(e.Key) // Avoid dealing with byte arrays.
		txn.writes[fp] = struct{}{}

In Dgraph, we call them conflict keys or something. Maybe call it the same. Don't call it writes, then it's hard to read.


txn.go, line 654 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

why did this change?

Just add a comment. That writes might be empty if the conflict detection is disabled. Therefore, we should check the pending writes.

@jarifibrahim jarifibrahim merged commit 056d859 into master Jun 3, 2020
@jarifibrahim jarifibrahim deleted the ibrahim/conflict-detection-flag branch June 3, 2020 17:40
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
jarifibrahim pushed a commit that referenced this pull request Oct 2, 2020
This commit adds support for disabling conflict detection by setting the
option.DetectConflicts=false. When conflict detection is disabled
badger will not store information required to detect the conflict. This
reduces the amount of memory used by transactions running parallely and
also allows transactions to be processed at a faster rate.

fixes BADGER-207
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.

3 participants