Hello Guozhang,
The KIP is now updated to reflect this choice in strategy.
Please let me know your thoughts there.
Kind Regards,
Luís
From: Guozhang Wang
Sent: 23 April 2018 19:32
To: dev@kafka.apache.org
Subject: Re: RE: [DISCUSS] KIP-280: Enhanced log compaction
Hi Luis,
I think by "generalizing it" we could go beyond numerical values, and
that's why I suggested we do not need to require that the type serialized
to the bytes have any numerical semantics since it has to ben serialized to
a byte array anyways. I understand that for your use case, the intended
record header compaction value is a number, but imagine if someone else
wants to compact the same-keyed messages based on some record header
key-value pair whose value types before serializing to bytes are not
numbers at all, but just some strings:
key: "A", value: "a1", header: ["bar" -> "a".bytes()],
key: "A", value: "a2", header: ["bar" -> "c".bytes()],
key: "A", value: "a3", header: ["bar" -> "b".bytes()],
Could we allow them to use that header for compaction as well?
Now going back to your use case, for numbers that could be negative values,
as long as users are aware of the requirement and change the default
encoding schemes when they generate the producer record while setting the
headers so that the serialized bytes still obey the value that should be OK
(again, as I said, we push this responsibility to users to define the right
serde mechanism, but that seems to be more flexible). For example: -INF
serialized to 0x00000000, -INF+1 serialized to 0x00000001, etc.
Guozhang
On Mon, Apr 23, 2018 at 10:19 AM, Luís Cabral <luis_cab...@yahoo.com.invalid
> wrote:
> Hello Guozhang,
>
> Thanks for the fast reply!
>
> As for the matter of the timestamp, it’s now added to the KIP, so I hope
> this is correctly addressed.
> Kindly let me know if you would like some adaptions to the concept.
>
>
> bq. The issue that I do not understand completely is why you'd keep saying
> that why we need to convert it to a String, first then converting to any
> other fields.
>
> Maybe I’m over-engineering it again, and the problem can be simplified to
> restricting this to values greater than or equal to zero, which ends up
> being ok for my own use case...
> This would then generally guarantee the lexicographic ordering, as you say.
> Is this what you mean? Should I then add this restriction to the KIP?
>
> Cheers,
> Luis
>
> From: Guozhang Wang
> Sent: 23 April 2018 17:55
> To: dev@kafka.apache.org
> Subject: Re: RE: [DISCUSS] KIP-280: Enhanced log compaction
>
> Hello Luis,
>
> Thanks for your email, replying to your points in the following:
>
> > I don't personally see advantages in it, but also the only disadvantage
> that I can think of is putting multiple meanings on this field.
>
> If we do not treat timestamp as a special value of the config, then I
> cannot use the timestamp field of the record as the compaction value, since
> we will only look into the record header other than the default offset,
> right? Then users wanting to use the timestamp as the compaction value have
> to put that timestamp into the record header with a name, which duplicates
> the field unnecessary. So to me without treating it as a special value we
> are doomed to have duplicate record field.
>
> > Having it this way would jeopardize my own particular use case, as I need
> to have an incremental number representing the version (i.e.: 1, 2, 3, 5,
> 52, et cetera)
>
> The issue that I do not understand completely is why you'd keep saying that
> why we need to convert it to a String, first then converting to any other
> fields. Since the header is organized in:
>
> public interface Header {
>
> String key();
>
> byte[] value();
>
> }
>
>
> Which means that the header value can be of any types. So with your use
> case why can't you just serialize your incremental version number into a
> byte array directly, whose lexico-order obeys the version number value?? I
> think the default byte serialization mechanism of the integer is sufficient
> for this purpose (assuming that increment number is int).
>
>
>
> Guozhang
>
>
>
>
> On Mon, Apr 23, 2018 at 2:30 AM, Luís Cabral <luis_cab...@yahoo.com.invalid
> >
> wrote:
>
> > Hi Guozhang,
> >
> > Thank you very much for the patience in explaining your points, I've
> > learnt quite a bit in researching and experimenting after your replies.
> >
> >
> > bq. I still think it is worth defining `timestamp` as a special
> compaction
> > value
> >
> > I don't personally see advantages in it, but also the only disadvantage
> > that I can think of is putting multiple meanings on this field, which
> does
> > not seem enough to dissuade anyone, so I've added it to the KIP as a
> > compromise.
> > (please also see the pull request in case you want to confirm the
> > implementation matches your idea)
> >
> >
> > bq. Should it be "the record with the highest value will be kept"?
> >
> >
> > That is describing a scenario where the records being compared have the
> > same value, in which case the offset is used as a tie-breaker.
> > With trying to cover as much as possible, the "Proposed Changes" may have
> > became confusing to read, sorry for that...
> >
> >
> > bq. Users are then responsible to encode their compaction field according
> > to the byte array lexico-ordering to full fill their ordering semantics.
> It
> > is more flexible to enforce users to encode their compaction field always
> > as a long type.
> >
> > This was indeed my focus on the previous replies, since I am not sure how
> > this would work without adding a lot of responsibility on the client
> side.
> > So, rather than trying to debate best practices, since I don't know which
> > ones are being followed in this project, I will instead debate my own
> > selfish need for this feature:
> > Having it this way would jeopardize my own particular use case, as I need
> > to have an incremental number representing the version (i.e.: 1, 2, 3, 5,
> > 52, et cetera). It does not totally invalidate it, since we can always
> > convert it to String on the client side and left-pad with 0's to the max
> > length of a long, but it seems a shame to have to do this as it would
> > increase the data transfer size (I'm trying to avoid it becoming a
> > bottleneck during high throughput periods). This would likely mean that I
> > would start abusing the "timestamp" approach discussed above, as it keeps
> > the messages nimble, but it would again be a shame to be forced into
> such a
> > hacky solution.
> > This is how I see it, and why I would like to avoid it. But maybe there
> is
> > some smarter way that you know of on how to handle it on the client side
> > that would invalidate these concerns?
> > Please let me know, and I would also greatly value some more feedback
> from
> > other people regarding this topic, so please don't be shy!
> >
> > Kind Regards,Luis On Friday, April 20, 2018, 7:41:30 PM GMT+2,
> Guozhang
> > Wang <wangg...@gmail.com> wrote:
> >
> > Hi Luís,
> >
> > What I'm thinking primarily is that we only need to compare the
> compaction
> > values as LONG for the offset and timestmap "type" (I still think it is
> > worth defining `timestamp` as a special compaction value, with the
> reasons
> > below).
> >
> > Not sure if you've seen my other comment earlier regarding the offset /
> > timestmap, I'm pasting / editing them here to illustrate my idea:
> >
> > --------------
> >
> > I think maybe we have a mis-communication here: I'm not against the idea
> of
> > using headers, but just trying to argue that we could make `timestamp`
> > field a special config value that is referring to the timestamp field in
> > the metadata. So from log cleaner's pov:
> >
> > 1. if the config value is "offset", look into the offset field,
> *comparing
> > their value as long*
> > 2. if the config value is "timestamp", look into the timestamp field,
> > *comparing
> > their value as long*
> > 3. otherwise, say the config value is "foo", search for key "foo" in the
> > message header, comparing the value as *byte arrays*
> >
> > I.e. "offset" and "timestamp" are treated as special cases other than
> case
> > 3) above.
> >
> > --------------
> >
> > I think your main concern is that "Although the byte[] can be compared,
> it
> > is not actually comparable as the versioning is based on a long", while
> I'm
> > thinking we can indeed generalize it: there is not hard reasons that the
> > "compaction value" has to be a long, and since the goal of this KIP is to
> > generalize the log compaction logic to consider header fields, why not
> > allowing it to be of any types than enforcing them still to be a long
> type?
> > Users are then responsible to encode their compaction field according to
> > the byte array lexico-ordering to full fill their ordering semantics. It
> is
> > more flexible to enforce users to encode their compaction field always
> as a
> > long type. Let me know WDYT.
> >
> >
> >
> > Also I have some minor comments on the wiki itself:
> >
> > 1) "When both records being compared contain a matching "compaction
> value",
> > then the record with the highest offset will be kept;"
> >
> > Should it be "the record with the highest value will be kept"?
> >
> >
> >
> >
> > Guozhang
> >
> >
> > On Fri, Apr 20, 2018 at 1:05 AM, Luís Cabral
> <luis_cab...@yahoo.com.invalid
> > >
> > wrote:
> >
> > > Guozhang, is this reply ok with you?
> > >
> > >
> > > If you insist on the byte[] comparison directly, then I would need some
> > > suggestions on how to represent a "version" with it, and then the KIP
> > could
> > > be changed to that.
> > > On Tuesday, April 17, 2018, 2:44:16 PM GMT+2, Luís Cabral <
> > > luis_cab...@yahoo.com> wrote:
> > >
> > > Oops, missed that email...
> > >
> > > bq. It is because when we compare the bytes we do not treat them as
> longs
> > > atall, so we just compare them based on bytes; I admit that if users's
> > > headertypes have some semantic meanings (e.g. it is encoded from a
> long)
> > > they weare forcing them to choose the encoder that obeys key
> > > lexicographicordering; but I felt it is more general than enforcing any
> > > fields that maybe used for log cleaner to be defined as a special type.
> > >
> > > Yes, you can compare bytes between each other (its what that code
> does).
> > > You can then assume (or infer) that the encoding used allows for
> > > lexicographic ordering, which I hope you do not do a lot of. This is
> > > (logically) the same as converting to String and then comparing the
> > > strings, except that it allows for abstracting from the String encoding
> > > (again, either with assumptions or with inferred knowledge).
> > > This is purely academic, however, as the versioning is based on a long,
> > > which is not compatible with this approach. So, is this comment a
> > > fact-check stating that it is possible to compare byte[] overall, or is
> > it
> > > about trying to use it in this KIP?
> > >
> > > Cheers
> > >
> > > PS (because I'm stubborn): It is still not comparable, this comparison
> is
> > > all based on assumptions about the content of the byte array, but I
> hope
> > we
> > > can leave this stuff to Stack Overflow instead of debating it here :)
> > >
> > >
> >
> >
> >
> > --
> > -- Guozhang
> >
>
>
>
> --
> -- Guozhang
>
>
--
-- Guozhang
