ddanielr commented on code in PR #6378:
URL: https://github.com/apache/accumulo/pull/6378#discussion_r3282953557


##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -496,15 +505,19 @@ public MergeInfo getMergeInfo(TableId tableId) {
         log.warn("Unexpected error reading merge state", ex);
         return new MergeInfo();
       }
+    } finally {
+      l.unlock();
     }
   }
 
   public void setMergeState(MergeInfo info, MergeState state)
       throws KeeperException, InterruptedException {
     ServerContext context = getContext();
-    synchronized (mergeLock) {
-      String path =
-          getZooKeeperRoot() + Constants.ZTABLES + "/" + 
info.getExtent().tableId() + "/merge";
+    final TableId tid = info.getExtent().tableId();

Review Comment:
   This code looks like we might hit a NPE on the `.tableId()` call if the 
extent doesn't exist.
   
   MergeInfo has a no-args constructor that does not set the extent. 
   Instead the extent is then set via  a `MergeInfo.readFields()` call.
   
   However for all calls of `setMergeState` the extent or state of `MergeInfo` 
is checked so this doesn't seem to be an issue.



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

Reply via email to