Skip to content

Commit

Permalink
storage: prevent reuse of time bounded iterators
Browse files Browse the repository at this point in the history
`(*rocksDBBatch).NewTimeboundedIterator` was caching the iterator in a
way that would return the iterator in future calls to
`(*rocksDBBatch).NewIterator`, which was a correctness problem.

Luckily, the fallout was somewhat limited by the fact that we only use
time bounded iterators when resolving an intent, and that is usually
(but not always) the last operation carried out on the batch.

This was introduced in #21078, which landed around Jan 15 2018, so
the bug has not made it into a release yet, though we'll have to bump
our alpha (#21657).

Release note: None

Fixes #21689.
  • Loading branch information
tbg committed Jan 23, 2018
1 parent 2f71c12 commit 61dd154
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions pkg/storage/engine/rocksdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -1559,17 +1559,14 @@ func (r *rocksDBBatch) NewTimeBoundIterator(start, end hlc.Timestamp) Iterator {
if r.distinctOpen {
panic("distinct batch open")
}
// Used the cached iterator, creating it on first access.
iter := &r.normalIter
if iter.iter.iter == nil {
r.ensureBatch()
iter.iter.initTimeBound(r.batch, start, end, r)
}
if iter.batch != nil {
panic("iterator already in use")
}

// Don't cache these iterators; we're unlikely to get many calls for the same
// time bounds.
r.ensureBatch()
var iter rocksDBBatchIterator
iter.iter.initTimeBound(r.batch, start, end, r)
iter.batch = r
return iter
return &iter
}

func (r *rocksDBBatch) Commit(syncCommit bool) error {
Expand Down

0 comments on commit 61dd154

Please sign in to comment.