jinmeiliao commented on a change in pull request #6835:
URL: https://github.com/apache/geode/pull/6835#discussion_r710557094
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcher.java
##########
@@ -362,31 +387,67 @@ protected void runDispatcher() {
}
waitForResumption();
}
- try {
- clientMessage = (ClientMessage) _messageQueue.peek();
- } catch (RegionDestroyedException skipped) {
- break;
+
+ // if message is not delivered due to authentication expiation, this
clientMessage
+ // would not be null.
+ if (clientMessage == null) {
+ try {
+ clientMessage = (ClientMessage) _messageQueue.peek();
+ } catch (RegionDestroyedException skipped) {
+ break;
+ }
}
+
getStatistics().setQueueSize(_messageQueue.size());
if (isStopped()) {
break;
}
- if (clientMessage != null) {
- // Process the message
- long start = getStatistics().startTime();
- //// BUGFIX for BUG#38206 and BUG#37791
- boolean isDispatched = dispatchMessage(clientMessage);
- getStatistics().endMessage(start);
- if (isDispatched) {
+
+ if (clientMessage == null) {
+ _messageQueue.remove();
+ continue;
+ }
+
+ // Process the message
+ long start = getStatistics().startTime();
+ try {
+ if (dispatchMessage(clientMessage)) {
+ getStatistics().endMessage(start);
_messageQueue.remove();
if (clientMessage instanceof ClientMarkerMessageImpl) {
getProxy().setMarkerEnqueued(false);
}
}
- } else {
+ clientMessage = null;
+ wait_for_re_auth_start_time = -1;
+ } catch (NotAuthorizedException notAuthorized) {
+ // behave as if the message is dispatched, remove from the queue
+ logger.info("skip delivering message: " + clientMessage,
notAuthorized);
_messageQueue.remove();
+ clientMessage = null;
+ } catch (AuthenticationExpiredException expired) {
+ if (wait_for_re_auth_start_time == -1) {
+ wait_for_re_auth_start_time = System.currentTimeMillis();
+ // only send the message to clients who can handle the message
+ if
(getProxy().getVersion().isNewerThanOrEqualTo(RE_AUTHENTICATION_START_VERSION))
{
+ EventID eventId = createEventId();
+ sendMessageDirectly(new ClientReAuthenticateMessage(eventId));
+ }
+ // for older client, we still wait, just in case client will
perform some operations to
+ // trigger credential refresh on its own.
+ Thread.sleep(200);
+ } else {
+ long elapsedTime = System.currentTimeMillis() -
wait_for_re_auth_start_time;
+ if (elapsedTime > reAuthenticateWaitTime) {
+ logger.warn("Client did not re-authenticate back successfully in
" + elapsedTime
+ + "ms. Unregister this client proxy.");
+ pauseOrUnregisterProxy(expired);
+ } else {
+ Thread.sleep(200);
Review comment:
> Thanks for that explanation. It helps a lot.
>
> Whenever the dispatcher catches an `AuthExpirationException`, if it
doesn't terminate the connection, all message processing is paused for 200ms.
>
> I'm wondering how often does authentication expire generally? The product
doesn't ship with a security manager that implements expiration (other than a
test class), so it would be up to each particular customer's security manager
implementation right?
>
> Do you think it's ok to sleep for 200ms each time or do you think it would
be worth the effort to shorten that time window by introducing say, a latch
(we'd do an `await()` at the points where we're currently sleeping, and we'd
issue a `countDown()` at the point where the client re-authenticates). **Is
there an obvious place in the product where we could issue that `countDown()`?**
>
> PS: In evaluating this change I looked for how
`ClientReAuthenticateMessage` is used but I don't see it used anywhere in the
product except where it's thrown in this class (and registered as a DSFID).
That leads me to believe that a user's security manager would have to register
a handler for that message. `ClientReAuthenticateMessage` is not mentioned in
the product documentation nor in examples so I'm just guessing here.
Like I replied to Anil, there is not a definite event that the server needs
to "wait" for, the client may never re-authenticate back, so all the server
can rely on is to keep trying to deliver the message with the "subject" it
knows about (with 200 ms interval) for 5 seconds (configurable by system
property).
--
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]
