Hi, Ron, Thanks for the KIP. Looks good to me overall. So, +1 assuming the following minor comments will be addressed.
100. connections.max.reauth.ms: A client can authenticate with the broker using SSL. This config has no impact on such connections. It would be useful to make it clear in the documentation. Also, in this case, I guess the broker won't kill the SSL connection after connections.max.reauth.ms? 101. "If re-authentication succeeds then any requests that queued up during re-authentication will subsequently be able to flow through, and eventually the connection will re-authenticate again, etc.". This is more of an implementation detail. I guess the proposal is to queue up new requests in the client when there is is pending re-authentication. An alternative is to mark the Channel unavailable for send during re-authentication. This has the slight benefit of reducing the client memory footprint. 102. "A client-side metric will be created that documents the latency imposed by re-authentication." What's the name of this metric? Does it measure avg or max? 103. "Upgrade all brokers to v2.1.0 or later at whatever rate is desired with 'connections.max.reauth.ms' allowed to default to 0. If SASL is used for the inter-broker protocol then brokers will check the SASL_AUTHENTICATE API version and use a V1 request when communicating to a broker that has been upgraded to 2.1.0, but the client will see the "0" session max lifetime and will not re-authenticate. ". Currently, for the inter broker usage of NetworkClient (ReplicaFetcherThread, ControllerChannelManager, etc), the broker version discovery logic is actually disabled and the client is expected to use the new version of the request after inter.broker.protocol.version is set to the current version. So, we will need to rely on this for deciding whether the NetworkClient should use the re-authenticate request or not, during upgrade. Jun On Mon, Sep 24, 2018 at 4:39 PM, Ron Dagostino <rndg...@gmail.com> wrote: > Still looking for a final +1 binding vote to go with the 9 votes so far (2 > binding, 7 non-binding). > > Ron > > > On Sep 24, 2018, at 3:53 PM, Ron Dagostino <rndg...@gmail.com> wrote: > > > > **Please vote** . It's getting late in the day and this KIP still > requires 1 more binding up-vote to be considered for the 2.1.0 release. > > > > The current vote is 2 binding +1 votes (Rajini and Harsha) and 7 > non-binding +1 votes (myself, Mike, Konstantin, Boerge, Edoardo, Stanislav, > and Mickael). > > > > Ron > > > >> On Mon, Sep 24, 2018 at 9:47 AM Ron Dagostino <rndg...@gmail.com> > wrote: > >> Hi Everyone. This KIP still requires 1 more binding up-vote to be > considered for the 2.1.0 release. **Please vote before today's end-of-day > deadline.** > >> > >> The current vote is 2 binding +1 votes (Rajini and Harsha) and 7 > non-binding +1 votes (myself, Mike, Konstantin, Boerge, Edoardo, Stanislav, > and Mickael). > >> > >> Ron > >> > >>> On Fri, Sep 21, 2018 at 11:48 AM Mickael Maison < > mickael.mai...@gmail.com> wrote: > >>> +1 (non-binding) > >>> Thanks for the KIP, this is a very nice feature. > >>> On Fri, Sep 21, 2018 at 4:56 PM Stanislav Kozlovski > >>> <stanis...@confluent.io> wrote: > >>> > > >>> > Thanks for the KIP, Ron! > >>> > +1 (non-binding) > >>> > > >>> > On Fri, Sep 21, 2018 at 5:26 PM Ron Dagostino <rndg...@gmail.com> > wrote: > >>> > > >>> > > Hi Everyone. This KIP requires 1 more binding up-vote to be > considered for > >>> > > the 2.1.0 release; please vote before the Monday deadline. > >>> > > > >>> > > The current vote is 2 binding +1 votes (Rajini and Harsha) and 5 > >>> > > non-binding +1 votes (myself, Mike, Konstantin, Boerge, and > Edoardo). > >>> > > > >>> > > Ron > >>> > > > >>> > > On Wed, Sep 19, 2018 at 12:40 PM Harsha <ka...@harsha.io> wrote: > >>> > > > >>> > > > KIP looks good. +1 (binding) > >>> > > > > >>> > > > Thanks, > >>> > > > Harsha > >>> > > > > >>> > > > On Wed, Sep 19, 2018, at 7:44 AM, Rajini Sivaram wrote: > >>> > > > > Hi Ron, > >>> > > > > > >>> > > > > Thanks for the KIP! > >>> > > > > > >>> > > > > +1 (binding) > >>> > > > > > >>> > > > > On Tue, Sep 18, 2018 at 6:24 PM, Konstantin Chukhlomin < > >>> > > > chuhlo...@gmail.com> > >>> > > > > wrote: > >>> > > > > > >>> > > > > > +1 (non binding) > >>> > > > > > > >>> > > > > > > On Sep 18, 2018, at 1:18 PM, michael.kamin...@nytimes.com > wrote: > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > On 2018/09/18 14:59:09, Ron Dagostino <rndg...@gmail.com> > wrote: > >>> > > > > > >> Hi everyone. I would like to start the vote for KIP-368: > >>> > > > > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >>> > > > > > 368%3A+Allow+SASL+Connections+to+Periodically+Re- > Authenticate > >>> > > > > > >> > >>> > > > > > >> This KIP proposes adding the ability for SASL clients > (and brokers > >>> > > > when > >>> > > > > > a > >>> > > > > > >> SASL mechanism is the inter-broker protocol) to > re-authenticate > >>> > > > their > >>> > > > > > >> connections to brokers and for brokers to close > connections that > >>> > > > > > continue > >>> > > > > > >> to use expired sessions. > >>> > > > > > >> > >>> > > > > > >> Ron > >>> > > > > > >> > >>> > > > > > > > >>> > > > > > > +1 (non binding) > >>> > > > > > > >>> > > > > > > >>> > > > > >>> > > > >>> > > >>> > > >>> > -- > >>> > Best, > >>> > Stanislav >
