This is an automated email from the ASF dual-hosted git repository.
dlmarion 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 cd27402d50 Reduce calls to ZooCache to get table information when we
already have it (#4685)
cd27402d50 is described below
commit cd27402d50a862afc1441f6a3b51b0f9bd37f625
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Mon Jun 24 13:40:16 2024 -0400
Reduce calls to ZooCache to get table information when we already have it
(#4685)
Related to #4684
Co-authored-by: Keith Turner <ktur...@apache.org>
---
.../accumulo/core/util/tables/TableZooHelper.java | 20 +++++++
.../server/client/ClientServiceHandler.java | 5 +-
.../server/security/AuditedSecurityOperation.java | 7 ++-
.../server/security/SecurityOperation.java | 5 +-
.../apache/accumulo/manager/tableOps/Utils.java | 32 +++++++++--
.../manager/tableOps/clone/ClonePermissions.java | 4 +-
.../manager/tableOps/clone/CloneZookeeper.java | 4 +-
.../manager/tableOps/create/PopulateZookeeper.java | 2 +-
.../manager/tableOps/create/SetupPermissions.java | 3 +-
.../manager/tableOps/rename/RenameTable.java | 2 +-
.../tableImport/ImportPopulateZookeeper.java | 2 +-
.../tableImport/ImportSetupPermissions.java | 2 +-
.../org/apache/accumulo/test/CreateTableIT.java | 67 ++++++++++++++++++++++
13 files changed, 132 insertions(+), 23 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java
b/core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java
index 6345f9d5dd..de53d2bf1e 100644
---
a/core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java
+++
b/core/src/main/java/org/apache/accumulo/core/util/tables/TableZooHelper.java
@@ -31,11 +31,14 @@ import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.client.NamespaceNotFoundException;
import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.Namespace;
import org.apache.accumulo.core.clientImpl.Namespaces;
import org.apache.accumulo.core.data.NamespaceId;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.fate.zookeeper.ZooCache;
import org.apache.accumulo.core.manager.state.tables.TableState;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import org.apache.accumulo.core.metadata.RootTable;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
@@ -59,6 +62,12 @@ public class TableZooHelper implements AutoCloseable {
* getCause() of NamespaceNotFoundException
*/
public TableId getTableId(String tableName) throws TableNotFoundException {
+ if (MetadataTable.NAME.equals(tableName)) {
+ return MetadataTable.ID;
+ }
+ if (RootTable.NAME.equals(tableName)) {
+ return RootTable.ID;
+ }
try {
return
_getTableIdDetectNamespaceNotFound(EXISTING_TABLE_NAME.validate(tableName));
} catch (NamespaceNotFoundException e) {
@@ -90,6 +99,12 @@ public class TableZooHelper implements AutoCloseable {
}
public String getTableName(TableId tableId) throws TableNotFoundException {
+ if (MetadataTable.ID.equals(tableId)) {
+ return MetadataTable.NAME;
+ }
+ if (RootTable.ID.equals(tableId)) {
+ return RootTable.NAME;
+ }
String tableName = getTableMap().getIdtoNameMap().get(tableId);
if (tableName == null) {
throw new TableNotFoundException(tableId.canonical(), null, null);
@@ -185,6 +200,11 @@ public class TableZooHelper implements AutoCloseable {
public NamespaceId getNamespaceId(TableId tableId) throws
TableNotFoundException {
checkArgument(context != null, "instance is null");
checkArgument(tableId != null, "tableId is null");
+
+ if (MetadataTable.ID.equals(tableId) || RootTable.ID.equals(tableId)) {
+ return Namespace.ACCUMULO.id();
+ }
+
ZooCache zc = context.getZooCache();
byte[] n = zc.get(context.getZooKeeperRoot() + Constants.ZTABLES + "/" +
tableId
+ Constants.ZTABLE_NAMESPACE);
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
index 76f0692eb8..466bc53257 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java
@@ -216,12 +216,11 @@ public class ClientServiceHandler implements
ClientService.Iface {
NamespaceId namespaceId;
try {
namespaceId = context.getNamespaceId(tableId);
+ security.grantTablePermission(credentials, user, tableId, tableName,
+ TablePermission.getPermissionById(permission), namespaceId);
} catch (TableNotFoundException e) {
throw new TException(e);
}
-
- security.grantTablePermission(credentials, user, tableId,
- TablePermission.getPermissionById(permission), namespaceId);
}
@Override
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
b/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
index 3718a91b62..e4f61ed9ce 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/security/AuditedSecurityOperation.java
@@ -622,10 +622,10 @@ public class AuditedSecurityOperation extends
SecurityOperation {
@Override
public void grantTablePermission(TCredentials credentials, String user,
TableId tableId,
- TablePermission permission, NamespaceId namespaceId) throws
ThriftSecurityException {
- String tableName = getTableName(tableId);
+ String tableName, TablePermission permission, NamespaceId namespaceId)
+ throws ThriftSecurityException, TableNotFoundException {
try {
- super.grantTablePermission(credentials, user, tableId, permission,
namespaceId);
+ super.grantTablePermission(credentials, user, tableId, tableName,
permission, namespaceId);
audit(credentials, GRANT_TABLE_PERMISSION_AUDIT_TEMPLATE, permission,
tableName, user);
} catch (ThriftSecurityException ex) {
audit(credentials, ex, GRANT_TABLE_PERMISSION_AUDIT_TEMPLATE,
permission, tableName, user);
@@ -750,4 +750,5 @@ public class AuditedSecurityOperation extends
SecurityOperation {
throw e;
}
}
+
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
index 36277652fd..dd59cebd93 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
@@ -743,8 +743,9 @@ public class SecurityOperation {
}
}
- public void grantTablePermission(TCredentials c, String user, TableId
tableId,
- TablePermission permission, NamespaceId namespaceId) throws
ThriftSecurityException {
+ public void grantTablePermission(TCredentials c, String user, TableId
tableId, String tableName,
+ TablePermission permission, NamespaceId namespaceId)
+ throws ThriftSecurityException, TableNotFoundException {
if (!canGrantTable(c, user, tableId, namespaceId)) {
throw new ThriftSecurityException(c.getPrincipal(),
SecurityErrorCode.PERMISSION_DENIED);
}
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 def74371c2..4942b8efec 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
@@ -49,22 +49,42 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.Text;
import org.apache.zookeeper.KeeperException;
+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);
- public static void checkTableDoesNotExist(ServerContext context, String
tableName,
+ public static void checkTableNameDoesNotExist(ServerContext context, String
tableName,
TableId tableId, TableOperation operation) throws
AcceptableThriftTableOperationException {
- TableId id = context.getTableNameToIdMap().get(tableName);
-
- if (id != null && !id.equals(tableId)) {
- throw new AcceptableThriftTableOperationException(null, tableName,
operation,
- TableOperationExceptionType.EXISTS, null);
+ try {
+ for (String tid : context.getZooReader()
+ .getChildren(context.getZooKeeperRoot() + Constants.ZTABLES)) {
+ String zTablePath = context.getZooKeeperRoot() + Constants.ZTABLES +
"/" + tid;
+ try {
+ byte[] tname = context.getZooReader().getData(zTablePath +
Constants.ZTABLE_NAME);
+ Preconditions.checkState(tname != null, "Malformed table entry in
ZooKeeper at %s",
+ zTablePath);
+ if (tableName.equals(new String(tname, UTF_8))) {
+ 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);
+ continue;
+ }
+ }
+ } catch (KeeperException | InterruptedException e) {
+ log.error("Error checking to see if tableId {} exists in ZooKeeper",
tableId, e);
+ throw new AcceptableThriftTableOperationException(null, tableName,
TableOperation.CREATE,
+ TableOperationExceptionType.OTHER, e.getMessage());
}
+
}
public static <T extends AbstractId<T>> T getNextId(String name,
ServerContext context,
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/ClonePermissions.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/ClonePermissions.java
index 7ac8d6fd0b..76389668d0 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/ClonePermissions.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/clone/ClonePermissions.java
@@ -50,8 +50,8 @@ class ClonePermissions extends ManagerRepo {
for (TablePermission permission : TablePermission.values()) {
try {
environment.getContext().getSecurityOperation().grantTablePermission(
- environment.getContext().rpcCreds(), cloneInfo.user,
cloneInfo.tableId, permission,
- cloneInfo.namespaceId);
+ environment.getContext().rpcCreds(), cloneInfo.user,
cloneInfo.tableId,
+ cloneInfo.tableName, permission, cloneInfo.namespaceId);
} catch (ThriftSecurityException e) {
LoggerFactory.getLogger(ClonePermissions.class).error("{}",
e.getMessage(), e);
throw e;
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 cfc10d015c..917b8f6995 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
@@ -59,8 +59,8 @@ class CloneZookeeper extends ManagerRepo {
try {
// write tableName & tableId to zookeeper
- Utils.checkTableDoesNotExist(environment.getContext(),
cloneInfo.tableName, cloneInfo.tableId,
- TableOperation.CLONE);
+ Utils.checkTableNameDoesNotExist(environment.getContext(),
cloneInfo.tableName,
+ 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 40ad2d276d..31a2210727 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
@@ -52,7 +52,7 @@ class PopulateZookeeper extends ManagerRepo {
Utils.getTableNameLock().lock();
try {
// write tableName & tableId to zookeeper
- Utils.checkTableDoesNotExist(manager.getContext(),
tableInfo.getTableName(),
+ Utils.checkTableNameDoesNotExist(manager.getContext(),
tableInfo.getTableName(),
tableInfo.getTableId(), TableOperation.CREATE);
manager.getTableManager().addTable(tableInfo.getTableId(),
tableInfo.getNamespaceId(),
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
index 5b840c16ed..6fa7496e4b 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java
@@ -45,7 +45,8 @@ class SetupPermissions extends ManagerRepo {
for (TablePermission permission : TablePermission.values()) {
try {
security.grantTablePermission(env.getContext().rpcCreds(),
tableInfo.getUser(),
- tableInfo.getTableId(), permission, tableInfo.getNamespaceId());
+ tableInfo.getTableId(), tableInfo.getTableName(), permission,
+ tableInfo.getNamespaceId());
} catch (ThriftSecurityException e) {
LoggerFactory.getLogger(SetupPermissions.class).error("{}",
e.getMessage(), e);
throw e;
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 a91b0ca8a8..5b078e2f2c 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.checkTableDoesNotExist(manager.getContext(), newTableName, tableId,
+ Utils.checkTableNameDoesNotExist(manager.getContext(), newTableName,
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 eb57a80032..41fc0b0f8d 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,7 +75,7 @@ class ImportPopulateZookeeper extends ManagerRepo {
Utils.getTableNameLock().lock();
try {
// write tableName & tableId to zookeeper
- Utils.checkTableDoesNotExist(env.getContext(), tableInfo.tableName,
tableInfo.tableId,
+ Utils.checkTableNameDoesNotExist(env.getContext(), tableInfo.tableName,
tableInfo.tableId,
TableOperation.CREATE);
String namespace = TableNameUtil.qualify(tableInfo.tableName).getFirst();
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
index 8f53de1011..bdcdcce98d 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableImport/ImportSetupPermissions.java
@@ -48,7 +48,7 @@ class ImportSetupPermissions extends ManagerRepo {
for (TablePermission permission : TablePermission.values()) {
try {
security.grantTablePermission(env.getContext().rpcCreds(),
tableInfo.user,
- tableInfo.tableId, permission, tableInfo.namespaceId);
+ tableInfo.tableId, tableInfo.tableName, permission,
tableInfo.namespaceId);
} catch (ThriftSecurityException e) {
LoggerFactory.getLogger(ImportSetupPermissions.class).error("{}",
e.getMessage(), e);
throw e;
diff --git a/test/src/main/java/org/apache/accumulo/test/CreateTableIT.java
b/test/src/main/java/org/apache/accumulo/test/CreateTableIT.java
new file mode 100644
index 0000000000..b26271a3e4
--- /dev/null
+++ b/test/src/main/java/org/apache/accumulo/test/CreateTableIT.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.time.Duration;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.harness.SharedMiniClusterBase;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+public class CreateTableIT extends SharedMiniClusterBase {
+
+ @Override
+ protected Duration defaultTimeout() {
+ return Duration.ofMinutes(5);
+ }
+
+ @BeforeAll
+ public static void setup() throws Exception {
+ SharedMiniClusterBase.startMiniCluster();
+ }
+
+ @AfterAll
+ public static void teardown() {
+ SharedMiniClusterBase.stopMiniCluster();
+ }
+
+ @Test
+ public void testCreateLotsOfTables() throws Exception {
+ try (AccumuloClient client =
Accumulo.newClient().from(getClientProps()).build()) {
+
+ String[] tableNames = getUniqueNames(1000);
+
+ for (int i = 0; i < tableNames.length; i++) {
+ // Create waits for the Fate operation to complete
+ long start = System.currentTimeMillis();
+ client.tableOperations().create(tableNames[i]);
+ System.out.println("Table creation took: " +
(System.currentTimeMillis() - start) + "ms");
+ }
+ // Confirm all 1000 user tables exist in addition to Root, Metadata,
+ // and ScanRef tables
+ assertEquals(1003, client.tableOperations().list().size());
+ }
+ }
+
+}
