metamorphic test failure: inconsistent keys in range #3174
Comments
|
Reproduced on master (69994dd): |
|
Can you share the repro command? I tried the following, but it doesn't repro for me on Darwin or Linux, on $ CGO_ENABLED=0 go test -test.v -run TestMeta$ ./internal/metamorphic -seed 1703020073158492000 -keep |
|
Looks like it's |
|
Ah, you're right. This does it: |
|
Interestingly I got a different error with that same seed: |
|
@RaduBerinde is this with your change to surface the start bound by any chance? I'm guessing not, but still good to know. |
|
No, it is reproducible on master (see 2nd comment). I realized we are not runnign |
|
I wrote a little tool to reduce the number of operations to get a small reproduction. I got one with these ops: and these OPTIONS Error is: |
|
@RaduBerinde nice find! It looks like "boomerang files" which are files that get shared to another pebble, and then shared back to the same pebble, need to also have I think we'd want to set that to |
|
Thanks. Yes, that seems to fix the problem. I am however running in other problems related to table format versions being too low. I'm working on a change to require VirtualSSTables format to use shared objects. |
We weren't setting this flag correctly When a shared file is ingested by another store and then ingested back into the original store, leading to an "out of order keys" error in `TestMetaTwoInstances`. Informs cockroachdb#3174.
|
I posted PR #3179 for the "key out of order" issue. I can still repro a compare failure. I will extend the "reduce" tool to work on compare as well. |
We weren't setting this flag correctly When a shared file is ingested by another store and then ingested back into the original store, leading to an "out of order keys" error in `TestMetaTwoInstances`. Informs #3174.
|
I managed to reduce a repro to this: -iter25.SeekGE("lipbcfybquf@3", "") // [false] <nil> #7
+iter25.SeekGE("lipbcfybquf@3", "") // [true,"lipbcfybquf@3",<no point>,["hesu","njojeaszbsh")=>{"@6"="ybdmeuxousglnwebka"}*] <nil> #7
|
|
I simplified the keys; the repro is now: This is a visual of the operations: When running with these OPTIONS, we get: This is incorrect, as this portion of the RangeKeySet has been removed by the RangeKeyDel. The Replicate op involves sharing and virtualizing an SST file. The physical SST file contains these range keys (note, this is after seqnum rewriting): [ Question: the snapshot prevented the RANGEKEYSET from being removed when writing out the SST file, correct? ] The virtual raw range key iter contains: Tracking what our hefty iterator stack is doing is pretty hard, but it looks like the The snapshot is important for the reproduction. Without the snapshot, the SST contains nicely resolved fragments: Also, if we query the original It feels that the problem stems from the fact that we rewrite the sequence numbers to the same value, and that we have keys that are supposed to shadow other keys. With the new sequence numbers, the RangeKeyDelete does not shadow the RangeKeySet because they have the same sequence number. @itsbilal what do you think? Perhaps we never want to share SSTs that have unresolved keys because of snapshots? Also, what assertions are we missing? We should somehow check that shared SSTs have no shadowed keys? |
This change updates the virtual sstable range key iterator to hide obsolete range keys for shared ingested files. It also updates the sstable writer to set the obsolete key bit for range key spans that are underneath existing range key spans with a matching prefix and/or the one above is a del. This change also cleans up some dangling references to the older `isForeign` way of determining whether to hide obsolete keys, in lieu of the newer isSharedIngested approach to address boomerang shared files. Fixes cockroachdb#3174.
|
@RaduBerinde thank you for digging into this and for the clear and concise explanation of what's going on! As we discussed on Slack, the best approach at addressing this is to use the hideObsolete logic for block iters to hide away the keys that were only preserved for snapshots. I made #3187 which applies that fix. In your example the shadowed RANGEKEYSET would have its obsolete bit set, and when that sstable gets shared, the receiver will hide away the RANGEKEYSET before applying the sequence number substitution. I'll have to think more about what assertions we can add that'd harden this. It might be tricky but we can probably throw something into |
This change updates the virtual sstable range key iterator to hide obsolete range keys for shared ingested files. It also updates the sstable writer to set the obsolete key bit for range key spans that are underneath existing range key spans with a matching prefix and/or the one above is a del. This change also cleans up some dangling references to the older `isForeign` way of determining whether to hide obsolete keys, in lieu of the newer isSharedIngested approach to address boomerang shared files. Fixes cockroachdb#3174.
This change updates the virtual sstable range key iterator to hide obsolete range keys for shared ingested files. It does this by reusing the ForeignSSTTransformer that was removed in cockroachdb#2782. This change also cleans up some dangling references to the older `isForeign` way of determining whether to hide obsolete keys, in lieu of the newer isSharedIngested approach to address boomerang shared files. Fixes cockroachdb#3174.
This change updates the virtual sstable range key iterator to hide obsolete range keys for shared ingested files. It does this by reusing the ForeignSSTTransformer that was removed in cockroachdb#2782. This change also cleans up some dangling references to the older `isForeign` way of determining whether to hide obsolete keys, in lieu of the newer isSharedIngested approach to address boomerang shared files. Fixes cockroachdb#3174.
This change updates the virtual sstable range key iterator to hide obsolete range keys for shared ingested files. It does this by reusing the ForeignSSTTransformer that was removed in cockroachdb#2782. This change also cleans up some dangling references to the older `isForeign` way of determining whether to hide obsolete keys, in lieu of the newer isSharedIngested approach to address boomerang shared files. Fixes cockroachdb#3174.
This change updates the virtual sstable range key iterator to hide obsolete range keys for shared ingested files. It does this by reusing the ForeignSSTTransformer that was removed in cockroachdb#2782. This change also cleans up some dangling references to the older `isForeign` way of determining whether to hide obsolete keys, in lieu of the newer isSharedIngested approach to address boomerang shared files. Fixes cockroachdb#3174.
This change updates the virtual sstable range key iterator to hide obsolete range keys for shared ingested files. It does this by reusing the ForeignSSTTransformer that was removed in #2782. This change also cleans up some dangling references to the older `isForeign` way of determining whether to hide obsolete keys, in lieu of the newer isSharedIngested approach to address boomerang shared files. Fixes #3174.
CI ran into a metamorphic test failure in PR #3173. It's unlikely (but not impossible) that the PR itself introduces the problem (it's mostly code cleanup).
https://github.com/cockroachdb/pebble/actions/runs/7266582327/job/19798611718?pr=3173
The text was updated successfully, but these errors were encountered: