Skip to content

Commit

Permalink
Revert "Set readTs in write batch to latest done readTs so that we ca…
Browse files Browse the repository at this point in the history
…n discard older versions of keys during compactions."

This reverts commit 6b796b3.

Note:

We're seeing an Assert failure due to this change in Dgraph.

```
zero1     | 2019/05/03 23:06:22 Name: badger.PendingReads doneUntil: 4. Index: 2
zero1     | github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/badger/y.AssertTruef
zero1     | 	/home/mrjn/go/src/github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/badger/y/error.go:62
zero1     | github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/badger/y.(*WaterMark).process.func1
zero1     | 	/home/mrjn/go/src/github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/badger/y/watermark.go:175
zero1     | github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/badger/y.(*WaterMark).process
zero1     | 	/home/mrjn/go/src/github.com/dgraph-io/dgraph/vendor/github.com/dgraph-io/badger/y/watermark.go:225
zero1     | runtime.goexit
zero1     | 	/usr/lib/go-1.10/src/runtime/asm_amd64.s:2361
```
  • Loading branch information
manishrjain committed May 3, 2019
1 parent e0748f5 commit d98dd68
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 68 deletions.
12 changes: 2 additions & 10 deletions batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,7 @@ type WriteBatch struct {
// creating and committing transactions. Due to the nature of SSI guaratees provided by Badger,
// blind writes can never encounter transaction conflicts (ErrConflict).
func (db *DB) NewWriteBatch() *WriteBatch {
txn := db.newTransaction(true, true)
// If we let it stay at zero, compactions would not allow older key versions to be deleted,
// because the read timestamps of pending txns, would be zero. Therefore, we set it to the
// maximum read timestamp that's done. This allows compactions to discard key versions below
// this read timestamp, while also not blocking on pending txns to finish before starting this
// one.
txn.readTs = db.orc.readMark.DoneUntil()
return &WriteBatch{db: db, txn: txn}
return &WriteBatch{db: db, txn: db.newTransaction(true, true)}
}

// Cancel function must be called if there's a chance that Flush might not get
Expand Down Expand Up @@ -135,8 +128,7 @@ func (wb *WriteBatch) commit() error {
wb.wg.Add(1)
wb.txn.CommitWith(wb.callback)
wb.txn = wb.db.newTransaction(true, true)
// See comment about readTs in NewWriteBatch.
wb.txn.readTs = wb.db.orc.readMark.DoneUntil()
wb.txn.readTs = 0 // We're not reading anything.
return wb.err
}

Expand Down
58 changes: 0 additions & 58 deletions batch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package badger

import (
"fmt"
"io/ioutil"
"os"
"testing"
"time"

Expand Down Expand Up @@ -69,59 +67,3 @@ func TestWriteBatch(t *testing.T) {
require.NoError(t, err)
})
}

func TestWriteBatchCompaction(t *testing.T) {
dir, err := ioutil.TempDir(".", "badger-test")
require.NoError(t, err)
defer os.RemoveAll(dir)

opts := DefaultOptions
opts.ValueDir = dir
opts.Dir = dir

db, err := Open(opts)
require.NoError(t, err)

wb := db.NewWriteBatch()
entries := 10000
for i := 0; i < entries; i++ {
require.Nil(t, wb.Set([]byte(fmt.Sprintf("foo%d", i)), []byte("bar"), 0))
}
require.Nil(t, wb.Flush())

wb = db.NewWriteBatch()
// Delete 50% of the entries
for i := 0; i < entries/2; i++ {
require.Nil(t, wb.Delete([]byte(fmt.Sprintf("foo%d", i))))
}
require.Nil(t, wb.Flush())

// It is necessary that we call db.Update(..) before compaction so that the db.orc.readMark
// value is incremented. The transactions in WriteBatch call do not increment the
// db.orc.readMark value and hence compaction wouldn't discard any entries added by write batch
// if we do not increment the db.orc.readMark value
require.Nil(t, db.Update(func(txn *Txn) error {
txn.Set([]byte("key1"), []byte("val1"))
return nil
}))

// Close DB to force compaction
require.Nil(t, db.Close())

db, err = Open(opts)
require.NoError(t, err)
defer db.Close()

iopt := DefaultIteratorOptions
iopt.AllVersions = true
txn := db.NewTransaction(false)
defer txn.Discard()
it := txn.NewIterator(iopt)
defer it.Close()
countAfterCompaction := 0
for it.Rewind(); it.Valid(); it.Next() {
countAfterCompaction++
}
// We have deleted 50% of the keys
require.Less(t, countAfterCompaction, entries+entries/2)
}

0 comments on commit d98dd68

Please sign in to comment.