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.

Sign up for GitHub

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

Jump to bottom

kv/kvserver: TestStoreReadInconsistent fails under race on Pebble #48461

Closed
petermattis opened this issue May 6, 2020 · 1 comment · Fixed by #48490
Closed

kv/kvserver: TestStoreReadInconsistent fails under race on Pebble #48461

petermattis opened this issue May 6, 2020 · 1 comment · Fixed by #48490
Assignees
@petermattis
Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-test-failure Broken test (automatically or manually discovered).

Comments

@petermattis
Copy link
Collaborator

Saw this on CI, then reproduced on #48145 outside of CI:

~ make testrace PKG=./kv/kvserver TESTS=TestStoreReadInconsistent 2>&1 | tee out
Running make with -j16
GOPATH set to /Users/pmattis/Development/go
mkdir -p lib
ln -sf /Users/pmattis/Development/go/native/x86_64-apple-darwin19.3.0/geos/lib/lib{geos,geos_c}.dylib lib
GOFLAGS= go test  -race -tags ' make x86_64_apple_darwin19.3.0' -ldflags '-X github.com/cockroachdb/cockroach/pkg/build.typ=development -extldflags "" -X "github.com/cockroachdb/cockroach/pkg/build.tag=v20.1.0-beta.4-1295-gc0da218d4c" -X "github.com/cockroachdb/cockroach/pkg/build.rev=c0da218d4c38af6a4aa6eb063506792a7ea02bd0" -X "github.com/cockroachdb/cockroach/pkg/build.cgoTargetTriple=x86_64-apple-darwin19.3.0"  ' -run "TestStoreReadInconsistent"  -timeout 30m ./pkg/./kv/kvserver
I200505 21:12:30.495986 1 rand.go:85  Random seed: -1185532325996819864
I200505 21:12:30.502612 14 vendor/github.com/cockroachdb/pebble/version_set.go:140  [JOB 1] MANIFEST created 000001
I200505 21:12:30.502767 14 vendor/github.com/cockroachdb/pebble/open.go:270  [JOB 1] WAL created 000002
W200505 21:12:30.572287 46 kv/kvserver/intentresolver/intent_resolver.go:440  failed to push during intent resolution: failed to push meta={id=92f3646b key="true-a" pri=0.00000005 epo=0 ts=0.000000123,106 min=0.000000123,106 seq=1} lock=true stat=PENDING rts=0,0 wto=false max=0,0
panic: interface conversion: roachpb.Response is nil, not *roachpb.ReverseScanResponse [recovered]
	panic: interface conversion: roachpb.Response is nil, not *roachpb.ReverseScanResponse [recovered]
	panic: interface conversion: roachpb.Response is nil, not *roachpb.ReverseScanResponse [recovered]
	panic: interface conversion: roachpb.Response is nil, not *roachpb.ReverseScanResponse

goroutine 14 [running]:
testing.tRunner.func1(0xc000222700)
	/Users/pmattis/Development/go1.13/src/testing/testing.go:874 +0x69f
panic(0x8e71580, 0xc000a39320)
	/Users/pmattis/Development/go1.13/src/runtime/panic.go:679 +0x1b2
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).Recover(0xc000702000, 0xa412f00, 0xc000132010)
	/Users/pmattis/Development/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:183 +0x154
panic(0x8e71580, 0xc000a39320)
	/Users/pmattis/Development/go1.13/src/runtime/panic.go:679 +0x1b2
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).Stop(0xc000702000, 0xa412f00, 0xc000132010)
	/Users/pmattis/Development/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:479 +0x74d
panic(0x8e71580, 0xc000a39320)
	/Users/pmattis/Development/go1.13/src/runtime/panic.go:679 +0x1b2
github.com/cockroachdb/cockroach/pkg/kv/kvserver.TestStoreReadInconsistent.func1(0xc000222700)
	/Users/pmattis/Development/go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/store_test.go:2073 +0x4fd7
