I think it is true: currently timestamp field can only be non-negative or
-1, and any other values will be rejected; this proposal allows negative
values but did not change the semantics of current values (non-negative and
-1).
So for current users, if they still only use non-negative or -1 values,
they should not notice this change at all; and they should not be using
other negative values anyways since their app would fail due to rejected
records.
Guozhang
On Mon, Dec 17, 2018 at 6:41 PM Gwen Shapira <g...@confluent.io> wrote:
> 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
>
--
-- Guozhang
