keith-turner commented on code in PR #5334:
URL: https://github.com/apache/accumulo/pull/5334#discussion_r1985594241


##########
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/CloneTable.java:
##########
@@ -34,23 +34,18 @@ public class CloneTable extends ManagerRepo {
   private static final long serialVersionUID = 1L;
   private final CloneInfo cloneInfo;
 
-  public CloneTable(String user, NamespaceId namespaceId, TableId srcTableId, 
String tableName,
-      Map<String,String> propertiesToSet, Set<String> propertiesToExclude, 
boolean keepOffline) {
-    cloneInfo = new CloneInfo();
-    cloneInfo.user = user;
-    cloneInfo.srcTableId = srcTableId;
-    cloneInfo.tableName = tableName;
-    cloneInfo.propertiesToExclude = propertiesToExclude;
-    cloneInfo.propertiesToSet = propertiesToSet;
-    cloneInfo.srcNamespaceId = namespaceId;
-    cloneInfo.keepOffline = keepOffline;
+  public CloneTable(String user, NamespaceId srcNamespaceId, TableId 
srcTableId,
+      NamespaceId namespaceId, String tableName, Map<String,String> 
propertiesToSet,
+      Set<String> propertiesToExclude, boolean keepOffline) {
+    cloneInfo = new CloneInfo(srcNamespaceId, srcTableId, namespaceId, 
tableName, propertiesToSet,
+        propertiesToExclude, keepOffline, user);
   }
 
   @Override
   public long isReady(long tid, Manager environment) throws Exception {
-    long val = Utils.reserveNamespace(environment, cloneInfo.srcNamespaceId, 
tid, false, true,
+    long val = Utils.reserveNamespace(environment, cloneInfo.getNamespaceId(), 
tid, false, true,
         TableOperation.CLONE);
-    val += Utils.reserveTable(environment, cloneInfo.srcTableId, tid, false, 
true,
+    val += Utils.reserveTable(environment, cloneInfo.getSrcTableId(), tid, 
false, true,

Review Comment:
   Looked into this a bit more and opened #5389.   If #5389 were fixed then it 
would be good if this code got a read lock on the source namespace.  Most other 
code will get the read lock on a namespace and then lock the table, it would be 
good to follow that pattern here w/ the soruce namespace planning that #5389 
will be fixed.



-- 
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-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to