testing.tRunner(0xc000222700, 0xc000314450)
	/Users/pmattis/Development/go1.13/src/testing/testing.go:909 +0x19a
created by testing.(*T).Run
	/Users/pmattis/Development/go1.13/src/testing/testing.go:960 +0x652
FAIL	github.com/cockroachdb/cockroach/pkg/kv/kvserver	4.417s
FAIL

I'm suspecting a difference in Pebble's MVCC layer.

Cc @itsbilal in case you happen to immediately know what the problem is. Otherwise, I'll dig into this tomorrow.
@petermattis petermattis added C-test-failure Broken test (automatically or manually discovered). A-storage Relating to our storage engine (Pebble) on-disk storage. labels May 6, 2020
@petermattis petermattis self-assigned this May 6, 2020
@petermattis
Copy link
Collaborator Author

A little more debugging info shows this is the same problem as #48463:

cannot read undeclared span /M{in-ax}

petermattis added a commit to petermattis/cockroach that referenced this issue May 6, 2020
@petermattis
kv/kvserver,storage: fix spanset assertions when running on Pebble
d6ad488 
Fix spanset assertions when running race-enabled tests on Pebble. In
race-enabled tests, `pebbleMVCCScanner` runs on a `spanset.Iterator`
which asserts that the keys used are within the spans declared by the KV
op. `pebbleMVCCScanner.scan()` was violating these assertions at the
scan boundaries due to the usage of `MVCCKey{}` (a.k.a. min-key) and
`keys.KeyMax`.

Fix `spanset.Iterator.{Next,Prev,NextKey}` to only check whether the key
is allowed if the underlying iterator is valid. This doesn't appear to
have been a problem in practice, but the previous code was definitely
incorrect.

Fix `TestReplicaLaziness` to not scan from
`[keys.KeyMin,keys.KeyMax]`. That's not a valid thing to do and runs
afoul of the spanset assertions when running on Pebble.

Fixes cockroachdb#48461
Fixes cockroachdb#48460

Release note: None
@petermattis petermattis mentioned this issue May 6, 2020
Merged
craig bot pushed a commit that referenced this issue May 6, 2020
@yuzefovich @petermattis
Merge #48357 #48490
2cc13d4 
48357: colexec: add comprehensive benchmark of projection operators r=yuzefovich a=yuzefovich

This commit updates the benchmark of projection operators to run all
interesting combinations of type pairs and binary or comparison
operators. It also extracts the arguments of `RandomVec` test utility
function into a struct and adds a few knobs to that struct as well.

Release note: None

48490: kv/kvserver,storage: fix spanset assertions when running on Pebble r=petermattis a=petermattis

Fix spanset assertions when running race-enabled tests on Pebble. In
race-enabled tests, `pebbleMVCCScanner` runs on a `spanset.Iterator`
which asserts that the keys used are within the spans declared by the KV
op. `pebbleMVCCScanner.scan()` was violating these assertions at the
scan boundaries due to the usage of `MVCCKey{}` (a.k.a. min-key) and
`keys.KeyMax`.

Fix `spanset.Iterator.{Next,Prev,NextKey}` to only check whether the key
is allowed if the underlying iterator is valid. This doesn't appear to
have been a problem in practice, but the previous code was definitely
incorrect.

Fix `TestReplicaLaziness` to not scan from
`[keys.KeyMin,keys.KeyMax]`. That's not a valid thing to do and runs
afoul of the spanset assertions when running on Pebble.

Fixes #48461
Fixes #48460

Release note: None

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Peter Mattis <petermattis@gmail.com>
@craig craig bot closed this as completed in 024b9d5 May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@petermattis petermattis

Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-test-failure Broken test (automatically or manually discovered).
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kv/kvserver,storage: fix spanset assertions when running on Pebble petermattis/cockroach
1 participant
@petermattis