Guozhang,
Can you speak to the following note under "impact on existing users":
"No impact on current users, they should update their infrastructure
in that order: Broker, Consumers, Producers."
I think this isn't true, but if it is - we need to fix the proposal
and make sure we allow upgrade at any order (as is usual for Kafka
since version 0.10.0.0)
On Sun, Dec 9, 2018 at 9:21 PM Guozhang Wang <wangg...@gmail.com> wrote:
>
> Hi folks,
>
> Thanks for your replies! Just to clarify, the proposal itself does not
> introduce any additional fields in the message format (some new attributes
> are mentioned in the Rejected Alternatives though), so my understand is that
> we do not need to increment the magic byte version of the message itself.
>
> Also, ConsumerRecord.NO_TIMESTAMP (-1) is still used as "no timestamp", and
> as we've discussed before it is a good trade-off to say "we do not have way
> express Wednesday, December 31, 1969 11:59:59.999". I.e. we are extending the
> timestamp to have other negative values than -1, but we did not change the
> semantics if value -1 itself. So I think although we do bump up the request
> protocol version for semantics changes, it is not necessary for this case
> (please correct me if there are cases that would not work for it).
>
> I do agree with Magnus's point that for ListOffsetsRequest, we should
> consider using different values than -1 / -2 to indicate `EARLEST / LATEST
> TIMESTAMP` now since for example timestamp -2 does have a meaningful
> semantics now, and hence its protocol version would need bump as we change
> its field. And I think a single byte indicating the type as (EARLEST, LATEST,
> ACTUAL_TIMESTAMP_VALUE) should be sufficient, but I'll leave it to Konstandin
> to decide if he wants to do this KIP or do it in another follow-up KIP.
>
>
> Guozhang
>
>
> On Fri, Dec 7, 2018 at 5:06 PM Jun Rao <j...@confluent.io> wrote:
>>
>> Hi, Konstandin,
>>
>> Thanks for the KIP. I agree with Magnus on the protocol version changes. As
>> for the sentinel value, currently ConsumerRecord.NO_TIMESTAMP (-1) is used
>> for V0 message format. For compatibility, it seems that we still need to
>> preserve that.
>>
>> Jun
>>
>> On Thu, Dec 6, 2018 at 2:32 AM Magnus Edenhill <mag...@edenhill.se> wrote:
>>
>> > Sorry for getting in the game this late, and on the wrong thread!
>> >
>> > I think negative timestamps makes sense and is a good addition,
>> > but I have a couple of concerns with the proposal:
>> >
>> > 1. I believe any change to the protocol format or semantics require a
>> > protocol bump, in this case for ProduceRequest, FetchRequest,
>> > ListOffsetsRequest.
>> > 2. ListOffsetsRequest should be changed to allow logical (END, BEGINNING)
>> > and absolute lookups without special treatment of two absolute values as
>> > logical (-1, -2), this seems like a hack and will require application logic
>> > to avoid these timestamps, that's leaky abstraction.
>> > Perhaps add a new field `int8 LookupType = { BEGINNING=-2, END=-1,
>> > TIMESTAMP=0 }`: the broker will either look up using the absolute
>> > Timestamp, or logical offset value, depending on the value of LookupType.
>> > 3. With the added Attribute for extended timestamp, do we really need to
>> > have a sentinel value for an unset timestamp (-1 or Long.MIN_VALUE)?
>> > To make the logic simpler I suggest the attribute is renamed to just
>> > Timestamp, and if the Timestamp attribute is set, the Timestamp field is
>> > always a proper timestamp. If the bit is not set, no timestamp was
>> > provided.
>> >
>> > /Magnus
>> >
>> >
>> >
>> > Den tors 6 dec. 2018 kl 08:06 skrev Gwen Shapira <g...@confluent.io>:
>> >
>> > > I may be missing something, but why are we using an attribute for
>> > > this? IIRC, we normally bump protocol version to indicate semantic
>> > > changes. If I understand correctly, not using an attribute will allow
>> > > us to not change the message format (just the protocol), which makes
>> > > the upgrade significantly easier (since we don't up/down convert).
>> > >
>> > > Another thing I don't understand: The compatibility map indicates that
>> > > NO_TIMESTAMP is now Long.MIN_VALUE, but a bit above that you say that
>> > > -1 semantics does not change.
>> > >
>> > > Last: At around version 1.0 we decide to completely avoid changes that
>> > > require a particular order of upgrades (this is why we added the
>> > > versions API, up/down conversion, etc). So I'd like to see if we can
>> > > avoid this here (should be relatively easy?). I'm CCing Magnus,
>> > > because I think he remembers *why* we made this decision in first
>> > > place.
>> > >
>> > > Gwen
>> > > On Thu, Dec 6, 2018 at 4:44 PM Guozhang Wang <wangg...@gmail.com> wrote:
>> > > >
>> > > > Bump up on this thread again: we have two binding votes already and
>> > need
>> > > > another committer to take a look at it and vote.
>> > > >
>> > > >
>> > > > Guozhang
>> > > >
>> > > > On Fri, Oct 19, 2018 at 11:34 AM Konstantin Chukhlomin <
>> > > chuhlo...@gmail.com>
>> > > > wrote:
>> > > >
>> > > > > bump
>> > > > >
>> > > > > On Tue, Jun 12, 2018 at 1:48 PM Bill Bejeck <bbej...@gmail.com>
>> > wrote:
>> > > > >
>> > > > > > +1
>> > > > > >
>> > > > > > Thanks,
>> > > > > > Bill
>> > > > > >
>> > > > > > On Sun, Jun 10, 2018 at 5:32 PM Ted Yu <yuzhih...@gmail.com>
>> > wrote:
>> > > > > >
>> > > > > > > +1
>> > > > > > >
>> > > > > > > On Sun, Jun 10, 2018 at 2:17 PM, Matthias J. Sax <
>> > > > > matth...@confluent.io>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > +1 (binding)
>> > > > > > > >
>> > > > > > > > Thanks for the KIP.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > -Matthias
>> > > > > > > >
>> > > > > > > > On 5/29/18 9:14 AM, Konstantin Chukhlomin wrote:
>> > > > > > > > > Thanks, updated the KIP.
>> > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > > > > > > 228+Negative+record+timestamp+support <
>> > https://cwiki.apache.org/
>> > > > > > > >
>> > > confluence/display/KAFKA/KIP-228+Negative+record+timestamp+support>
>> > > > > > > > >
>> > > > > > > > >> On May 24, 2018, at 1:39 PM, Guozhang Wang <
>> > > wangg...@gmail.com>
>> > > > > > > wrote:
>> > > > > > > > >>
>> > > > > > > > >> Thanks Konstantin, I have a minor comment on the wiki page's
>> > > > > > "Proposed
>> > > > > > > > >> Solution" section:
>> > > > > > > > >>
>> > > > > > > > >> "The solution is to ignore that problem and it is a choice
>> > of
>> > > the
>> > > > > > user
>> > > > > > > > >> to..":
>> > > > > > > > >>
>> > > > > > > > >> ----------
>> > > > > > > > >>
>> > > > > > > > >> I'd suggest we rephrase it and admit "-1 is a special value
>> > in
>> > > > > > Kafka,
>> > > > > > > > which
>> > > > > > > > >> means we do not have a valid way to express Wednesday,
>> > > December
>> > > > > 31,
>> > > > > > > 1969
>> > > > > > > > >> 11:59:59 PM UTC." rather than letting users to choose how to
>> > > > > > interpret
>> > > > > > > > it,
>> > > > > > > > >> since as mentioned, both at Kafka broker side, as well as at
>> > > Kafka
>> > > > > > > > client
>> > > > > > > > >> side (Streams, Producer) we have already using this protocol
>> > > to
>> > > > > set
>> > > > > > -1
>> > > > > > > > as a
>> > > > > > > > >> special value of `unknown`, so users choosing how to
>> > > interpret it
>> > > > > > > freely
>> > > > > > > > >> would lead to confusing behaviors. Instead, we should well
>> > > > > document
>> > > > > > > this
>> > > > > > > > >> caveat, and educate users in the document that, given this
>> > > > > > situation,
>> > > > > > > if
>> > > > > > > > >> you do need to express Wednesday, December 31, 1969 11:59:59
>> > > PM
>> > > > > UTC,
>> > > > > > > > >> consider shifting it by one millisecond.
>> > > > > > > > >>
>> > > > > > > > >>
>> > > > > > > > >> Otherwise, I'm +1 on this KIP.
>> > > > > > > > >>
>> > > > > > > > >>
>> > > > > > > > >> Guozhang
>> > > > > > > > >>
>> > > > > > > > >>
>> > > > > > > > >>
>> > > > > > > > >> On Wed, May 23, 2018 at 2:40 PM, Konstantin Chukhlomin <
>> > > > > > > > chuhlo...@gmail.com>
>> > > > > > > > >> wrote:
>> > > > > > > > >>
>> > > > > > > > >>> All,
>> > > > > > > > >>>
>> > > > > > > > >>> Thanks for the feedback on KIP-228. I've updated the KIP,
>> > and
>> > > > > would
>> > > > > > > > like
>> > > > > > > > >>> to start to start a vote.
>> > > > > > > > >>>
>> > > > > > > > >>> KIP:
>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > > > > > > >>> 228+Negative+record+timestamp+support <
>> > > https://cwiki.apache.org/
>> > > > > > > > >>>
>> > > > > confluence/display/KAFKA/KIP-228+Negative+record+timestamp+support>
>> > > > > > > > >>>
>> > > > > > > > >>> Pull Request:
>> > > https://github.com/apache/kafka/pull/5072/files <
>> > > > > > > > >>> https://github.com/apache/kafka/pull/5072/files>
>> > > > > > > > >>>
>> > > > > > > > >>> Discussion Thread: http://mail-archives.apache.
>> > > > > > > > >>> org/mod_mbox/kafka-dev/201712.mbox/%3c178138FA-CA30-44A8-
>> > > > > > > > >>> b92a-b966b8ca1...@gmail.com%3e <
>> > http://mail-archives.apache.
>> > > > > > > > >>> org/mod_mbox/kafka-dev/201712.mbox/%3C178138FA-CA30-44A8-
>> > > > > > > > >>> b92a-b966b8ca1...@gmail.com%3E>
>> > > > > > > > >>>
>> > > > > > > > >>> Thanks!
>> > > > > > > > >>
>> > > > > > > > >>
>> > > > > > > > >>
>> > > > > > > > >>
>> > > > > > > > >> --
>> > > > > > > > >> -- Guozhang
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > >
>> > > > > --
>> > > > > Konstantin Chukhlomin
>> > > > >
>> > > >
>> > > >
>> > > > --
>> > > > -- Guozhang
>> > >
>> > >
>> > >
>> > > --
>> > > Gwen Shapira
>> > > Product Manager | Confluent
>> > > 650.450.2760 | @gwenshap
>> > > Follow us: Twitter | blog
>> > >
>> >
>
>
>
> --
> -- Guozhang
--
Gwen Shapira
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter | blog
