kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boyang Chen <reluctanthero...@gmail.com>
Subject Re: [DISCUSS] KIP-622 Add currentSystemTimeMs and currentStreamTimeMs to ProcessorContext
Date Wed, 01 Jul 2020 02:24:43 GMT
Thanks Will for the KIP. A couple questions and suggestions:

1. I think for new APIs to make most sense, we should add a minimal example
demonstrating how it could be useful to structure unit tests w/o the new
APIs.
2. If this is a testing-only feature, could we only add it
to MockProcessorContext?
3. Regarding the API, since this will be added to the ProcessorContext with
many subclasses, does it make sense to provide default implementations as
well?

Boyang

On Tue, Jun 30, 2020 at 6:56 PM William Bottrell <bottrellw@gmail.com>
wrote:

> Thanks, John! I made the change. How much longer should I let there be
> discussion before starting a VOTE?
>
> On Sat, Jun 27, 2020 at 6:50 AM John Roesler <vvcephei@apache.org> wrote:
>
> > Thanks, Will,
> >
> > That looks good to me. I would only add "cached" or something
> > to indicate that it wouldn't just transparently look up the current
> > System.currentTimeMillis every time.
> >
> > For example:
> > /**
> >  * Returns current cached wall-clock system timestamp in milliseconds.
> >  *
> >  * @return the current cached wall-clock system timestamp in milliseconds
> >  */
> > long currentSystemTimeMs();
> >
> > I don't want to give specific information about _when_ exactly the
> > timestamp cache will be updated, so that we can adjust it in the
> > future, but it does seem important to make people aware that they
> > won't see the timestamp advance during the execution of
> > Processor.process(), for example.
> >
> > With that modification, I'll be +1 on this proposal.
> >
> > Thanks again for the KIP!
> > -John
> >
> > On Thu, Jun 25, 2020, at 02:32, William Bottrell wrote:
> > > Thanks, John! I appreciate you adjusting my lingo. I made the change to
> > the
> > > KIP. I will add the note about system time to the javadoc.
> > >
> > > On Wed, Jun 24, 2020 at 6:52 PM John Roesler <vvcephei@apache.org>
> > wrote:
> > >
> > > > Hi Will,
> > > >
> > > > This proposal looks good to me overall. Thanks for the contribution!
> > > >
> > > > Just a couple of minor notes:
> > > >
> > > > The system time method would return a cached timestamp that Streams
> > looks
> > > > up once when it starts processing a record. This may be confusing, so
> > it
> > > > might be good to state it in the javadoc.
> > > >
> > > > I thought the javadoc for the stream time might be a bit confusing.
> We
> > > > normally talk about “Tasks” not “partition groups” in the public
api.
> > Maybe
> > > > just saying that it’s “the maximum timestamp of any record yet
> > processed by
> > > > the task” would be both high level and accurate.
> > > >
> > > > Thanks again!
> > > > -John
> > > >
> > > > On Mon, Jun 22, 2020, at 02:10, William Bottrell wrote:
> > > > > Thanks, Bruno. I updated the KIP, so hopefully it makes more sense.
> > > > Thanks
> > > > > to Matthias J. Sax and Piotr Smolinski for helping with details.
> > > > >
> > > > > I welcome more feedback. Let me know if something doesn't make
> sense
> > or I
> > > > > need to provide more detail. Also, feel free to enlighten me.
> Thanks!
> > > > >
> > > > > On Thu, Jun 11, 2020 at 1:11 PM Bruno Cadonna <bruno@confluent.io>
> > > > wrote:
> > > > >
> > > > > > Hi Will,
> > > > > >
> > > > > > Thank you for the KIP.
> > > > > >
> > > > > > 1. Could you elaborate a bit more on the motivation in the KIP?
> An
> > > > > > example would make the motivation clearer.
> > > > > >
> > > > > > 2. In section "Proposed Changes" you do not need to show the
> > > > > > implementation and describe internals. A description of the
> > expected
> > > > > > behavior of the newly added methods should suffice.
> > > > > >
> > > > > > 3. In "Compatibility, Deprecation, and Migration Plan" you should
> > > > > > state that the change is backward compatible because the two
> > methods
> > > > > > will be added and no other method will be changed or removed.
> > > > > >
> > > > > > Best,
> > > > > > Bruno
> > > > > >
> > > > > > On Wed, Jun 10, 2020 at 10:06 AM William Bottrell <
> > bottrellw@gmail.com
> > > > >
> > > > > > wrote:
> > > > > > >
> > > > > > > Add currentSystemTimeMs and currentStreamTimeMs to
> > ProcessorContext
> > > > > > > <
> > > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-622%3A+Add+currentSystemTimeMs+and+currentStreamTimeMs+to+ProcessorContext
> > > > > > >
> > > > > > >
> > > > > > > I am extremely new to Kafka, but thank you to John Roesler
and
> > > > Matthias
> > > > > > J.
> > > > > > > Sax for pointing me in the right direction. I accept any
and
> all
> > > > > > feedback.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Will
> > > > > >
> > > > >
> > > >
> > >
> >
>

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