ACCUMULO-802 fixed deadlock problem with the table namespace locks during fate operations
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/c2ed43e4 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/c2ed43e4 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/c2ed43e4 Branch: refs/heads/master Commit: c2ed43e4b58cc2278b9955fc36ad9ba788a171c2 Parents: e1cf746 Author: Sean Hickey <tallirishll...@gmail.com> Authored: Tue Aug 13 15:10:51 2013 -0400 Committer: Christopher Tubbs <ctubb...@apache.org> Committed: Wed Dec 4 18:46:10 2013 -0500 ---------------------------------------------------------------------- .../java/org/apache/accumulo/master/Master.java | 13 +++++++++---- .../accumulo/master/tableOps/CloneTable.java | 18 ++++++++++-------- .../accumulo/master/tableOps/CreateTable.java | 5 +++-- .../accumulo/master/tableOps/DeleteTable.java | 10 ++++++---- .../accumulo/master/tableOps/RenameTable.java | 10 +++++----- .../test/randomwalk/concurrent/Setup.java | 2 +- .../system/randomwalk/conf/modules/Concurrent.xml | 2 +- 7 files changed, 35 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/c2ed43e4/server/master/src/main/java/org/apache/accumulo/master/Master.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/Master.java b/server/master/src/main/java/org/apache/accumulo/master/Master.java index 0af32d4..338550a 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/Master.java +++ b/server/master/src/main/java/org/apache/accumulo/master/Master.java @@ -42,6 +42,7 @@ import org.apache.accumulo.core.client.IsolatedScanner; import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.client.RowIterator; import org.apache.accumulo.core.client.Scanner; +import org.apache.accumulo.core.client.TableNamespaceNotFoundException; import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.client.admin.TableOperationsImpl; import org.apache.accumulo.core.client.admin.TimeType; @@ -927,7 +928,11 @@ public class Master implements LiveTServerSet.Listener, TableObserver, CurrentSt TimeType timeType = TimeType.valueOf(ByteBufferUtil.toString(arguments.get(1))); - fate.seedTransaction(opid, new TraceRepo<Master>(new CreateTable(c.getPrincipal(), tableName, timeType, options)), autoCleanup); + try { + fate.seedTransaction(opid, new TraceRepo<Master>(new CreateTable(c.getPrincipal(), tableName, timeType, options, getInstance())), autoCleanup); + } catch (TableNamespaceNotFoundException e) { + throw new ThriftTableOperationException(null, tableName, TableOperation.CREATE, TableOperationExceptionType.NOTFOUND, e.getMessage()); + } break; } case RENAME: { @@ -941,7 +946,7 @@ public class Master implements LiveTServerSet.Listener, TableObserver, CurrentSt if (!security.canRenameTable(c, tableId, oldTableName, newTableName)) throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); - fate.seedTransaction(opid, new TraceRepo<Master>(new RenameTable(tableId, oldTableName, newTableName)), autoCleanup); + fate.seedTransaction(opid, new TraceRepo<Master>(new RenameTable(tableId, oldTableName, newTableName, getInstance())), autoCleanup); break; } @@ -970,7 +975,7 @@ public class Master implements LiveTServerSet.Listener, TableObserver, CurrentSt propertiesToSet.put(entry.getKey(), entry.getValue()); } - fate.seedTransaction(opid, new TraceRepo<Master>(new CloneTable(c.getPrincipal(), srcTableId, tableName, propertiesToSet, propertiesToExclude)), + fate.seedTransaction(opid, new TraceRepo<Master>(new CloneTable(c.getPrincipal(), srcTableId, tableName, propertiesToSet, propertiesToExclude, getInstance())), autoCleanup); break; @@ -981,7 +986,7 @@ public class Master implements LiveTServerSet.Listener, TableObserver, CurrentSt checkNotMetadataTable(tableName, TableOperation.DELETE); if (!security.canDeleteTable(c, tableId)) throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); - fate.seedTransaction(opid, new TraceRepo<Master>(new DeleteTable(tableId)), autoCleanup); + fate.seedTransaction(opid, new TraceRepo<Master>(new DeleteTable(tableId, getInstance())), autoCleanup); break; } case ONLINE: { http://git-wip-us.apache.org/repos/asf/accumulo/blob/c2ed43e4/server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTable.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTable.java b/server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTable.java index 31e5e3e..46edc25 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTable.java +++ b/server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneTable.java @@ -139,9 +139,10 @@ class CloneZookeeper extends MasterRepo { @Override public long isReady(long tid, Master environment) throws Exception { cloneInfo.namespaceId = TableNamespaces.getNamespaceId(environment.getInstance(), Tables.extractNamespace(cloneInfo.tableName)); - long val = Utils.reserveTable(cloneInfo.tableId, tid, true, false, TableOperation.CLONE); - if (!cloneInfo.srcNamespaceId.equals(cloneInfo.namespaceId)) + long val = 0; + if (!cloneInfo.srcNamespaceId.equals(cloneInfo.namespaceId)) val += Utils.reserveTableNamespace(cloneInfo.namespaceId, tid, false, true, TableOperation.CLONE); + val += Utils.reserveTable(cloneInfo.tableId, tid, true, false, TableOperation.CLONE); return val; } @@ -157,7 +158,7 @@ class CloneZookeeper extends MasterRepo { TableManager.getInstance().cloneTable(cloneInfo.srcTableId, cloneInfo.tableId, cloneInfo.tableName, cloneInfo.propertiesToSet, cloneInfo.propertiesToExclude, NodeExistsPolicy.OVERWRITE); Tables.clearCache(instance); - + TableManager.getInstance().addNamespaceToTable(cloneInfo.tableId, cloneInfo.namespaceId); return new CloneMetadata(cloneInfo); @@ -171,7 +172,7 @@ class CloneZookeeper extends MasterRepo { Instance instance = HdfsZooInstance.getInstance(); TableManager.getInstance().removeTable(cloneInfo.tableId); Utils.unreserveTable(cloneInfo.tableId, tid, true); - if (!cloneInfo.namespaceId.equals(cloneInfo.srcNamespaceId)) + if (!cloneInfo.namespaceId.equals(cloneInfo.srcNamespaceId)) Utils.unreserveTableNamespace(cloneInfo.namespaceId, tid, false); Tables.clearCache(instance); } @@ -223,20 +224,21 @@ public class CloneTable extends MasterRepo { private static final long serialVersionUID = 1L; private CloneInfo cloneInfo; - public CloneTable(String user, String srcTableId, String tableName, Map<String,String> propertiesToSet, Set<String> propertiesToExclude) { + public CloneTable(String user, String srcTableId, String tableName, Map<String,String> propertiesToSet, Set<String> propertiesToExclude, Instance inst) { cloneInfo = new CloneInfo(); cloneInfo.user = user; cloneInfo.srcTableId = srcTableId; cloneInfo.tableName = tableName; cloneInfo.propertiesToExclude = propertiesToExclude; cloneInfo.propertiesToSet = propertiesToSet; + cloneInfo.srcNamespaceId = Tables.getNamespace(inst, cloneInfo.srcTableId); } @Override public long isReady(long tid, Master environment) throws Exception { - cloneInfo.srcNamespaceId = Tables.getNamespace(environment.getInstance(), cloneInfo.srcTableId); - long val = Utils.reserveTable(cloneInfo.srcTableId, tid, false, true, TableOperation.CLONE); - val += Utils.reserveTableNamespace(cloneInfo.srcNamespaceId, tid, false, true, TableOperation.CLONE); + + long val = Utils.reserveTableNamespace(cloneInfo.srcNamespaceId, tid, false, true, TableOperation.CLONE); + val += Utils.reserveTable(cloneInfo.srcTableId, tid, false, true, TableOperation.CLONE); return val; } http://git-wip-us.apache.org/repos/asf/accumulo/blob/c2ed43e4/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java b/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java index 217bfda..f657a1b 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java +++ b/server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java @@ -22,6 +22,7 @@ import java.util.Map.Entry; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.client.Instance; +import org.apache.accumulo.core.client.TableNamespaceNotFoundException; import org.apache.accumulo.core.client.admin.TimeType; import org.apache.accumulo.core.client.impl.TableNamespaces; import org.apache.accumulo.core.client.impl.Tables; @@ -279,18 +280,18 @@ public class CreateTable extends MasterRepo { private TableInfo tableInfo; - public CreateTable(String user, String tableName, TimeType timeType, Map<String,String> props) { + public CreateTable(String user, String tableName, TimeType timeType, Map<String,String> props, Instance inst) throws TableNamespaceNotFoundException { tableInfo = new TableInfo(); tableInfo.tableName = tableName; tableInfo.timeType = TabletTime.getTimeID(timeType); tableInfo.user = user; tableInfo.props = props; + tableInfo.namespaceId = TableNamespaces.getNamespaceId(inst, Tables.extractNamespace(tableInfo.tableName)); } @Override public long isReady(long tid, Master environment) throws Exception { // reserve the table's namespace to make sure it doesn't change while the table is created - tableInfo.namespaceId = TableNamespaces.getNamespaceId(environment.getInstance(), Tables.extractNamespace(tableInfo.tableName)); return Utils.reserveTableNamespace(tableInfo.namespaceId, tid, false, true, TableOperation.CREATE); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/c2ed43e4/server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTable.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTable.java b/server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTable.java index 439d0c2..98d1ae9 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTable.java +++ b/server/master/src/main/java/org/apache/accumulo/master/tableOps/DeleteTable.java @@ -22,6 +22,7 @@ import java.util.Map.Entry; import org.apache.accumulo.core.client.BatchScanner; import org.apache.accumulo.core.client.Connector; +import org.apache.accumulo.core.client.Instance; import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.impl.Tables; @@ -220,15 +221,16 @@ public class DeleteTable extends MasterRepo { private String tableId, namespaceId; - public DeleteTable(String tableId) { + public DeleteTable(String tableId, Instance inst) { this.tableId = tableId; + this.namespaceId = Tables.getNamespace(inst, tableId); } @Override public long isReady(long tid, Master environment) throws Exception { - this.namespaceId = Tables.getNamespace(environment.getInstance(), tableId); - return Utils.reserveTable(tableId, tid, true, true, TableOperation.DELETE) - + Utils.reserveTableNamespace(namespaceId, tid, false, false, TableOperation.CREATE); + + return Utils.reserveTableNamespace(namespaceId, tid, false, false, TableOperation.DELETE) + + Utils.reserveTable(tableId, tid, true, true, TableOperation.DELETE); } @Override http://git-wip-us.apache.org/repos/asf/accumulo/blob/c2ed43e4/server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java ---------------------------------------------------------------------- diff --git a/server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java b/server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java index 8b82593..871c0a0 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java +++ b/server/master/src/main/java/org/apache/accumulo/master/tableOps/RenameTable.java @@ -42,15 +42,15 @@ public class RenameTable extends MasterRepo { @Override public long isReady(long tid, Master environment) throws Exception { - this.namespaceId = Tables.getNamespace(environment.getInstance(), tableId); - return Utils.reserveTable(tableId, tid, true, true, TableOperation.RENAME) - + Utils.reserveTableNamespace(namespaceId, tid, false, true, TableOperation.RENAME); + return Utils.reserveTableNamespace(namespaceId, tid, false, true, TableOperation.RENAME) + + Utils.reserveTable(tableId, tid, true, true, TableOperation.RENAME); } - public RenameTable(String tableId, String oldTableName, String newTableName) { + public RenameTable(String tableId, String oldTableName, String newTableName, Instance inst) { this.tableId = tableId; this.oldTableName = oldTableName; this.newTableName = newTableName; + this.namespaceId = Tables.getNamespace(inst, tableId); } @Override @@ -96,7 +96,7 @@ public class RenameTable extends MasterRepo { } finally { Utils.tableNameLock.unlock(); Utils.unreserveTable(tableId, tid, true); - Utils.unreserveTableNamespace(namespaceId, tid, false); + Utils.unreserveTableNamespace(this.namespaceId, tid, false); } Logger.getLogger(RenameTable.class).debug("Renamed table " + tableId + " " + oldTableName + " " + newTableName); http://git-wip-us.apache.org/repos/asf/accumulo/blob/c2ed43e4/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Setup.java ---------------------------------------------------------------------- diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Setup.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Setup.java index 7b077ee..edab2b5 100644 --- a/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Setup.java +++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/Setup.java @@ -31,7 +31,7 @@ public class Setup extends Test { Random rand = new Random(); state.set("rand", rand); - int numTables = Integer.parseInt(props.getProperty("numTables", "15")); + int numTables = Integer.parseInt(props.getProperty("numTables", "9")); int numNamespaces = Integer.parseInt(props.getProperty("numNamespaces", "2")); log.debug("numTables = " + numTables); log.debug("numNamespaces = " + numNamespaces); http://git-wip-us.apache.org/repos/asf/accumulo/blob/c2ed43e4/test/system/randomwalk/conf/modules/Concurrent.xml ---------------------------------------------------------------------- diff --git a/test/system/randomwalk/conf/modules/Concurrent.xml b/test/system/randomwalk/conf/modules/Concurrent.xml index cce8675..cb057c2 100644 --- a/test/system/randomwalk/conf/modules/Concurrent.xml +++ b/test/system/randomwalk/conf/modules/Concurrent.xml @@ -59,7 +59,7 @@ </node> <node id="ct.Setup"> - <property key="numTables" value="15"/> + <property key="numTables" value="9"/> <property key="numNamespaces" value="2"/> <edge id="ct.CreateTable" weight="1"/> </node>