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
>
