Skip to content

Commit

Permalink
storage,libroach: Check for MaxKeys when reading from intent history
Browse files Browse the repository at this point in the history 
We weren't checking for MaxKeys (or TargetBytes) being reached
in the case where we read from intent history in the MVCC scanner.
All other cases go through addAndAdvance(), which had these checks.

Almost certainly fixes #46652. Would be very surprised if it was
something else.

Release note (bug fix): Fixes a bug where a read operation in a transaction
would error out for exceeding the maximum count of results returned.
  • Loading branch information
@itsbilal
itsbilal committed Apr 29, 2020
1 parent 26b9716 commit 3628ed9
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 0 deletions.
6 changes: 6 additions & 0 deletions c-deps/libroach/mvcc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,12 @@ template <bool reverse> class mvccScanner {
// sequence, read that value.
const bool found = getFromIntentHistory();
if (found) {
if (target_bytes_ > 0 && kvs_->NumBytes() >= target_bytes_) {
max_keys_ = kvs_->Count();
}
if (max_keys_ > 0 && kvs_->Count() == max_keys_) {
return false;
}
return advanceKey();
}
// 11. If no value in the intent history has a sequence number equal to
Expand Down
8 changes: 8 additions & 0 deletions pkg/storage/pebble_mvcc_scanner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,14 @@ func (p *pebbleMVCCScanner) getAndAdvance() bool {
// history that has a sequence number equal to or less than the read
// sequence, read that value.
if p.getFromIntentHistory() {
if p.targetBytes > 0 && p.results.bytes >= p.targetBytes {
// When the target bytes are met or exceeded, stop producing more
// keys. We implement this by reducing maxKeys to the current
// number of keys.
//
// TODO(bilal): see if this can be implemented more transparently.
p.maxKeys = p.results.count
}
if p.maxKeys > 0 && p.results.count == p.maxKeys {
return false
}
Expand Down
111 changes: 111 additions & 0 deletions pkg/storage/testdata/mvcc_histories/max_keys
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,114 @@ scan: "e" -> /BYTES/val-e @0.000000001,0
scan: "e" -> /BYTES/val-e @0.000000001,0
scan: "c" -> /BYTES/val-c @0.000000001,0
scan: "a" -> /BYTES/val-a @0.000000001,0

# Regression test for #46652: Test appropriate termination when the MaxKeys-th
# key is in the intent history.

run ok
with t=A ts=11,0 max=3
txn_begin
txn_step seq=10
put k=k v=a
put k=l v=a
put k=m v=a
put k=n v=a
txn_step seq=20
put k=k v=b
put k=l v=b
put k=m v=b
put k=n v=b
txn_step seq=30
put k=k v=c
put k=l v=c
put k=m v=c
put k=n v=c
txn_step seq=20
scan k=k end=o
scan k=k end=o reverse=true
----
scan: "k" -> /BYTES/b @0,0
scan: "l" -> /BYTES/b @0,0
scan: "m" -> /BYTES/b @0,0
scan: resume span ["n","o")
scan: "n" -> /BYTES/b @0,0
scan: "m" -> /BYTES/b @0,0
scan: "l" -> /BYTES/b @0,0
scan: resume span ["k","k\x00")
>> at end:
txn: "A" meta={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=20} lock=true stat=PENDING rts=0.000000011,0 wto=false max=0,0
data: "a"/0.000000001,0 -> /BYTES/val-a
data: "aa"/0.000000002,0 -> /<empty>
data: "aa"/0.000000001,0 -> /BYTES/val-aa
data: "c"/0.000000001,0 -> /BYTES/val-c
data: "e"/0.000000001,0 -> /BYTES/val-e
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "k"/0.000000011,0 -> /BYTES/c
meta: "l"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "l"/0.000000011,0 -> /BYTES/c
meta: "m"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "m"/0.000000011,0 -> /BYTES/c
meta: "n"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "n"/0.000000011,0 -> /BYTES/c

run ok
with t=A ts=11,0 max=3
txn_step seq=30
resolve_intent k=k status=COMMITTED
resolve_intent k=l status=COMMITTED
resolve_intent k=m status=COMMITTED
resolve_intent k=n status=COMMITTED
----
>> at end:
txn: "A" meta={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} lock=true stat=PENDING rts=0.000000011,0 wto=false max=0,0
data: "a"/0.000000001,0 -> /BYTES/val-a
data: "aa"/0.000000002,0 -> /<empty>
data: "aa"/0.000000001,0 -> /BYTES/val-aa
data: "c"/0.000000001,0 -> /BYTES/val-c
data: "e"/0.000000001,0 -> /BYTES/val-e
data: "k"/0.000000011,0 -> /BYTES/c
data: "l"/0.000000011,0 -> /BYTES/c
data: "m"/0.000000011,0 -> /BYTES/c
data: "n"/0.000000011,0 -> /BYTES/c

# Same case as above, except with a committed value at the key after MaxKeys.

run ok
with t=B ts=12,0 max=3
txn_begin
txn_step seq=10
put k=k v=a
put k=l v=a
put k=m v=a
txn_step seq=20
put k=k v=b
put k=l v=b
put k=m v=b
txn_step seq=30
put k=k v=c
put k=l v=c
put k=m v=c
txn_step seq=20
scan k=k end=o
----
scan: "k" -> /BYTES/b @0,0
scan: "l" -> /BYTES/b @0,0
scan: "m" -> /BYTES/b @0,0
scan: resume span ["n","o")
>> at end:
txn: "B" meta={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000012,0 min=0,0 seq=20} lock=true stat=PENDING rts=0.000000012,0 wto=false max=0,0
data: "a"/0.000000001,0 -> /BYTES/val-a
data: "aa"/0.000000002,0 -> /<empty>
data: "aa"/0.000000001,0 -> /BYTES/val-aa
data: "c"/0.000000001,0 -> /BYTES/val-c
data: "e"/0.000000001,0 -> /BYTES/val-e
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000012,0 min=0,0 seq=30} ts=0.000000012,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "k"/0.000000012,0 -> /BYTES/c
data: "k"/0.000000011,0 -> /BYTES/c
meta: "l"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000012,0 min=0,0 seq=30} ts=0.000000012,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "l"/0.000000012,0 -> /BYTES/c
data: "l"/0.000000011,0 -> /BYTES/c
meta: "m"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000012,0 min=0,0 seq=30} ts=0.000000012,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "m"/0.000000012,0 -> /BYTES/c
data: "m"/0.000000011,0 -> /BYTES/c
data: "n"/0.000000011,0 -> /BYTES/c
51 changes: 51 additions & 0 deletions pkg/storage/testdata/mvcc_histories/target_bytes
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,54 @@ scan: "c" -> /BYTES/ghijkllkjihg @0.000000123,45
scan: "aa" -> /<empty> @0.000000250,1
scan: "a" -> /BYTES/abcdef @0.000000123,45
scan: 98 bytes (target 65)

