wardlican commented on code in PR #3950:
URL: https://github.com/apache/amoro/pull/3950#discussion_r3007252077
##########
amoro-optimizer/amoro-optimizer-common/src/main/java/org/apache/amoro/optimizer/common/AbstractOptimizerOperator.java:
##########
@@ -86,25 +157,79 @@ private boolean shouldReturnNull(Throwable t) {
return false;
}
- protected <T> T callAuthenticatedAms(AmsAuthenticatedCallOperation<T>
operation)
+ /**
+ * Call authenticated AMS with a specific AMS URL.
+ *
+ * @param amsUrl The AMS node URL to call
+ * @param operation The operation to execute
+ * @return The result of the operation
+ * @throws TException If the operation fails
+ */
+ protected <T> T callAuthenticatedAms(String amsUrl,
AmsAuthenticatedCallOperation<T> operation)
throws TException {
+ // Maximum retry time window for auth errors in master-slave mode (30
seconds)
+ long maxAuthRetryTimeWindow = TimeUnit.SECONDS.toMillis(90);
+ Long firstAuthErrorTime = null;
+
while (isStarted()) {
if (tokenIsReady()) {
String token = getToken();
try {
- return
operation.call(OptimizingClientPools.getClient(config.getAmsUrl()), token);
+ // Reset retry time on successful call
+ firstAuthErrorTime = null;
+ return operation.call(OptimizingClientPools.getClient(amsUrl),
token);
Review Comment:
Resetting the timer before every invocation causes the `elapsedTime >=
maxAuthRetryTimeWindow` branch to never trigger, meaning `resetToken` is never
executed. Consequently, the optimizer will retry indefinitely in master-slave
mode and will be unable to recover in the event of a genuine authentication
failure.
When the method returns successfully, `firstAuthErrorTime` acts as a local
variable that is destroyed upon method exit; therefore, it requires no manual
reset whatsoever. The correct fix is simply to delete these two lines:
--
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]
