Skip to content

Commit 3b5d9cf

Browse files
keith-turnerkeith-turner
committed
Merge branch '2.1' into 3.1
2 parents 187733f + 5657bc8 commit 3b5d9cf

File tree

6 files changed

+93
-40
lines changed

6 files changed

+93
-40
lines changed

‎server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java

+10-35
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,14 @@
2323
import java.io.IOException;
2424
import java.math.BigInteger;
2525
import java.util.Base64;
26-
import java.util.HashMap;
27-
import java.util.Map;
2826
import java.util.SortedSet;
2927
import java.util.TreeSet;
3028
import java.util.concurrent.locks.Lock;
3129
import java.util.concurrent.locks.ReentrantLock;
3230
import java.util.function.Function;
3331

3432
import org.apache.accumulo.core.Constants;
35-
import org.apache.accumulo.core.client.NamespaceNotFoundException;
3633
import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException;
37-
import org.apache.accumulo.core.clientImpl.Namespace;
38-
import org.apache.accumulo.core.clientImpl.Namespaces;
3934
import org.apache.accumulo.core.clientImpl.thrift.TableOperation;
4035
import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType;
4136
import org.apache.accumulo.core.data.AbstractId;
@@ -60,8 +55,6 @@
6055
import org.slf4j.Logger;
6156
import org.slf4j.LoggerFactory;
6257