# Regression test for a bug simiar to #46652: Test appropriate termination when
# the TargetBytes-th kv pair is in the intent history.

run ok
with t=A ts=11,0 targetbytes=32
txn_begin
txn_step seq=10
put k=k v=a
put k=l v=a
put k=m v=a
put k=n v=a
txn_step seq=20
put k=k v=b
put k=l v=b
put k=m v=b
put k=n v=b
txn_step seq=30
put k=k v=c
put k=l v=c
put k=m v=c
put k=n v=c
txn_step seq=20
scan k=k end=o
scan k=k end=o reverse=true
----
scan: "k" -> /BYTES/b @0,0
scan: "l" -> /BYTES/b @0,0
scan: resume span ["m","o")
scan: 32 bytes (target 32)
scan: "n" -> /BYTES/b @0,0
scan: "m" -> /BYTES/b @0,0
scan: resume span ["k","l\x00")
scan: 32 bytes (target 32)
>> at end:
txn: "A" meta={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=20} lock=true stat=PENDING rts=0.000000011,0 wto=false max=0,0
data: "a"/0.000000123,45 -> /BYTES/abcdef
data: "a"/0.000000001,0 -> /BYTES/nevergoingtobeseen
data: "aa"/0.000000250,1 -> /<empty>
data: "aa"/0.000000001,0 -> /BYTES/willbetombstoned
data: "c"/0.000000123,45 -> /BYTES/ghijkllkjihg
data: "e"/0.000000123,45 -> /BYTES/mnopqr
data: "e"/0.000000001,0 -> /BYTES/sameasabove
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "k"/0.000000011,0 -> /BYTES/c
meta: "l"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "l"/0.000000011,0 -> /BYTES/c
meta: "m"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "m"/0.000000011,0 -> /BYTES/c
meta: "n"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "n"/0.000000011,0 -> /BYTES/c

0 comments on commit 3628ed9

Please sign in to comment.