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

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

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

 ##########
 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:
   I think these two lines are all you need to do for this change... make a copy and return. 
   
   It seems like all you need is an immutable copy of the GCStatus object, which can be done without any locking.  The locking is not needed and just adds complexity.  No matter what you do, you will only ever be able to get a snapshot of the GC at a moment in time and we want to avoid making too many changes to 1.9.
   
   You could just add a separate method like ```getSnapshot()``` which makes a copy of status.  This would make a merge forward easier as well.

----------------------------------------------------------------
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