I checked and I’m pretty sure we do, but it doesn’t apply any liveness optimisation. I had misunderstood the optimisation you proposed. Ideally we would encode any non-live timestamp with the delta offset, but since that’s a distinct optimisation perhaps that can be left to another patch.

Are we happy, though, that the two different deletion time serialisers can store different ranges of timestamp? Both are large ranges, but I am not 100% comfortable with them diverging.

On 3 Jul 2023, at 05:45, Berenguer Blasi <berenguerbl...@gmail.com> wrote:



It can look into it. I don't have a deep knowledge of the sstable format hence why I wanted to document it someday. But DeletionTime is being serialized in other places as well iirc and I doubt (finger in the air) we'll have that Epoch handy.

On 29/6/23 17:22, Benedict wrote:
So I’m just taking a quick peek at SerializationHeader and we already have a method for reading and writing a deletion time with offsets from EncodingStats.

So perhaps we simply have a bug where we are using DeletionTime Serializer instead of SerializationHeader.writeLocalDeletionTime? It looks to me like this is already available at most (perhaps all) of the relevant call sites.

 

On 29 Jun 2023, at 15:53, Josh McKenzie <jmcken...@apache.org> wrote:


I would prefer we not plan on two distinct changes to this
I agree with this sentiment, and

+1, if you have time for this approach and no other in this window.
People are going to use 5.0 for awhile. Better to have an improvement in their hands for that duration than no improvement at all IMO. Justifies the cost of the double implementation and transitions to me.

On Tue, Jun 27, 2023, at 5:43 AM, Mick Semb Wever wrote:

Just for completeness the change is a handful loc. The rest is added tests and we'd loose the sstable format change opportunity window.



+1, if you have time for this approach and no other in this window.

(If you have time for the other, or someone else does, then the technically superior approach should win)



Reply via email to