Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-06-05 Thread Cheng Tan
The KIP is approved upon 3 binding votes. Thanks for all the feedback and votes! - Cheng

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-06-05 Thread Gwen Shapira
+1 (binding) Thank you for the contribution. On Wed, Jun 3, 2020 at 12:53 AM Cheng Tan wrote: > Dear Rajini, > > Thanks for the feedback. > > 1) > Because "request.timeout.ms" only affects in-flight requests, after the > API NetworkClient.ready() is invoked, the connection won't get closed

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-06-03 Thread Colin McCabe
Hi Cheng, Thanks for working on this. Looks good. How about "socket.connection.setup.timeout.ms" and "socket.connection.setup.timeout.max.ms" (not connections with an S)? +1 (binding) best, Colin On Wed, Jun 3, 2020, at 06:24, Rajini Sivaram wrote: > Hi Cheng, > > Thanks for the updates,

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-06-03 Thread Rajini Sivaram
Hi Cheng, Thanks for the updates, looks good. +1 (binding) Regards, Rajini On Wed, Jun 3, 2020 at 8:53 AM Cheng Tan wrote: > Dear Rajini, > > Thanks for the feedback. > > 1) > Because "request.timeout.ms" only affects in-flight requests, after the > API NetworkClient.ready() is invoked, the

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-06-03 Thread Cheng Tan
Dear Rajini, Thanks for the feedback. 1) Because "request.timeout.ms" only affects in-flight requests, after the API NetworkClient.ready() is invoked, the connection won't get closed after "request.timeout.ms” hits. Before a) the SocketChannel is connected b) ssl handshake

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-06-02 Thread Rajini Sivaram
Hi Cheng, Not sure if the discussion should move back to the DISCUSS thread. I have a few questions: 1) The KIP motivation says that in some cases `request.timeout.ms` doesn't timeout connections properly and as a result it takes 127s to detect a connection failure. This sounds like a bug rather

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-05-19 Thread Cheng Tan
Dear Colin, Thanks for the reply. Your reasoning make sense. I’ve modified the KIP-601 with two changes: 1. Now KIP-601 is proposing a exponential connection setup timeout, which is controlled by socket.connections.setup.timeout.ms (init value) and socket.connections.setup.timeout.max.ms

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-05-18 Thread Colin McCabe
On Mon, May 18, 2020, at 14:41, Cheng Tan wrote: > Dear Colin, > > > Thanks for the suggestions. > > > For example, if a new node joins the cluster, it will have 0 failed connect > > attempts, whereas the existing nodes will probably have more than 0. So > > all the clients will ignore every

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-05-18 Thread Cheng Tan
Dear Colin, Thanks for the suggestions. > For example, if a new node joins the cluster, it will have 0 failed connect > attempts, whereas the existing nodes will probably have more than 0. So all > the clients will ignore every other node and pile on to the new one. That's > not good The

Re: [VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-05-18 Thread Colin McCabe
Hi Cheng, socket.connection.setup.timeout.ms seems more consistent with our existing configuration names than socket.connections.setup.timeout.ms (with an s). What do you think? > If no connected or connecting node exists, provide the disconnected node which > respects the reconnect backoff

[VOTE] KIP-601: Configurable socket connection timeout in NetworkClient

2020-05-15 Thread Cheng Tan
Hello developers, Big thanks for all the feedbacks. KIP-601 is finalized and ready for a vote. https://cwiki.apache.org/confluence/display/KAFKA/KIP-601%3A+Configurable+socket+connection+timeout+in+NetworkClient