kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gwen Shapira <g...@confluent.io>
Subject Re: [VOTE] KIP-32 Add CreateTime and LogAppendTime to Kafka message.
Date Tue, 05 Jan 2016 19:16:42 GMT
+1

On Tue, Jan 5, 2016 at 11:12 AM, Guozhang Wang <wangguoz@gmail.com> wrote:

> +1, the KIP looks good to me now.
>
> Guozhang
>
> On Tue, Jan 5, 2016 at 1:47 AM, Becket Qin <becket.qin@gmail.com> wrote:
>
> > Hi Adi,
> >
> > Please see inline comments. And thanks for correcting my typos :)
> >
> > > On Jan 5, 2016, at 3:37 AM, Aditya Auradkar
> > <aauradkar@linkedin.com.INVALID> wrote:
> > >
> > > Hey Becket/Anna -
> > >
> > > I have a few comments about the KIP.
> > >
> > > 1. (Minor) Can we rename the KIP? It's currently "Add CreateTime and
> > > LogAppendTime etc..". This is actually the title of the now rejected
> > Option
> > Done. Please notice the link will change as well. The previous link in
> the
> > thread no longer works.
> > > 1.
> > > 2. (Minor) Can we rename the proposed option? It isn't really "option
> 4"
> > > anymore.
> > Done.
> > > 3. I'm not clear on what exactly happens to compressed messages
> > > when message.timestamp.type=LogAppendTime? Does every batch get
> > > recompressed because the inner message gets rewritten with the server
> > > timestamp? Or does the message set on disk have the timestamp set to
> -1.
> > In
> > > that case, what do we use as timestamp for the message?
> > The current proposal is to keep the inner message timestamp=-1. Broker
> > only overwrites it if it is not -1, in that case the broker will do
> > recompression.
> > > 4. Do message.timestamp.type and max.message.time.difference.ms need
> to
> > be
> > > per-topic configs? It seems that this is really a client config i.e. a
> > > client is the source of timestamps not a topic. It could also be a
> > > broker-level config to keep things simple.
> > I feel the current proposal is more server oriented, i.e, server decides
> > whether accept, overwrite or reject a timestamp. This ensures the
> timestamp
> > on the broker has the same type. I am not sure about the granularity. My
> > gut feeling is that topic level configuration helps with multi tenant
> > cluster. Eventually it is the application who'd decide how they want to
> > define the timestamp and it might be hard to for all the applications
> have
> > same timestamp usage in the same cluster.
> > > 5. The "Proposed Changes" section in the KIP tries to build a
> time-based
> > > index for query but that is a separate proposal (KIP-33). Can we more
> > > crisply identify what exactly will change when this KIP (and 31) is
> > > implemented? It isn't super clear to me at this point.
> > Good point. They are not going to be implemented in this KIP. They are
> > listed because they are closely related to KIP-32.
> >
> > Other than the time index and changes on top of it, KIP-31 and KIP-32
> will
> > implement everything else needed for message format change.
> > >
> > > Aside from that, I think the "Rejected Alternatives" section of the KIP
> > is
> > > excellent. Very good insight into what options were discussed and
> > rejected.
> > >
> > > Aditya
> > >
> > >> On Mon, Dec 28, 2015 at 3:57 PM, Becket Qin <becket.qin@gmail.com>
> > wrote:
> > >>
> > >> Thanks Guozhang, Gwen and Neha for the comments. Sorry for late reply
> > >> because I only have occasional gmail access from my phone...
> > >>
> > >> I just updated the wiki for KIP-32.
> > >>
> > >> Gwen,
> > >>
> > >> Yes, the migration plan is what you described.
> > >>
> > >> I agree with your comments on the version.
> > >> I changed message.format.version to use the release version.
> > >> I did not change the internal version, we can discuss this in a
> separate
> > >> thread.
> > >>
> > >> Thanks,
> > >>
> > >> Jiangjie (Becket) Qin
> > >>
> > >>
> > >>
> > >>> On Dec 24, 2015, at 5:38 AM, Guozhang Wang <wangguoz@gmail.com>
> wrote:
> > >>>
> > >>> Also I agree with Gwen that such changes may worth a 0.10 release or
> > even
> > >>> 1.0, having it in 0.9.1 would be quite confusing to users.
> > >>>
> > >>> Guozhang
> > >>>
> > >>>> On Wed, Dec 23, 2015 at 1:36 PM, Guozhang Wang <wangguoz@gmail.com>
> > >> wrote:
> > >>>>
> > >>>> Becket,
> > >>>>
> > >>>> Please let us know once you have updated the wiki page regarding
the
> > >>>> migration plan. Thanks!
> > >>>>
> > >>>> Guozhang
> > >>>>
> > >>>>> On Wed, Dec 23, 2015 at 11:52 AM, Gwen Shapira <gwen@confluent.io>
> > >> wrote:
> > >>>>>
> > >>>>> Thanks Becket, Anne and Neha for responding to my concern.
> > >>>>>
> > >>>>> I had an offline discussion with Anne where she helped me
> understand
> > >> the
> > >>>>> migration process. It isn't as bad as it looks in the KIP :)
> > >>>>>
> > >>>>> If I understand it correctly, the process (for users) will
be:
> > >>>>>
> > >>>>> 1. Prepare for upgrade (set format.version = 0, ApiVersion
= 0.9.0)
> > >>>>> 2. Rolling upgrade of brokers
> > >>>>> 3. Bump ApiVersion to 0.9.0-1, so fetch requests between brokers
> will
> > >> use
> > >>>>> the new protocol
> > >>>>> 4. Start upgrading clients
> > >>>>> 5. When "enough" clients are upgraded, bump format.version
to 1
> > >> (rolling).
> > >>>>>
> > >>>>> Becket, can you confirm?
> > >>>>>
> > >>>>> Assuming this is the process, I'm +1 on the change.
> > >>>>>
> > >>>>> Reminder to coders and reviewers that pull-requests with
> user-facing
> > >>>>> changes should include documentation changes as well as code
> changes.
> > >>>>> And a polite request to try to be helpful to users on when
to use
> > >>>>> create-time and when to use log-append-time as configuration
- this
> > is
> > >> not
> > >>>>> a trivial decision.
> > >>>>>
> > >>>>> A separate point I'm going to raise in a different thread is
that
> we
> > >> need
> > >>>>> to streamline our versions a bit:
> > >>>>> 1. I'm afraid that 0.9.0-1 will be confusing to users who care
> about
> > >>>>> released versions (what if we forget to change it before the
> release?
> > >> Is
> > >>>>> it
> > >>>>> meaningful enough to someone running off trunk?), we need to
come
> up
> > >> with
> > >>>>> something that will work for both LinkedIn and everyone else.
> > >>>>> 2. ApiVersion has real version numbers. message.format.version
has
> > >>>>> sequence
> > >>>>> numbers. This makes us look pretty silly :)
> > >>>>>
> > >>>>> My version concerns can be addressed separately and should
not hold
> > >> back
> > >>>>> this KIP.
> > >>>>>
> > >>>>> Gwen
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> On Tue, Dec 22, 2015 at 11:01 PM, Becket Qin <becket.qin@gmail.com
> >
> > >>>>> wrote:
> > >>>>>
> > >>>>>> Hi Anna,
> > >>>>>>
> > >>>>>> Thanks for initiating the voting process. I did not start
the
> voting
> > >>>>>> process because there were still some ongoing discussion
with Jun
> > >> about
> > >>>>> the
> > >>>>>> timestamp regarding compressed messages. That is why the
wiki page
> > >>>>> hasn't
> > >>>>>> reflected the latest conversation as Guozhang pointed out.
> > >>>>>>
> > >>>>>> Like Neha said I think we have reached general agreement
on this
> > KIP.
> > >> So
> > >>>>>> it is probably fine to start the KIP voting. At least we
draw more
> > >>>>>> attention to the KIP even if there are some new discussion
to
> bring
> > >> up.
> > >>>>>>
> > >>>>>> Regarding the upgrade plan, given we decided to implement
KIP-31
> and
> > >>>>>> KIP-32 in the same patch to avoid change binary protocol
twice,
> the
> > >>>>> upgrade
> > >>>>>> plan was mostly discussed on the discussion thread of KIP-31.
> > >>>>>>
> > >>>>>> Since the voting has been initiated, I will update the
wiki with
> > >> latest
> > >>>>>> conversation to avoid further confusion.
> > >>>>>>
> > >>>>>> BTW, I actually have started coding work on KIP-31 and
KIP-32 and
> > will
> > >>>>>> focus on the patch before I return from vacation in mid
Jan
> because
> > I
> > >>>>> have
> > >>>>>> no LInkedIn VPN access in China anyway...
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>>
> > >>>>>> Jiangjie
> > >>>>>>
> > >>>>>>> On Dec 23, 2015, at 12:31 PM, Anna Povzner <anna@confluent.io>
> > >> wrote:
> > >>>>>>>
> > >>>>>>> Hi Gwen,
> > >>>>>>>
> > >>>>>>> I just wanted to point out that I just started the
vote. Becket
> > wrote
> > >>>>> the
> > >>>>>>> proposal and led the discussions.
> > >>>>>>>
> > >>>>>>> What I understood from reading the discussion thread,
the
> migration
> > >>>>> plan
> > >>>>>>> was discussed at the KIP meeting, and not much on the
mailing
> list
> > >>>>>> itself.
> > >>>>>>>
> > >>>>>>> My question about the migration plan was same as Guozhang
wrote:
> > The
> > >>>>> case
> > >>>>>>> when an upgraded broker receives an old producer request.
The
> > >>>>> proposal is
> > >>>>>>> for the broker to fill in the timestamp field with
the current
> time
> > >> at
> > >>>>>> the
> > >>>>>>> broker. Cons: it goes against the definition of CreateTime
type
> of
> > >> the
> > >>>>>>> timestamp (we are "over-writing" it at the broker).
Pros: It
> looks
> > >>>>> like
> > >>>>>>> most of the use-cases would actually want that behavior,
because
> > >>>>>> otherwise
> > >>>>>>> timestamp is useless and they will need to support
an old,
> > >>>>> pre-timestamp,
> > >>>>>>> behavior. E.g., if we modify log retention policy to
use the
> > >>>>> timestamp,
> > >>>>>> we
> > >>>>>>> would need to support an old implementation (the one
that does
> not
> > >> use
> > >>>>>>> timestamps in the message). So I actually have a preference
for
> the
> > >>>>>>> proposed approach.
> > >>>>>>>
> > >>>>>>> Thanks,
> > >>>>>>> Anna
> > >>>>>>>
> > >>>>>>>> On Tue, Dec 22, 2015 at 8:02 PM, Neha Narkhede
<
> neha@confluent.io
> > >
> > >>>>>> wrote:
> > >>>>>>>>
> > >>>>>>>> Hey Gwen,
> > >>>>>>>>
> > >>>>>>>> Migration plan wasn't really discussed a ton in
the previous
> > >> threads.
> > >>>>>> So it
> > >>>>>>>> will be great to dive deep and see if there are
gaps there. I
> had
> > >>>>> some
> > >>>>>>>> questions, but the details listed on the KIP are
great.
> > >>>>>>>>
> > >>>>>>>> It is complex, though the plan outlined in the
wiki assumes a
> zero
> > >>>>>> downtime
> > >>>>>>>> upgrade assuming that producers and consumers can't
be upgraded
> in
> > >>>>>> tandem.
> > >>>>>>>> This is typical for companies that have a significant
Kafka
> > >>>>> footprint,
> > >>>>>> like
> > >>>>>>>> LinkedIn.
> > >>>>>>>>
> > >>>>>>>> Thanks,
> > >>>>>>>> Neha
> > >>>>>>>>
> > >>>>>>>>> On Tue, Dec 22, 2015 at 7:48 PM, Gwen Shapira
<
> gwen@confluent.io
> > >
> > >>>>>> wrote:
> > >>>>>>>>>
> > >>>>>>>>> Hi Anna,
> > >>>>>>>>>
> > >>>>>>>>> Thanks for the KIP, especially for the details
on all the
> > >>>>> alternatives
> > >>>>>>>> and
> > >>>>>>>>> how we arrived at the proposal. Its really
great!
> > >>>>>>>>>
> > >>>>>>>>> Can you point me at where the migration plan
was discussed? It
> > >> looks
> > >>>>>>>> overly
> > >>>>>>>>> complex and I have a bunch of questions, but
if there was a
> > >>>>> discussion,
> > >>>>>>>> I'd
> > >>>>>>>>> like to read up rather than repeating it :)
> > >>>>>>>>>
> > >>>>>>>>> Gwen
> > >>>>>>>>>
> > >>>>>>>>>> On Tue, Dec 22, 2015 at 12:34 PM, Anna
Povzner <
> > anna@confluent.io
> > >>>
> > >>>>>>>>> wrote:
> > >>>>>>>>>
> > >>>>>>>>>> Hi,
> > >>>>>>>>>>
> > >>>>>>>>>> I am opening the voting thread for KIP-32:
Add CreateTime and
> > >>>>>>>>>> LogAppendTime to Kafka message.
> > >>>>>>>>>>
> > >>>>>>>>>> For reference, here's the KIP wiki:
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-32+-+Add+CreateTime+and+LogAppendTime+to+Kafka+message
> > >>>>>>>>>>
> > >>>>>>>>>> And the mailing list threads:
> > >>>>>>>>>>
> > >>>>>>>>>> September:
> > >>
> >
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201509.mbox/%3CCAHrRUm6NMg%3DPh4HAJdxr%3DpmZhfFcD5OEV2yxj3fg%2BXnEBTW%2B3w%40mail.gmail.com%3E
> > >>>>>>>>>>
> > >>>>>>>>>> October:
> > >>
> >
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201510.mbox/%3CCAHrRUm7RiBAJxwO15s1tztz%3D15oibO-QJ%2B_w8AxafTnuw3jjCw%40mail.gmail.com%3E
> > >>>>>>>>>>
> > >>>>>>>>>> December:
> > >>
> >
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201512.mbox/%3CCAHrRUm4ugxDYzyy26MGRGKpK4hsjT4EKTuu18M3wztYq4PE%3DaQ%40mail.gmail.com%3E
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> Thanks
> > >>>>>>>>>> Anna
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> --
> > >>>>>>>> Thanks,
> > >>>>>>>> Neha
> > >>>>
> > >>>>
> > >>>>
> > >>>> --
> > >>>> -- Guozhang
> > >>>
> > >>>
> > >>>
> > >>> --
> > >>> -- Guozhang
> > >>
> >
>
>
>
> --
> -- Guozhang
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message