ctubbsii commented on a change in pull request #2175:
URL: https://github.com/apache/accumulo/pull/2175#discussion_r657225211



##########
File path: 
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/delete/DeleteTable.java
##########
@@ -43,8 +43,13 @@ public DeleteTable(NamespaceId namespaceId, TableId tableId) 
{
   @Override
   public long isReady(long tid, Manager env) throws Exception {
 
-    // Before attempting to delete the table, cancel any running user
-    // compactions.
+    // If we have the table write lock, then it's possible that we have
+    // already canceled the compactions and we can skip to deleting the tables.
+    if (Utils.hasNamespaceReadLock(env, namespaceId, tid)
+        && Utils.hasTableWriteLock(env, tableId, tid)) {
+      return 0;
+    }
+    // Cancel any running user compactions before deleting table
     if (Utils.reserveNamespace(env, namespaceId, tid, false, true, 
TableOperation.COMPACT_CANCEL)

Review comment:
       > > Couldn't you just unlock in the call method before returning the 
DeleteTable repo?
   > 
   > To be clear, you are suggesting that we move the 
CancelCompactions.mutateZooKeeper() from it's current location in 
DeleteTable.isReady() to DeleteTable.call()?
   
   No.
   
   I was suggesting that instead of Keith's suggestion to have a whole 
PredeleteTableUnlock op just to unreserve the locks reserved in 
PredeleteTable.isReady, that his suggested PredeleteTable.call could do it 
itself before it returns `new DeleteTable(...)`




-- 
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:
[email protected]


Reply via email to