DomGarguilo commented on code in PR #6433:
URL: https://github.com/apache/accumulo/pull/6433#discussion_r3436565192
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/next/InformationFetcher.java:
##########
@@ -792,26 +792,53 @@ private void fetchTabletInformation(SystemInformation
summary, UpdateTasks futur
public void run() {
long lastRunTime = 0;
+ final Timer noConnectionTimer = Timer.startNew();
+ final int clearStateMins = 10;
+ final Duration clearStateDuration = Duration.ofMinutes(clearStateMins);
+ final long minimumRefreshTimeMs = 5000;
while (true) {
+ // Only refresh internal data structure every 5s (old monitor logic).
+ while (NanoTime.millisElapsed(lastRunTime, NanoTime.now()) <
minimumRefreshTimeMs) {
+ LOG.trace("Waiting for the 5s refresh interval");
+ try {
+ Thread.sleep(250);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(
+ "Thread " + Thread.currentThread().getName() + " interrupted",
e);
+ }
+ }
+
// Don't fetch new data if there are no connections.
- // On an initial connection, no data may be displayed.
- // If a connection has not been made in a while, stale data may be
displayed.
- // Only refresh every 5s (old monitor logic).
- while (!newConnectionEvent.get() && connectionCount.get() == 0
- && NanoTime.millisElapsed(lastRunTime, NanoTime.now()) > 5000) {
+ // When summaryRef is not set, then the REST endpoint will wait
+ // until data is retrieved. summaryRef is not set on initial
+ // connection or when there has been no connection for 5 minutes
Review Comment:
```suggestion
// connection or when there has been no connection for the configured
duration
```
Looks like "5 minutes" is already stale
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/next/InformationFetcher.java:
##########
@@ -792,26 +792,53 @@ private void fetchTabletInformation(SystemInformation
summary, UpdateTasks futur
public void run() {
long lastRunTime = 0;
+ final Timer noConnectionTimer = Timer.startNew();
+ final int clearStateMins = 10;
+ final Duration clearStateDuration = Duration.ofMinutes(clearStateMins);
+ final long minimumRefreshTimeMs = 5000;
while (true) {
+ // Only refresh internal data structure every 5s (old monitor logic).
+ while (NanoTime.millisElapsed(lastRunTime, NanoTime.now()) <
minimumRefreshTimeMs) {
+ LOG.trace("Waiting for the 5s refresh interval");
+ try {
+ Thread.sleep(250);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(
+ "Thread " + Thread.currentThread().getName() + " interrupted",
e);
+ }
+ }
+
// Don't fetch new data if there are no connections.
- // On an initial connection, no data may be displayed.
- // If a connection has not been made in a while, stale data may be
displayed.
- // Only refresh every 5s (old monitor logic).
- while (!newConnectionEvent.get() && connectionCount.get() == 0
- && NanoTime.millisElapsed(lastRunTime, NanoTime.now()) > 5000) {
+ // When summaryRef is not set, then the REST endpoint will wait
+ // until data is retrieved. summaryRef is not set on initial
+ // connection or when there has been no connection for 5 minutes
+ noConnectionTimer.restart();
+ while (!newConnectionEvent.get() && connectionCount.get() == 0) {
+ LOG.trace("Waiting for a connection, connections: {}",
connectionCount.get());
try {
- Thread.sleep(100);
+ Thread.sleep(250);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new IllegalStateException(
"Thread " + Thread.currentThread().getName() + " interrupted",
e);
}
+ // If a connection has not been made in 5 minutes,
+ // then clear the summaryRef so that stale data is not displayed.
+ if (this.summaryRef.get() != null &&
noConnectionTimer.hasElapsed(clearStateDuration)) {
Review Comment:
```suggestion
if (noConnectionTimer.hasElapsed(clearStateDuration)) {
```
The code below returns the actual previous value anyways and `oldSummary !=
null` handles whether to clear or not already.
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/next/InformationFetcher.java:
##########
@@ -792,26 +792,53 @@ private void fetchTabletInformation(SystemInformation
summary, UpdateTasks futur
public void run() {
long lastRunTime = 0;
+ final Timer noConnectionTimer = Timer.startNew();
+ final int clearStateMins = 10;
+ final Duration clearStateDuration = Duration.ofMinutes(clearStateMins);
+ final long minimumRefreshTimeMs = 5000;
while (true) {
+ // Only refresh internal data structure every 5s (old monitor logic).
+ while (NanoTime.millisElapsed(lastRunTime, NanoTime.now()) <
minimumRefreshTimeMs) {
+ LOG.trace("Waiting for the 5s refresh interval");
+ try {
+ Thread.sleep(250);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(
+ "Thread " + Thread.currentThread().getName() + " interrupted",
e);
+ }
+ }
+
// Don't fetch new data if there are no connections.
- // On an initial connection, no data may be displayed.
- // If a connection has not been made in a while, stale data may be
displayed.
- // Only refresh every 5s (old monitor logic).
- while (!newConnectionEvent.get() && connectionCount.get() == 0
- && NanoTime.millisElapsed(lastRunTime, NanoTime.now()) > 5000) {
+ // When summaryRef is not set, then the REST endpoint will wait
+ // until data is retrieved. summaryRef is not set on initial
+ // connection or when there has been no connection for 5 minutes
+ noConnectionTimer.restart();
+ while (!newConnectionEvent.get() && connectionCount.get() == 0) {
+ LOG.trace("Waiting for a connection, connections: {}",
connectionCount.get());
try {
- Thread.sleep(100);
+ Thread.sleep(250);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new IllegalStateException(
"Thread " + Thread.currentThread().getName() + " interrupted",
e);
}
+ // If a connection has not been made in 5 minutes,
Review Comment:
```suggestion
// If a connection has not been made in the configured duration,
```
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/next/InformationFetcher.java:
##########
@@ -792,26 +792,53 @@ private void fetchTabletInformation(SystemInformation
summary, UpdateTasks futur
public void run() {
long lastRunTime = 0;
+ final Timer noConnectionTimer = Timer.startNew();
+ final int clearStateMins = 10;
+ final Duration clearStateDuration = Duration.ofMinutes(clearStateMins);
Review Comment:
```suggestion
final Duration clearStateDuration = Duration.ofMinutes(10);
```
Doesn't look like `clearStateMins` is used elsewhere (except in a comment
where we could use `clearStateDuration.toMinutes()` instead). could just remove
it.
--
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]
