michaeljmarshall opened a new pull request, #19506: URL: https://github.com/apache/pulsar/pull/19506
Fixes #19431 ### Motivation The `originalAuthState` and the `authState` are not thread safe members of the `ServerCnx` class. As such, we should only access them on the context's event loop thread. ### Modifications * Replace scheduled task to check all connections for authentication refresh with a scheduled task per `ServerCnx` that will run on the appropriate event loop. This logic could theoretically be optimized to minimize scheduled tasks. However, I am following the keep alive implementation and making the assumption that the Netty scheduled tasks are not too expensive. * Slightly refactor conditional logic in `refreshAuthenticationCredentials` method to remove the logic that it only short circuits when the `ServerCnx` is in `Failed` state. We should never call this method when the `isActive` flag has gone to `false`, and the only case when the state isn't `Connected` is when we recently failed due to an exception. ### Verifying this change Added new test and make sure existing tests pass. Also, I added some new `assert` lines. These only run when the correct JVM arg is passed. ### Does this pull request potentially affect one of the following parts: This is not a breaking change. ### Documentation - [x] `doc-not-needed` This is an internal improvement. ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/28 -- 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]
