kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guozhang Wang <wangg...@gmail.com>
Subject Re: [VOTE] KIP-32 Add CreateTime and LogAppendTime to Kafka message.
Date Tue, 05 Jan 2016 19:12:40 GMT
+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