Hi Chia-Ping, Thanks for the review!
This is a good point to add provision to convey the timeout from client to the server. Updated the KIP to add a new 'TimeoutMs' field in the ListOffsetsRequest and bumped the version to 10. To retain the existing ListOffsets request behaviour: 1. AdminClient will supply the "default.api.timeout.ms" as timeout in the ListOffsetsRequest and 2. Consumer will supply the "request.timeout.ms" as timeout in the ListOffsetsRequest PTAL. -- Kamal On Tue, Aug 27, 2024 at 2:32 PM Chia-Ping Tsai <chia7...@apache.org> wrote: > hi Kamal, > > > If we add the timeout in the ListOffsetsRequest.json, then when old > clients > talk with the new broker, we don't have > a timeout to set on the server. Moved adding the timeout to the > > Personally, adding the timeout to ListOffsetsRequest open a room to > clients to pick up the "suitable" wait time. The other purgatory-related > requests (fetch, produce, heartbeat, delete_record) having similar config > in request-level. > > Maybe we can keep both request-level timeout and server-side config. If > the request does not define the timeout (from old client), we use > server-side config instead. WDYT? > > Best, > Chia-PIng >
