Skip to content

Commit

Permalink
fix: Fix conflict detection for managed DB (#1716)
Browse files Browse the repository at this point in the history
I propose this simple fix for detecting conflicts in managed mode. Addresses https://discuss.dgraph.io/t/fatal-error-when-writing-conflicting-keys-in-managed-mode/14784.

When a write conflict exists for a managed DB, an internal assert can fail.
This occurs because a detected conflict is indicated with commitTs of 0, but handling the error is skipped for managed DB instances.

Rather than conflate conflict detection with a timestamp of 0, it can be indicated with another return value from hasConflict.

(cherry picked from commit 3279e18)
  • Loading branch information
roysc authored and NamanJain8 committed Jul 8, 2021
1 parent 3a47ec2 commit 641d35d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
12 changes: 5 additions & 7 deletions txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,12 @@ func (o *oracle) hasConflict(txn *Txn) bool {
return false
}

func (o *oracle) newCommitTs(txn *Txn) uint64 {
func (o *oracle) newCommitTs(txn *Txn) (uint64, bool) {
o.Lock()
defer o.Unlock()

if o.hasConflict(txn) {
return 0
return 0, true
}

var ts uint64
Expand Down Expand Up @@ -194,7 +194,7 @@ func (o *oracle) newCommitTs(txn *Txn) uint64 {
})
}

return ts
return ts, false
}

func (o *oracle) doneRead(txn *Txn) {
Expand Down Expand Up @@ -537,10 +537,8 @@ func (txn *Txn) commitAndSend() (func() error, error) {
orc.writeChLock.Lock()
defer orc.writeChLock.Unlock()

commitTs := orc.newCommitTs(txn)
// The commitTs can be zero if the transaction is running in managed mode.
// Individual entries might have their own timestamps.
if commitTs == 0 && !txn.db.opt.managedTxns {
commitTs, conflict := orc.newCommitTs(txn)
if conflict {
return nil, ErrConflict
}

Expand Down
9 changes: 9 additions & 0 deletions txn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,15 @@ func TestManagedDB(t *testing.T) {
}
}
txn.Discard()

// Write data to same key, causing a conflict
txn = db.NewTransactionAt(10, true)
txnb := db.NewTransactionAt(10, true)
txnb.Get(key(0))
require.NoError(t, txn.SetEntry(NewEntry(key(0), val(0))))
require.NoError(t, txnb.SetEntry(NewEntry(key(0), val(1))))
require.NoError(t, txn.CommitAt(11, nil))
require.Equal(t, ErrConflict, txnb.CommitAt(11, nil))
}
t.Run("disk mode", func(t *testing.T) {
db, err := Open(opt)
Expand Down

0 comments on commit 641d35d

Please sign in to comment.