Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-02-21 Thread deng ziming
Thank you David, I started a new one and let’s close this. Best, Ziming Deng > On Feb 21, 2022, at 5:31 PM, David Jacot wrote: > > Thanks for the update. LGTM. > > The vote thread looks weird. Could you check or > perhaps create a new one with the latest title? > > Best, > David > > On Fri,

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-02-21 Thread David Jacot
Thanks for the update. LGTM. The vote thread looks weird. Could you check or perhaps create a new one with the latest title? Best, David On Fri, Feb 11, 2022 at 2:57 PM deng ziming wrote: > > David, Thank you for pointing out this. > > After some time, I sorted out the main configurations, the

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-02-11 Thread deng ziming
David, Thank you for pointing out this. After some time, I sorted out the main configurations, the main difference are default.api.timeout.ms , retries, request.timeout.ms ,metadata.max.age.ms . Please tak

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-02-01 Thread David Jacot
Hey, Thanks for updating the KIP. I think that there are a few more configs which could be used. e.g. all the network related configs - they are in both consumer and admin configurations as well. Is `session.timeout.ms` relevant in our context? It does not seem to be used when querying offsets. R

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-02-01 Thread deng ziming
Hey David, I rechecked the ConsumerConfig and split the Compatibility section into 2 sub-sections regarding AdminClientConfig and ConsumerConfig, I also find a small bug that we use different timeout in `KafkaConsumer.beginningOffsets` and `KafkaConsumer.endOffsets`, I will fix this. Please he

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-02-01 Thread David Jacot
Thanks for the updated KIP. Regarding the compatibility section, I think that it would be great if we could really stress that the configurations that could reasonably be used to configure the tool are actually all supported by the admin client. Regarding the retry mechanism, the consumer will ret

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-01-31 Thread David Jacot
Hey, Thanks for the KIP. I have a few comments: 1. I think that it would be better to name the KIP: "GetOffsetShell should support max-timestamp" or something like that as this is the initial intent of the change. 2. There is a typo: `OffsetSpce` -> `OffsetSpec`. 3. It would be great if we coul

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-01-30 Thread deng ziming
Sorry all, I mean KIP-851 not KIP-734. In KIP-734 we add a new OffsetSpec to AdminClient, in this KIP I just extend this OffsetSpec to GetOffsetShell. > On Jan 30, 2022, at 6:29 PM, deng ziming wrote: > > Hey all, I'm starting the voting on KIP-734. > > This supports a new OffsetSpec in GetOf

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-01-30 Thread Luke Chen
Hi Ziming, Sorry that I checked the email archive , the subject is correct. There could be something wrong in gmail grouping. Anyway, It's good to support fetch max timestamp in GetOffsetShell. I'm +1 (non-binding) BTW, you forgot

Re: [VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-01-30 Thread Luke Chen
Hi Ziming, Just a reminder, you should start a new thread with email subject starting with "[VOTE] KIP-815". Please refer to other vote email threads for example. Luke On Sun, Jan 30, 2022 at 6:37 PM deng ziming wrote: > Hey all, I'm starting the voting on KIP-734. > > This supports a n

[VOTE] KIP-815: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-01-30 Thread deng ziming
Hey all, I'm starting the voting on KIP-734. This supports a new OffsetSpec in GetOffsetShell so that we can easily determine the offset and timestamp of the message with the largest timestamp on a partition. This seems a simple change but replaced KafkaConsumer with AdminClient in GetOffset