showuon commented on code in PR #12179:
URL: https://github.com/apache/kafka/pull/12179#discussion_r888827862


##########
clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java:
##########
@@ -679,10 +679,11 @@ private long 
calcCompletionTimesAndReturnSessionLifetimeMs() {
                 else if (connectionsMaxReauthMs == null)
                     retvalSessionLifetimeMs = 
zeroIfNegative(credentialExpirationMs - authenticationEndMs);
                 else
-                    retvalSessionLifetimeMs = zeroIfNegative(
-                            Math.min(credentialExpirationMs - 
authenticationEndMs, connectionsMaxReauthMs));
+                    retvalSessionLifetimeMs = 
zeroIfNegative(Math.min(credentialExpirationMs - authenticationEndMs, 
connectionsMaxReauthMs));
 
-                sessionExpirationTimeNanos = authenticationEndNanos + 1000 * 
1000 * retvalSessionLifetimeMs;
+                if (connectionsMaxReauthMs != null) {

Review Comment:
   > While there is the Authenticator interface where comments suggest that 
#serverSessionExpirationTimeNanos should be left as null when re-authentication 
is "disabled"
   
   Is 
[this](https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/network/Authenticator.java#L90-L102)
 javadoc what you mean? From the javadoc, I don't see it said we should return 
null for re-auth disabled. I think it's OK to return the value when re-auth 
disabled.
   
   > here are some clients or SASL mechanisms where we don't expect 
reauthentication to ever happen?
   
   Yes, it is. You can check 
[this](https://github.com/edenhill/librdkafka/issues/3304) for reference.
   
   Thanks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to