63-
import com.google.common.base.Preconditions;
64-
6558
public class Utils {
6659
private static final byte[] ZERO_BYTE = {'0'};
6760
private static final Logger log = LoggerFactory.getLogger(Utils.class);
@@ -70,10 +63,11 @@ public class Utils {
7063
* Checks that a table name is only used by the specified table id or not used at all.
7164
*/
7265
public static void checkTableNameDoesNotExist(ServerContext context, String tableName,
73-
TableId tableId, TableOperation operation) throws AcceptableThriftTableOperationException {
66+
NamespaceId destNamespaceId, TableId tableId, TableOperation operation)
67+
throws AcceptableThriftTableOperationException {
68+
69+
var newTableName = TableNameUtil.qualify(tableName).getSecond();
7470

75-
final Map<NamespaceId,String> namespaces = new HashMap<>();
76-
final boolean namespaceInTableName = tableName.contains(".");
7771
try {
7872
for (String tid : context.getZooSession().asReader()
7973
.getChildren(context.getZooKeeperRoot() + Constants.ZTABLES)) {
@@ -82,36 +76,17 @@ public static void checkTableNameDoesNotExist(ServerContext context, String tabl
8276
try {
8377
final byte[] tname =
8478
context.getZooSession().asReader().getData(zTablePath + Constants.ZTABLE_NAME);
85-
Preconditions.checkState(tname != null, "Malformed table entry in ZooKeeper at %s",
86-
zTablePath);
8779

88-
String namespaceName = Namespace.DEFAULT.name();
89-
if (namespaceInTableName) {
80+
if (newTableName.equals(new String(tname, UTF_8))) {
81+
// only make RPCs to get the namespace when the table names are equal
9082
final byte[] nId =
9183
context.getZooSession().asReader().getData(zTablePath + Constants.ZTABLE_NAMESPACE);
92-
if (nId != null) {
93-
final NamespaceId namespaceId = NamespaceId.of(new String(nId, UTF_8));
94-
if (!namespaceId.equals(Namespace.DEFAULT.id())) {
95-
namespaceName = namespaces.get(namespaceId);
96-
if (namespaceName == null) {
97-
try {
98-
namespaceName = Namespaces.getNamespaceName(context, namespaceId);
99-
namespaces.put(namespaceId, namespaceName);
100-
} catch (NamespaceNotFoundException e) {
101-
throw new AcceptableThriftTableOperationException(null, tableName,
102-
TableOperation.CREATE, TableOperationExceptionType.OTHER,
103-
"Table (" + tableId.canonical() + ") contains reference to namespace ("
104-
+ namespaceId + ") that doesn't exist");
105-
}
106-
}
107-
}
84+
if (destNamespaceId.canonical().equals(new String(nId, UTF_8))
85+
&& !tableId.canonical().equals(tid)) {
86+
throw new AcceptableThriftTableOperationException(tid, tableName, operation,
87+
TableOperationExceptionType.EXISTS, null);
10888
}
109-
}
11089

111-
if (tableName.equals(TableNameUtil.qualified(new String(tname, UTF_8), namespaceName))
112-
&& !tableId.equals(TableId.of(tid))) {
113-
throw new AcceptableThriftTableOperationException(tid, tableName, operation,
114-
TableOperationExceptionType.EXISTS, null);
11590
}
11691
} catch (NoNodeException nne) {
11792
log.trace("skipping tableId {}, either being created or has been deleted.", tid, nne);

‎server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/CloneZookeeper.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public Repo<Manager> call(long tid, Manager environment) throws Exception {
6161
// write tableName & tableId to zookeeper
6262

6363
Utils.checkTableNameDoesNotExist(environment.getContext(), cloneInfo.tableName,
64-
cloneInfo.tableId, TableOperation.CLONE);
64+
cloneInfo.namespaceId, cloneInfo.tableId, TableOperation.CLONE);
6565

6666
environment.getTableManager().cloneTable(cloneInfo.srcTableId, cloneInfo.tableId,
6767
cloneInfo.tableName, cloneInfo.namespaceId, cloneInfo.propertiesToSet,

‎server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/PopulateZookeeper.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,13 @@
2929
import org.apache.accumulo.manager.tableOps.Utils;
3030
import org.apache.accumulo.server.conf.store.TablePropKey;
3131
import org.apache.accumulo.server.util.PropUtil;
32+
import org.slf4j.Logger;
33+
import org.slf4j.LoggerFactory;
3234

3335
class PopulateZookeeper extends ManagerRepo {
3436

3537
private static final long serialVersionUID = 1L;
38+
private static final Logger log = LoggerFactory.getLogger(PopulateZookeeper.class);
3639

3740
private final TableInfo tableInfo;
3841

@@ -54,7 +57,7 @@ public Repo<Manager> call(long tid, Manager manager) throws Exception {
5457
try {
5558
// write tableName & tableId to zookeeper
5659
Utils.checkTableNameDoesNotExist(manager.getContext(), tableInfo.getTableName(),
57-
tableInfo.getTableId(), TableOperation.CREATE);
60+
tableInfo.getNamespaceId(), tableInfo.getTableId(), TableOperation.CREATE);
5861

5962
manager.getTableManager().addTable(tableInfo.getTableId(), tableInfo.getNamespaceId(),
6063
tableInfo.getTableName());

‎server/manager/src/main/java/org/apache/accumulo/manager/tableOps/rename/RenameTable.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public Repo<Manager> call(long tid, Manager manager) throws Exception {
7777

7878
Utils.getTableNameLock().lock();
7979
try {
80-
Utils.checkTableNameDoesNotExist(manager.getContext(), newTableName, tableId,
80+
Utils.checkTableNameDoesNotExist(manager.getContext(), newTableName, namespaceId, tableId,
8181
TableOperation.RENAME);
8282

8383
final String newName = qualifiedNewTableName.getSecond();

‎server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportPopulateZookeeper.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ public Repo<Manager> call(long tid, Manager env) throws Exception {
7676
Utils.getTableNameLock().lock();
7777
try {
7878
// write tableName & tableId to zookeeper
79-
Utils.checkTableNameDoesNotExist(env.getContext(), tableInfo.tableName, tableInfo.tableId,
80-
TableOperation.CREATE);
79+
Utils.checkTableNameDoesNotExist(env.getContext(), tableInfo.tableName, tableInfo.namespaceId,
80+
tableInfo.tableId, TableOperation.CREATE);
8181

8282
String namespace = TableNameUtil.qualify(tableInfo.tableName).getFirst();
8383
NamespaceId namespaceId = Namespaces.getNamespaceId(env.getContext(), namespace);

‎test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java

+75
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.SortedSet;
4040
import java.util.TreeMap;
4141
import java.util.TreeSet;
42+
import java.util.stream.Collectors;
4243

4344
import org.apache.accumulo.core.client.Accumulo;
4445
import org.apache.accumulo.core.client.AccumuloClient;
@@ -49,6 +50,7 @@
4950
import org.apache.accumulo.core.client.Scanner;
5051
import org.apache.accumulo.core.client.TableExistsException;
5152
import org.apache.accumulo.core.client.TableNotFoundException;
53+
import org.apache.accumulo.core.client.admin.CloneConfiguration;
5254
import org.apache.accumulo.core.client.admin.DiskUsage;
5355
import org.apache.accumulo.core.client.admin.NewTableConfiguration;
5456
import org.apache.accumulo.core.client.admin.TableOperations;
@@ -216,6 +218,79 @@ public void createTableWithBadProperties()
216218
() -> tableOps.setProperty(t0, Property.TABLE_BLOOM_ENABLED.getKey(), "foo"));
217219
}
218220

221+
@Test
222+
public void createTablesWithSameNameInDifferentNamespace() throws Exception {
223+
TableOperations tableOps = accumuloClient.tableOperations();
224+
String[] names = getUniqueNames(2);
225+
226+
accumuloClient.namespaceOperations().create("test1");
227+
accumuloClient.namespaceOperations().create("test2");
228+
229+
var tables = Set.of("test1.table1", "test1.table2", "test1.root", "test2.table1",
230+
"test2.table2", "test2.metadata", "table1", "table2", "metadata");
231+
232+
for (String table : tables) {
233+
tableOps.create(table);
234+
}
235+
236+
assertTrue(accumuloClient.tableOperations().list().containsAll(tables));
237+
238+
accumuloClient.namespaceOperations().create("test3");
239+
240+
Set<String> clones = new HashSet<>();
241+
for (String table : tables) {
242+
if (table.startsWith("test1.")) {
243+
var clone = table.replace("test1.", "test3.");
244+
tableOps.clone(table, clone, CloneConfiguration.empty());
245+
clones.add(clone);
246+
}
247+
}
248+
249+
assertTrue(accumuloClient.tableOperations().list().containsAll(tables));
250+
assertTrue(accumuloClient.tableOperations().list().containsAll(clones));
251+
252+
// Rename a table to a tablename that exists in another namespace
253+
tableOps.rename("test1.table1", "test1.metadata");
254+
255+
tables = new HashSet<>(tables);
256+
tables.remove("test1.table1");
257+
tables.add("test1.metadata");
258+
259+
assertTrue(accumuloClient.tableOperations().list().containsAll(tables));
260+
assertTrue(accumuloClient.tableOperations().list().containsAll(clones));
261+
assertFalse(accumuloClient.tableOperations().list().contains("test1.table"));
262+
263+
// Read and write data to tables w/ the same name in different namespaces to ensure no wires are
264+
// crossed.
265+
for (var table : Sets.union(tables, clones)) {
266+
try (var writer = accumuloClient.createBatchWriter(table)) {
267+
Mutation m = new Mutation("self");
268+
m.put("table", "name", table);
269+
writer.addMutation(m);
270+
}
271+
}
272+
273+
for (var table : Sets.union(tables, clones)) {
274+
try (var scanner = accumuloClient.createScanner(table)) {
275+
var seenValues =
276+
scanner.stream().map(e -> e.getValue().toString()).collect(Collectors.toSet());
277+
assertEquals(Set.of(table), seenValues);
278+
}
279+
}
280+
281+
for (var table : Sets.union(tables, clones)) {
282+
assertThrows(TableExistsException.class,
283+
() -> accumuloClient.tableOperations().create(table));
284+
}
285+
286+
for (var srcTable : List.of("metadata", "test1.table2")) {
287+
for (var destTable : Sets.union(tables, clones)) {
288+
assertThrows(TableExistsException.class, () -> accumuloClient.tableOperations()
289+
.clone(srcTable, destTable, CloneConfiguration.empty()));
290+
}
291+
}
292+
}
293+
219294
@Test
220295
public void createMergeClonedTable() throws Exception {
221296
String[] names = getUniqueNames(2);

0 commit comments

Comments
 (0)