Thanks for your feedback, Sagar, Boyang. I've added an additional API to take the Set<TopicPartition> as the partitions to fetch from. Good suggestion! I also updated the java doc in the KIP.
And for the question that the behavior can also be achieved by using manual offset commit + offset position rewind. That's true. But I have the same thoughts as Sagar, which is that, it's for advanced users. Another reason is for simplicity. If you've ever used the peek API from java collection (ex: Queue#peek), you should know what I'm talking about. When you have data in a queue, if you want to know what the first data is in the queue, you'd use peek(). You can also achieve it by remove() the 1st element from queue, and then added it back to the right position, but I believe that's not what you'd do. Thank you. Luke On Tue, Sep 21, 2021 at 1:02 AM Sagar <sagarmeansoc...@gmail.com> wrote: > Thanks Luke for the KIP. I think it makes sense. > > @Boyang, > > While it is possible to get the functionality using manual offset commit + > offset position rewind as you stated, IMHO it could still be a very handy > addition to the APIs. The way I see it, manual offset commit + offset > position rewind is for slightly more advanced users and the addition of > peek() API would make it trivial to get the mentioned functionality. > > I agree to the point of adding a mechanism to fetch a more fine grained set > of records. Maybe, add another API which takes a Set<TopicPartition>? In > this case, we would probably need to add a behaviour to throw some > exception when a user tries to peek from a TopicPartition that he/she isn't > subscribed to. > > nit: In the javadoc, this line => > > This method returns immediately if there are records available or > exception thrown. > > > should probably be => > > > This method returns immediately if there are no records available or > exception thrown. > > > Thanks! > > Sagar. > > > > > On Mon, Sep 20, 2021 at 4:22 AM Boyang Chen <reluctanthero...@gmail.com> > wrote: > > > Thanks Luke for the KIP. > > > > I think I understand the motivation is to avoid affecting offset > positions > > of the records, but the feature could be easily realized on the user side > > by using manual offset commit + offset position rewind. So the new peek() > > function doesn't provide any new functionality IMHO, weakening the > > motivation a bit. > > > > Additionally, for the peek() case, I believe that users may want to have > > more fine-grained exposure of records, such as from specific partitions > > instead of getting random records. It's probably useful to define an > option > > handle class in the parameters to help clarify what specific records to > be > > returned. > > > > Boyang > > > > On Sun, Sep 19, 2021 at 1:51 AM Luke Chen <show...@gmail.com> wrote: > > > > > Hi everyone, > > > > > > I'd like to discuss the following proposal to add Consumer#peek for > > > debugging/tuning. > > > > > > The main purpose for Consumer#peek is to allow users: > > > > > > 1. peek what records existed at broker side and not increasing the > > > position offsets. > > > 2. throw exceptions when there is connection error existed between > > > consumer and broker (or other exceptions will be thrown by "poll") > > > > > > > > > detailed description can be found her: > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=188746244 > > > > > > > > > Any comments and feedback are welcomed. > > > > > > Thank you. > > > Luke > > > > > >
