Cockroach: storage: have to copy GC thresholds to RHS on split

[tbg]

We don't appear to be seeding the new RHS of a split with the GCThreshold and TxnSpanGCThreshold of the LHS. This means that queries which are disallowed on the LHS can run after a split on the RHS, returning incomplete data.

You'd have to run queries close to the TTL (~24h by default) so it's not extremely likely for folks to run into this, but it's a serious bug nonetheless.

In addressing this issue, we should also do comprehensive testing.

[a6802739]

@tschottdorf I want to have a see on it, could I?

[tbg]

Yes, should be a fairly small code change. Make sure there's a test! We had a similar issue before when we didn't copy the LHS lease to the RHS, perhaps that test can easily be extended to also check GCThresholds. (we're talking about GC Threshold and TxnSpanGCThreshold here).

[a6802739]

@tschottdorf where is the test for lease after split?

[tbg]

@a6802739 that's TestStoreSplitTimestampCacheDifferentLeaseHolder.

[a6802739]

@tschottdorf about You'd have to run queries close to the TTL, you mean snapshot read？

[tbg]

yes, by default these would be transactions at old snapshot timestamps (for example one coming from SELECT ... AS OF SYSTEM TIME now-24h, but if someone configured a very small TTL (say 1 second) then any transaction running ~1s could run into problems with this anomaly.

[a6802739]

@tschottdorf the default TTL is about 24h. So if we write a key-value with a timestamp now-24h.prev() , after the GC, if we use SELECT ... AS OF SYSTEM TIME now-24h, we would not read the key with the timestamp now-24h.prev(), right?

And If I start a TestCluster, should I use what function to force the GC runs?

[tbg]

The GCThreshold prevents you from doing reads that would read incorrect results. And the current problem is that we don't copy the GCThreshold from the left hand side to the newly created right hand side of the split. That needs to be fixed and tested.

I looked again at TestStoreSplitTimestampCacheDifferentLeaseHolder and it's more complicated than what you want to test. I think you can just make a new test that sets a GCThreshold on a range (by sending a GCRequest to it), and then makes it split, and then reads the GCThreshold of the right hand side.