Thanks, Guozhang! All of your feedback sounds good to me. I’ll update the KIP when I am able.
3) I believe it is the position after the fetch, but I will confirm. I think omitting position may render beginning and end offsets useless as well, which leaves only lag. That would be fine with me, but it also seems nice to supply this extra metadata since it is well defined and probably handy for others. Therefore, I’d go the route of specifying the exact semantics and keeping it. Thanks for the review, John On Fri, Dec 11, 2020, at 17:36, Guozhang Wang wrote: > Hello John, > > Thanks for the updates! I've made a pass on the KIP and also the POC PR, > here are some minor comments: > > 1) nit: "receivedTimestamp" -> it seems the metadata keep getting updated, > and we do not create a new object but just update the values in-place, so > maybe calling it `lastUpdateTimstamp` is better? > > 2) It will be great to verify in javadocs that the new API > "ConsumerRecords#metadata(): Map<TopicPartition, Metadata>" may return a > superset of TopicPartitions than the existing API that returns the data by > partitions, in case users assume their map key-entries would always be the > same. > > 3) The "position()" API of the call needs better clarification: is it the > current position AFTER the records are returned, or is it BEFORE the > records are returned? Personally I'd suggest we do not include it if it is > not used anywhere yet just to avoid possible misuage, but I'm fine if you > like to keep it still; in that case just clarify its semantics. > > > Other than that，I'm +1 on the KIP as well ! > > > Guozhang > > > On Fri, Dec 11, 2020 at 8:15 AM Walker Carlson <wcarl...@confluent.io> > wrote: > > > Thanks for the KIP! > > > > +1 (non-binding) > > > > walker > > > > On Wed, Dec 9, 2020 at 11:40 AM Bruno Cadonna <br...@confluent.io> wrote: > > > > > Thanks for the KIP, John! > > > > > > +1 (non-binding) > > > > > > Best, > > > Bruno > > > > > > On 08.12.20 18:03, John Roesler wrote: > > > > Hello all, > > > > > > > > There hasn't been much discussion on KIP-695 so far, so I'd > > > > like to go ahead and call for a vote. > > > > > > > > As a reminder, the purpose of KIP-695 to improve on the > > > > "task idling" feature we introduced in KIP-353. This KIP > > > > will allow Streams to offer deterministic time semantics in > > > > join-type topologies. For example, it makes sure that > > > > when you join two topics, that we collate the topics by > > > > timestamp. That was always the intent with task idling (KIP- > > > > 353), but it turns out the previous mechanism couldn't > > > > provide the desired semantics. > > > > > > > > The details are here: > > > > https://cwiki.apache.org/confluence/x/JSXZCQ > > > > > > > > Thanks, > > > > -John > > > > > > > > > > > > -- > -- Guozhang >
