You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ctubbsii (via GitHub)" <gi...@apache.org> on 2023/05/16 17:15:52 UTC
[GitHub] [accumulo] ctubbsii opened a new pull request, #3403: Set finished time for GC cycles even if Exception
ctubbsii opened a new pull request, #3403:
URL: https://github.com/apache/accumulo/pull/3403
Always set the finished time for GC cycles, even if an exception occurred in the cycle. This prevents a GCStatus object containing a started time, but the finished time staying the default value of 0.
Ideally, this wouldn't be a problem if we used the setter/getter methods on Thrift objects so that we can tell the difference between a 0 that has been set and one that is 0 just because it's a primitive type that defaults to 0 when unset. Thrift tracks which fields are set using isSetFieldname methods, but since we're not consistently using those, and just grabbing the value of the field without checking if it has been set, we need to make sure that we've set it to something sensible.
I believe this fixes #3374
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #3403: Set finished time for GC cycles even if Exception
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on PR #3403:
URL: https://github.com/apache/accumulo/pull/3403#issuecomment-1552185828
Should be fixed with that latest commit.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #3403: Set finished time for GC cycles even if Exception
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on PR #3403:
URL: https://github.com/apache/accumulo/pull/3403#issuecomment-1551872773
Looks like this might not be a complete fix. I was able to reproduce the error by killing a tserver, and the error occurred in the Manager, not the GC. So, I think there's a place I missed.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii merged pull request #3403: Set finished time for GC cycles even if Exception
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii merged PR #3403:
URL: https://github.com/apache/accumulo/pull/3403
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #3403: Set finished time for GC cycles even if Exception
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on PR #3403:
URL: https://github.com/apache/accumulo/pull/3403#issuecomment-1551972166
After further investigation, it seems this will fix the issue where the finished state is not updated (and stays 0) and the cycle stats aren't updated, if there's an exception that occurred during the cycle.
This does not yet fix the situation where the current cycle is not yet finished, and has not set a finished time. In those cases, we set ManagerInformation to have a "Running" label for the gcStatus (it can also take a value of "Down" or "Waiting", but it doesn't appear to be used anywhere, and appears to be different than the StatusInformation resource object, which also has a gcStatus, but uses items like "OK" and "ERROR" instead, and power some specific visual items in the monitor.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3403: Set finished time for GC cycles even if Exception
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #3403:
URL: https://github.com/apache/accumulo/pull/3403#discussion_r1195472077
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/rest/gc/GarbageCollectorStats.java:
##########
@@ -52,6 +52,6 @@ public GarbageCollectorStats(String type, GcCycleStats thriftStats) {
this.inUse = thriftStats.inUse;
this.deleted = thriftStats.deleted;
this.errors = thriftStats.errors;
- this.duration = this.finished - thriftStats.started;
+ this.duration = thriftStats.finished - thriftStats.started;
Review Comment:
This change isn't a functional one... it just makes it more clear that they're both coming from the same source when computing the difference.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org