Hello Richard, Thanks for the KIP, I just have one clarification regarding "So the idea is to set the base timestamp to the delete horizon and adjust the deltas accordingly." My understanding is that during compaction, for each compacted new segment, we would set its base offset of each batch as the delete horizon, which is the "current system time that cleaner has seen so far", and adjust the delta timestamps of each of the inner records of the batch (and practically the deltas will be all negative)?
If that's case, could we do some back of the envelope calculation on what's the possible smallest case of deltas? Note that since we use varInt for delta values for each record, the smaller the negative delta, that would take more bytes to encode. Guozhang On Wed, Oct 16, 2019 at 6:48 PM Jason Gustafson <ja...@confluent.io> wrote: > +1. Thanks Richard. > > On Wed, Oct 16, 2019 at 10:04 AM Richard Yu <yohan.richard...@gmail.com> > wrote: > > > Hi all, > > > > Want to try to get this KIP wrapped up. So it would be great if we can > get > > some votes. > > > > Cheers, > > Richard > > > > On Tue, Oct 15, 2019 at 12:58 PM Jun Rao <j...@confluent.io> wrote: > > > > > Hi, Richard, > > > > > > Thanks for the updated KIP. +1 from me. > > > > > > Jun > > > > > > On Tue, Oct 15, 2019 at 12:46 PM Richard Yu < > yohan.richard...@gmail.com> > > > wrote: > > > > > > > Hi Jun, > > > > > > > > I've updated the link accordingly. :) > > > > Here is the updated KIP link: > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-534%3A+Retain+tombstones+and+transaction+markers+for+approximately+delete.retention.ms+milliseconds > > > > > > > > Cheers, > > > > Richard > > > > > > > > On Mon, Oct 14, 2019 at 5:12 PM Jun Rao <j...@confluent.io> wrote: > > > > > > > > > Hi, Richard, > > > > > > > > > > Thanks for the KIP. Looks good to me overall. A few minor comments > > > below. > > > > > > > > > > 1. Could you change the title from "Retain tombstones" to "Retain > > > > > tombstones and transaction markers" to make it more general? > > > > > > > > > > 2. Could you document which bit in the batch attribute will be used > > for > > > > the > > > > > new flag? The current format of the batch attribute is the > following. > > > > > > > > > > * > > > > > > > > > > > > > > > ------------------------------------------------------------------------------------------------- > > > > > * | Unused (6-15) | Control (5) | Transactional (4) | Timestamp > Type > > > > > (3) | Compression Type (0-2) | > > > > > > > > > > * > > > > > > > > > > > > > > > ------------------------------------------------------------------------------------------------- > > > > > > > > > > 3. Could you provide the reasons for the rejected proposals? For > > > > > proposal 1, one reason is that it doesn't cover the transaction > > > > > markers. For proposal 2, one reason is that the interval record > > header > > > > > could be exposed to the clients. > > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > > On Mon, Oct 14, 2019 at 4:42 PM Richard Yu < > > yohan.richard...@gmail.com > > > > > > > > > wrote: > > > > > > > > > > > Hi all, > > > > > > > > > > > > The discussion for KIP-534 seems to have concluded. > > > > > > So I wish to vote this in so that we can get it done. Its a small > > bug > > > > > fix. > > > > > > :) > > > > > > > > > > > > Below is the KIP link: > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-534%3A+Retain+tombstones+for+approximately+delete.retention.ms+milliseconds > > > > > > > > > > > > Cheers, > > > > > > Richard > > > > > > > > > > > > > > > > > > > > > -- -- Guozhang
