This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 5657bc8cf2 fixes creating same table name in different namespaces
(#5323)
5657bc8cf2 is described below
commit 5657bc8cf2bfdd622bb7db547634cb52a408a4a4
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Tue Feb 11 19:22:52 2025 -0500
fixes creating same table name in different namespaces (#5323)
When trying to create a table with the name `metadata` it failed because
`accumulo.metadata` existed. The code doing the check for existing
tables was not properly checking namespaces. Fixed the check and added
some test for test for the same table name in different namespaces.
Co-authored-by: Christopher Tubbs <ctubb...@apache.org>
---
.../apache/accumulo/manager/tableOps/Utils.java | 44 +++----------
.../manager/tableOps/clone/CloneZookeeper.java | 2 +-
.../manager/tableOps/create/PopulateZookeeper.java | 5 +-
.../manager/tableOps/rename/RenameTable.java | 2 +-
.../tableImport/ImportPopulateZookeeper.java | 4 +-
.../apache/accumulo/test/TableOperationsIT.java | 75 ++++++++++++++++++++++
6 files changed, 93 insertions(+), 39 deletions(-)
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java
index 858ac0e5e7..2e0f33c2a8 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java
@@ -23,8 +23,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import java.io.IOException;
import java.math.BigInteger;
import java.util.Base64;
-import java.util.HashMap;
-import java.util.Map;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.locks.Lock;
@@ -32,9 +30,7 @@ import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Function;
import org.apache.accumulo.core.Constants;
-import org.apache.accumulo.core.client.NamespaceNotFoundException;
import
org.apache.accumulo.core.clientImpl.AcceptableThriftTableOperationException;
-import org.apache.accumulo.core.clientImpl.Namespace;
import org.apache.accumulo.core.clientImpl.Namespaces;
import org.apache.accumulo.core.clientImpl.thrift.TableOperation;
import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType;
@@ -58,8 +54,6 @@ import org.apache.zookeeper.KeeperException.NoNodeException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Preconditions;
-
public class Utils {
private static final byte[] ZERO_BYTE = {'0'};
private static final Logger log = LoggerFactory.getLogger(Utils.class);
@@ -68,10 +62,11 @@ public class Utils {
* Checks that a table name is only used by the specified table id or not
used at all.
*/
public static void checkTableNameDoesNotExist(ServerContext context, String
tableName,
- TableId tableId, TableOperation operation) throws
AcceptableThriftTableOperationException {
+ NamespaceId destNamespaceId, TableId tableId, TableOperation operation)
+ throws AcceptableThriftTableOperationException {
+
+ var newTableName = TableNameUtil.qualify(tableName).getSecond();
- final Map<NamespaceId,String> namespaces = new HashMap<>();
- final boolean namespaceInTableName = tableName.contains(".");
try {
for (String tid : context.getZooReader()
.getChildren(context.getZooKeeperRoot() + Constants.ZTABLES)) {
@@ -79,36 +74,17 @@ public class Utils {
final String zTablePath = context.getZooKeeperRoot() +
Constants.ZTABLES + "/" + tid;
try {
final byte[] tname = context.getZooReader().getData(zTablePath +
Constants.ZTABLE_NAME);
- Preconditions.checkState(tname != null, "Malformed table entry in
ZooKeeper at %s",
- zTablePath);
- String namespaceName = Namespace.DEFAULT.name();
- if (namespaceInTableName) {
+ if (newTableName.equals(new String(tname, UTF_8))) {
+ // only make RPCs to get the namespace when the table names are
equal
final byte[] nId =
context.getZooReader().getData(zTablePath +
Constants.ZTABLE_NAMESPACE);
- if (nId != null) {
- final NamespaceId namespaceId = NamespaceId.of(new String(nId,
UTF_8));
- if (!namespaceId.equals(Namespace.DEFAULT.id())) {
- namespaceName = namespaces.get(namespaceId);
- if (namespaceName == null) {
- try {
- namespaceName = Namespaces.getNamespaceName(context,
namespaceId);
- namespaces.put(namespaceId, namespaceName);
- } catch (NamespaceNotFoundException e) {
- throw new AcceptableThriftTableOperationException(null,
tableName,
- TableOperation.CREATE,
TableOperationExceptionType.OTHER,
- "Table (" + tableId.canonical() + ") contains
reference to namespace ("
- + namespaceId + ") that doesn't exist");
- }
- }
- }
+ if (destNamespaceId.canonical().equals(new String(nId, UTF_8))
+ && !tableId.canonical().equals(tid)) {
+ throw new AcceptableThriftTableOperationException(tid,
tableName, operation,
+ TableOperationExceptionType.EXISTS, null);
}
- }
- if (tableName.equals(TableNameUtil.qualified(new String(tname,
UTF_8), namespaceName))
- && !tableId.equals(TableId.of(tid))) {
- throw new AcceptableThriftTableOperationException(tid, tableName,
operation,
- TableOperationExceptionType.EXISTS, null);
}
} catch (NoNodeException nne) {
log.trace("skipping tableId {}, either being created or has been
deleted.", tid, nne);
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/CloneZookeeper.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/CloneZookeeper.java
index 63d8d1be3b..77ef41df12 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/CloneZookeeper.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/CloneZookeeper.java
@@ -60,7 +60,7 @@ class CloneZookeeper extends ManagerRepo {
// write tableName & tableId to zookeeper
Utils.checkTableNameDoesNotExist(environment.getContext(),
cloneInfo.tableName,
- cloneInfo.tableId, TableOperation.CLONE);
+ cloneInfo.namespaceId, cloneInfo.tableId, TableOperation.CLONE);
environment.getTableManager().cloneTable(cloneInfo.srcTableId,
cloneInfo.tableId,
cloneInfo.tableName, cloneInfo.namespaceId,
cloneInfo.propertiesToSet,
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/PopulateZookeeper.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/PopulateZookeeper.java
index 31a2210727..db0c6db4d9 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/PopulateZookeeper.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/PopulateZookeeper.java
@@ -28,10 +28,13 @@ import org.apache.accumulo.manager.tableOps.TableInfo;
import org.apache.accumulo.manager.tableOps.Utils;
import org.apache.accumulo.server.conf.store.TablePropKey;
import org.apache.accumulo.server.util.PropUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
class PopulateZookeeper extends ManagerRepo {
private static final long serialVersionUID = 1L;
+ private static final Logger log =
LoggerFactory.getLogger(PopulateZookeeper.class);
private final TableInfo tableInfo;
@@ -53,7 +56,7 @@ class PopulateZookeeper extends ManagerRepo {
try {
// write tableName & tableId to zookeeper
Utils.checkTableNameDoesNotExist(manager.getContext(),
tableInfo.getTableName(),
- tableInfo.getTableId(), TableOperation.CREATE);
+ tableInfo.getNamespaceId(), tableInfo.getTableId(),
TableOperation.CREATE);
manager.getTableManager().addTable(tableInfo.getTableId(),
tableInfo.getNamespaceId(),
tableInfo.getTableName());
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/rename/RenameTable.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/rename/RenameTable.java
index a87292c825..0ae33f7891 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/rename/RenameTable.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/rename/RenameTable.java
@@ -76,7 +76,7 @@ public class RenameTable extends ManagerRepo {
Utils.getTableNameLock().lock();
try {
- Utils.checkTableNameDoesNotExist(manager.getContext(), newTableName,
tableId,
+ Utils.checkTableNameDoesNotExist(manager.getContext(), newTableName,
namespaceId, tableId,
TableOperation.RENAME);
final String newName = qualifiedNewTableName.getSecond();
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportPopulateZookeeper.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportPopulateZookeeper.java
index 867d7ac8ac..a1535710d2 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportPopulateZookeeper.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportPopulateZookeeper.java
@@ -75,8 +75,8 @@ class ImportPopulateZookeeper extends ManagerRepo {
Utils.getTableNameLock().lock();
try {
// write tableName & tableId to zookeeper
- Utils.checkTableNameDoesNotExist(env.getContext(), tableInfo.tableName,
tableInfo.tableId,
- TableOperation.CREATE);
+ Utils.checkTableNameDoesNotExist(env.getContext(), tableInfo.tableName,
tableInfo.namespaceId,
+ tableInfo.tableId, TableOperation.CREATE);
String namespace = TableNameUtil.qualify(tableInfo.tableName).getFirst();
NamespaceId namespaceId = Namespaces.getNamespaceId(env.getContext(),
namespace);
diff --git a/test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java
b/test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java
index 82bb5679b3..8007b81996 100644
--- a/test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java
@@ -40,6 +40,7 @@ import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
import org.apache.accumulo.core.client.Accumulo;
import org.apache.accumulo.core.client.AccumuloClient;
@@ -50,6 +51,7 @@ import org.apache.accumulo.core.client.IteratorSetting;
import org.apache.accumulo.core.client.Scanner;
import org.apache.accumulo.core.client.TableExistsException;
import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.client.admin.CloneConfiguration;
import org.apache.accumulo.core.client.admin.DiskUsage;
import org.apache.accumulo.core.client.admin.NewTableConfiguration;
import org.apache.accumulo.core.client.admin.TableOperations;
@@ -216,6 +218,79 @@ public class TableOperationsIT extends
AccumuloClusterHarness {
() -> tableOps.setProperty(t0, Property.TABLE_BLOOM_ENABLED.getKey(),
"foo"));
}
+ @Test
+ public void createTablesWithSameNameInDifferentNamespace() throws Exception {
+ TableOperations tableOps = accumuloClient.tableOperations();
+ String[] names = getUniqueNames(2);
+
+ accumuloClient.namespaceOperations().create("test1");
+ accumuloClient.namespaceOperations().create("test2");
+
+ var tables = Set.of("test1.table1", "test1.table2", "test1.root",
"test2.table1",
+ "test2.table2", "test2.metadata", "table1", "table2", "metadata");
+
+ for (String table : tables) {
+ tableOps.create(table);
+ }
+
+ assertTrue(accumuloClient.tableOperations().list().containsAll(tables));
+
+ accumuloClient.namespaceOperations().create("test3");
+
+ Set<String> clones = new HashSet<>();
+ for (String table : tables) {
+ if (table.startsWith("test1.")) {
+ var clone = table.replace("test1.", "test3.");
+ tableOps.clone(table, clone, CloneConfiguration.empty());
+ clones.add(clone);
+ }
+ }
+
+ assertTrue(accumuloClient.tableOperations().list().containsAll(tables));
+ assertTrue(accumuloClient.tableOperations().list().containsAll(clones));
+
+ // Rename a table to a tablename that exists in another namespace
+ tableOps.rename("test1.table1", "test1.metadata");
+
+ tables = new HashSet<>(tables);
+ tables.remove("test1.table1");
+ tables.add("test1.metadata");
+
+ assertTrue(accumuloClient.tableOperations().list().containsAll(tables));
+ assertTrue(accumuloClient.tableOperations().list().containsAll(clones));
+
assertFalse(accumuloClient.tableOperations().list().contains("test1.table"));
+
+ // Read and write data to tables w/ the same name in different namespaces
to ensure no wires are
+ // crossed.
+ for (var table : Sets.union(tables, clones)) {
+ try (var writer = accumuloClient.createBatchWriter(table)) {
+ Mutation m = new Mutation("self");
+ m.put("table", "name", table);
+ writer.addMutation(m);
+ }
+ }
+
+ for (var table : Sets.union(tables, clones)) {
+ try (var scanner = accumuloClient.createScanner(table)) {
+ var seenValues =
+ scanner.stream().map(e ->
e.getValue().toString()).collect(Collectors.toSet());
+ assertEquals(Set.of(table), seenValues);
+ }
+ }
+
+ for (var table : Sets.union(tables, clones)) {
+ assertThrows(TableExistsException.class,
+ () -> accumuloClient.tableOperations().create(table));
+ }
+
+ for (var srcTable : List.of("metadata", "test1.table2")) {
+ for (var destTable : Sets.union(tables, clones)) {
+ assertThrows(TableExistsException.class, () ->
accumuloClient.tableOperations()
+ .clone(srcTable, destTable, CloneConfiguration.empty()));
+ }
+ }
+ }
+
@Test
public void createMergeClonedTable() throws Exception {
String[] names = getUniqueNames(2);
