sodonnel commented on PR #11789:
URL: https://github.com/apache/ozone/pull/11789#issuecomment-6441534947
Thanks for working on this change. The change is logically correct, but I am
a little concerned about all the threads stalling when the refresh is in
progress, rather than letting one thread refresh and all others just continue
with the old value until the refresh is complete. A better approach with no
locking using Compare and Set would be like:
```
private static final long REFRESHING = -1L;
private volatile OzoneManagerVersion omVersion;
private final AtomicLong omVersionLastUpdate = new
AtomicLong(Time.monotonicNow());
private OzoneManagerVersion getOmVersion() {
long lastUpdate = omVersionLastUpdate.get();
if (lastUpdate == REFRESHING
|| Time.monotonicNow() - lastUpdate <
OM_VERSION_REFRESH_INTERVAL) {
return omVersion; // refresh in progress, or cache is fresh
}
// CAS: only one thread wins; all others return immediately above
if (omVersionLastUpdate.compareAndSet(lastUpdate, REFRESHING)) {
try {
omVersion = getOmVersion(ozoneManagerClient.getServiceInfo());
} catch (IOException e) {
LOG.warn("Could not refresh OM version; retaining {}.",
omVersion, e);
} finally {
omVersionLastUpdate.set(Time.monotonicNow());
}
}
return omVersion;
}
```
This will cause only 1 thread to stall, while the others return the old
value in the first IF statement or fall through to the return of the value at
the bottom of the method.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
