Skip to content

Commit 7f7cf05

Browse files
keith-turnerkeith-turner
committed
Merge branch '3.1'
2 parents f129a58 + 3b5d9cf commit 7f7cf05

File tree

6 files changed

+92
-40
lines changed

6 files changed

+92
-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,8 +23,6 @@
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.SortedMap;
2927
import java.util.SortedSet;
3028
import java.util.TreeMap;
@@ -34,11 +32,8 @@
3432
import java.util.function.Function;
3533

3634
import org.apache.accumulo.core.Constants;
37-
import org.apache.accumulo.core.client.NamespaceNotFoundException;
3835
import org.apache.accumulo.core.client.admin.TabletMergeability;
3936
import org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException;
40-
import org.apache.accumulo.core.clientImpl.Namespace;
41-
import org.apache.accumulo.core.clientImpl.Namespaces;
4237
import org.apache.accumulo.core.clientImpl.TabletMergeabilityUtil;
4338
import org.apache.accumulo.core.clientImpl.thrift.TableOperation;
4439
import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType;
@@ -64,8 +59,6 @@
6459
import org.slf4j.Logger;
6560
import org.slf4j.LoggerFactory;
6661

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

79-
final Map<NamespaceId,String> namespaces = new HashMap<>();
80-
final boolean namespaceInTableName = tableName.contains(".");
8175
try {
8276
for (String tid : context.getZooSession().asReader()
8377
.getChildren(context.getZooKeeperRoot() + Constants.ZTABLES)) {
@@ -86,36 +80,17 @@ public static void checkTableNameDoesNotExist(ServerContext context, String tabl
8680
try {
8781
final byte[] tname =
8882
context.getZooSession().asReader().getData(zTablePath + Constants.ZTABLE_NAME);
89-
Preconditions.checkState(tname != null, "Malformed table entry in ZooKeeper at %s",
90-
zTablePath);
9183

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

115-
if (tableName.equals(TableNameUtil.qualified(new String(tname, UTF_8), namespaceName))
116-
&& !tableId.equals(TableId.of(tid))) {
117-
throw new AcceptableThriftTableOperationException(tid, tableName, operation,
118-
TableOperationExceptionType.EXISTS, null);
11994
}
12095
} catch (NoNodeException nne) {
12196
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
@@ -62,7 +62,7 @@ public Repo<Manager> call(FateId fateId, Manager environment) throws Exception {
6262
// write tableName & tableId to zookeeper
6363

6464
Utils.checkTableNameDoesNotExist(environment.getContext(), cloneInfo.tableName,
65-
cloneInfo.tableId, TableOperation.CLONE);
65+
cloneInfo.namespaceId, cloneInfo.tableId, TableOperation.CLONE);
6666

6767
environment.getTableManager().cloneTable(cloneInfo.srcTableId, cloneInfo.tableId,
6868
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
@@ -30,10 +30,13 @@
3030
import org.apache.accumulo.manager.tableOps.Utils;
3131
import org.apache.accumulo.server.conf.store.TablePropKey;
3232
import org.apache.accumulo.server.util.PropUtil;
33+
import org.slf4j.Logger;
34+
import org.slf4j.LoggerFactory;
3335

3436
class PopulateZookeeper extends ManagerRepo {
3537

3638
private static final long serialVersionUID = 1L;
39+
private static final Logger log = LoggerFactory.getLogger(PopulateZookeeper.class);
3740

3841
private final TableInfo tableInfo;
3942

@@ -55,7 +58,7 @@ public Repo<Manager> call(FateId fateId, Manager manager) throws Exception {
5558
try {
5659
// write tableName & tableId to zookeeper
5760
Utils.checkTableNameDoesNotExist(manager.getContext(), tableInfo.getTableName(),
58-
tableInfo.getTableId(), TableOperation.CREATE);
61+
tableInfo.getNamespaceId(), tableInfo.getTableId(), TableOperation.CREATE);
5962

6063
manager.getTableManager().addTable(tableInfo.getTableId(), tableInfo.getNamespaceId(),
6164
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
@@ -79,7 +79,7 @@ public Repo<Manager> call(FateId fateId, Manager manager) throws Exception {
7979

8080
Utils.getTableNameLock().lock();
8181
try {
82-
Utils.checkTableNameDoesNotExist(manager.getContext(), newTableName, tableId,
82+
Utils.checkTableNameDoesNotExist(manager.getContext(), newTableName, namespaceId, tableId,
8383
TableOperation.RENAME);
8484

8585
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
@@ -77,8 +77,8 @@ public Repo<Manager> call(FateId fateId, Manager env) throws Exception {
7777
Utils.getTableNameLock().lock();
7878
try {
7979
// write tableName & tableId to zookeeper
80-
Utils.checkTableNameDoesNotExist(env.getContext(), tableInfo.tableName, tableInfo.tableId,
81-
TableOperation.CREATE);
80+
Utils.checkTableNameDoesNotExist(env.getContext(), tableInfo.tableName, tableInfo.namespaceId,
81+
tableInfo.tableId, TableOperation.CREATE);
8282

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

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

+74
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.apache.accumulo.core.client.Scanner;
5151
import org.apache.accumulo.core.client.TableExistsException;
5252
import org.apache.accumulo.core.client.TableNotFoundException;
53+
import org.apache.accumulo.core.client.admin.CloneConfiguration;
5354
import org.apache.accumulo.core.client.admin.DiskUsage;
5455
import org.apache.accumulo.core.client.admin.NewTableConfiguration;
5556
import org.apache.accumulo.core.client.admin.TableOperations;
@@ -224,6 +225,79 @@ public void createTableWithBadProperties()
224225
() -> tableOps.setProperty(t0, Property.TABLE_BLOOM_ENABLED.getKey(), "foo"));
225226
}
226227

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

0 commit comments

Comments
 (0)