-
Notifications
You must be signed in to change notification settings - Fork 471
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.
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
github.com/cockroachdb/pebble/internal/metamorphic: TestMeta failed #3906
Comments
github.com/cockroachdb/pebble/internal/metamorphic.TestMeta failed with artifacts on refs/heads/master @ 2801a3eab3d8:
HelpTo reproduce, try: go test -tags 'invariants' -exec 'stress -p 1' -timeout 0 -test.v -run 'TestMeta$' ./internal/metamorphic -seed 1725089402356366123 -ops "uniform:5000-10000" |
github.com/cockroachdb/pebble/internal/metamorphic.TestMeta failed with artifacts on refs/heads/master @ 376d455bf0f7:
HelpTo reproduce, try: go test -tags 'invariants' -exec 'stress -p 1' -timeout 0 -test.v -run 'TestMeta$' ./internal/metamorphic -seed 1725861370934242848 -ops "uniform:5000-10000" |
@RaduBerinde have you gotten a chance to triage this? I can start to take a look if not |
I haven't, sorry |
Looks like fallout from making the comparer more strict. |
The initial failure was different: |
github.com/cockroachdb/pebble/internal/metamorphic.TestMeta failed with artifacts on refs/heads/master @ 0665a3e18dd5:
HelpTo reproduce, try: go test -tags 'invariants' -exec 'stress -p 1' -timeout 0 -test.v -run 'TestMeta$' ./internal/metamorphic -seed 1726212723511935732 -ops "uniform:5000-10000" |
It's okay to call Split on a bare prefix but not a bare suffix. Informs cockroachdb#3906.
It's okay to call Split on a bare prefix but not a bare suffix. Informs #3906.
The comparer assertion failure looks like this is a consequence of the synthetic prefix. We strip the synthetic prefix from the search key, leaving just the suffix and then attempt to compare the block's KVs to the seek key that is now just a bare suffix. I'm also noticing that this documented requirement:
doesn't seem to be holding in the metamorphic tests, as we're applying the synthetic prefix to a table with prefixed-keys. Looking at this again, this doesn't seem like it could possibly ever hold? Ah I'm misreading "prefix-less". We don't mean a |
This is a bit tangential, but I don't think the columnar block format currently allows a sstable of prefix-less keys. The PrefixBytes encoding uses an empty slice to indicate a duplicate key, assuming that an empty slice is not valid. Edit: nevermind, we can take advantage of the fact that prefixes are required to be lexicographically ordered, so if there's an empty slice it necessarily must be either the first key in the block or a duplicate of the first key in the block: #3950 |
I think "prefix-less" in that comment is not phrased properly. Keys should not have empty prefixes, even when we have a synthetic prerix to prepend. We prpbably need to fix the meta test. |
github.com/cockroachdb/pebble/internal/metamorphic.TestMeta failed with artifacts on refs/heads/master @ aba5868b355a:
HelpTo reproduce, try: go test -tags 'invariants' -exec 'stress -p 1' -timeout 0 -test.v -run 'TestMeta$' ./internal/metamorphic -seed 1726818164249947891 -ops "uniform:5000-10000" |
@RaduBerinde If the user provides a seek key that's the beginning of an index's keyspace, we can end up calling |
When synthetic prefix is used, we can trim the synthetic prefix from a given seek key and compare it with the "bare" key in a table. After trimming the key prefix can become "empty" (with just the terminator left). This change extends the comparer test suite to check that we can remove leading bytes from prefixes and adjusts the testkeys and crdb comparers. Fixes cockroachdb#3906
github.com/cockroachdb/pebble/internal/metamorphic.TestMeta failed with artifacts on refs/heads/master @ 5294f89b6da3:
Help
To reproduce, try:
This test on roachdash | Improve this report!
Jira issue: PEBBLE-254
The text was updated successfully, but these errors were encountered: