ctubbsii commented on code in PR #4380:
URL: https://github.com/apache/accumulo/pull/4380#discussion_r1934549741
##########
server/base/src/main/java/org/apache/accumulo/server/mem/LowMemoryDetector.java:
##########
@@ -173,25 +176,24 @@ public void logGCInfo(AccumuloConfiguration conf) {
LOG.debug(sb.toString());
}
- final long keepAliveTimeout =
conf.getTimeInMillis(Property.INSTANCE_ZK_TIMEOUT);
- if (lastMemoryCheckTime > 0 && lastMemoryCheckTime < now) {
- final long diff = now - lastMemoryCheckTime;
- if (diff > keepAliveTimeout + 1000) {
+ final Duration keepAliveTimeout =
conf.getDuration(Property.INSTANCE_ZK_TIMEOUT);
+ if (lastMemoryCheckTime != null &&
lastMemoryCheckTime.hasElapsed(keepAliveTimeout)) {
+ if
(currentTimer.hasElapsed(keepAliveTimeout.plus(Duration.ofSeconds(1)))) {
LOG.warn(String.format(
"GC pause checker not called in a timely"
+ " fashion. Expected every %.1f seconds but was %.1f
seconds since last check",
- keepAliveTimeout / 1000., diff / 1000.));
+ keepAliveTimeout.toMillis() / 1000.,
currentTimer.elapsed(MILLISECONDS) / 1000.));
}
- lastMemoryCheckTime = now;
+ lastMemoryCheckTime = currentTimer;
return;
}
- if (maxIncreaseInCollectionTime > keepAliveTimeout) {
+ if (maxIncreaseInCollectionTime > keepAliveTimeout.toMillis()) {
Halt.halt("Garbage collection may be interfering with lock keep-alive.
Halting.", -1);
}
lastMemorySize = freeMemory;
- lastMemoryCheckTime = now;
+ lastMemoryCheckTime = currentTimer;
Review Comment:
Do you know if there's a possibility of using reset for this, instead of
replacing the timer? It seems odd to replace the timer.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1769,13 +1772,13 @@ public void importDataFiles(long tid,
Map<ReferencedTabletFile,DataFileInfo> fil
// Clients timeout and will think that this operation failed.
// Don't do it if we spent too long waiting for the lock
- long now = System.nanoTime();
+ Timer lockWaitTimer = Timer.startNew();
synchronized (this) {
if (isClosed()) {
throw new IOException("tablet " + extent + " is closed");
}
- long rpcTimeoutNanos = TimeUnit.MILLISECONDS.toNanos(
+ Duration rpcTimeout = Duration.ofMillis(
Review Comment:
This calls getTimeInMillis and then creates a Duration using
Duration.ofMillis, but I think AccumuloConfiguration can just do a getDuration
now.
--
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]
