[GitHub] [accumulo] EdColeman commented on a change in pull request #1360: Improve concurrent access to gc statistics.

[GitBox <gi...@apache.org>]

EdColeman commented on a change in pull request #1360: Improve concurrent access to gc statistics.
URL: https://github.com/apache/accumulo/pull/1360#discussion_r324403386
 
 

 ##########
 File path: server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
 ##########
 @@ -865,6 +897,12 @@ static boolean isDir(String delete) {
 
   @Override
   public GCStatus getStatus(TInfo info, TCredentials credentials) {
-    return status;
+    statusLock.lock();
+    try {
+      GCStatus copy = new GCStatus(status);
 
 Review comment:
   The code that I think that also needs to lock is when the "last run" values are set to the current values and the zero'ed for the next run.
   `   status.lastLog = status.currentLog;
        status.currentLog = new GcCycleStats();`
   
   The rest of the locking was to maintain current behavior - and to make it consistent and correct for places where the current code is not locking.  It seemed odd to have portions that are thread safe and other portions that require single-thread access for correct behavior. This seemed a better practice, especially when having access to the snapshot is done from other thread(s)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services