From ae64cfdbaa0d41c13f34c6f3ff63cf4bd37138f0 Mon Sep 17 00:00:00 2001 From: Kevin Rathbun Date: Fri, 25 Apr 2025 15:39:41 -0400 Subject: [PATCH 1/2] Minor TableOperations changes: - Added early `return true` for all system tables for `TableOperations.exists()` - Move built in table check for `TableOperations.setTabletAvailability()` to be handled on server side instead of client side. Result is RPC now occurs for a built in table, and exception for calling `setTabletAvailability` on a built in table is changed from `IllegalArgumentException` to `AccumuloException`. All the other `TableOperations` that can't be called on a system table result in an `AccumuloException` if they are called on a system table, so this change keeps consistency with the other `TableOperations` - This commit additionally made it so that, for all `TableOperations`, on the client side, the only validation done on a table is checking whether the argument is formatted correctly (via Validators.EXISTING_TABLE_NAME and Validators.NEW_TABLE_NAME) (e.g., table name is not blank, not too long, etc.). --- .../apache/accumulo/core/clientImpl/TableOperationsImpl.java | 5 +---- .../java/org/apache/accumulo/manager/FateServiceHandler.java | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index fd54934ade6..14910080440 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -41,7 +41,6 @@ import static org.apache.accumulo.core.util.LazySingletons.RANDOM; import static org.apache.accumulo.core.util.Validators.EXISTING_TABLE_NAME; import static org.apache.accumulo.core.util.Validators.NEW_TABLE_NAME; -import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_TABLE; import static org.apache.accumulo.core.util.threads.ThreadPoolNames.SPLIT_START_POOL; import static org.apache.accumulo.core.util.threads.ThreadPoolNames.SPLIT_WAIT_POOL; @@ -223,8 +222,7 @@ public SortedSet list() { public boolean exists(String tableName) { EXISTING_TABLE_NAME.validate(tableName); - if (tableName.equals(SystemTables.METADATA.tableName()) - || tableName.equals(SystemTables.ROOT.tableName())) { + if (SystemTables.containsTableName(tableName)) { return true; } @@ -2243,7 +2241,6 @@ private void validatePropertiesToSet(Map opts, Map public void setTabletAvailability(String tableName, Range range, TabletAvailability availability) throws AccumuloSecurityException, AccumuloException { EXISTING_TABLE_NAME.validate(tableName); - NOT_BUILTIN_TABLE.validate(tableName); checkArgument(range != null, "range is null"); checkArgument(availability != null, "tabletAvailability is null"); diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java index dd9788e85b6..61cedce94c5 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java @@ -27,7 +27,6 @@ import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_NAMESPACE; import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_TABLE; import static org.apache.accumulo.core.util.Validators.NOT_BUILTIN_TABLE_ID; -import static org.apache.accumulo.core.util.Validators.NOT_METADATA_TABLE; import static org.apache.accumulo.core.util.Validators.NOT_ROOT_TABLE_ID; import static org.apache.accumulo.core.util.Validators.VALID_TABLE_ID; import static org.apache.accumulo.core.util.Validators.sameNamespaceAs; @@ -692,7 +691,8 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, TFateId opid, TFat case TABLE_TABLET_AVAILABILITY: { TableOperation tableOp = TableOperation.SET_TABLET_AVAILABILITY; validateArgumentCount(arguments, tableOp, 3); - String tableName = validateName(arguments.get(0), tableOp, NOT_METADATA_TABLE); + String tableName = + validateName(arguments.get(0), tableOp, NOT_BUILTIN_TABLE.and(EXISTING_TABLE_NAME)); TableId tableId = null; try { tableId = manager.getContext().getTableId(tableName); From c564e58f4b52971718587984b296ef139a30a19f Mon Sep 17 00:00:00 2001 From: Kevin Rathbun Date: Mon, 28 Apr 2025 15:04:55 -0400 Subject: [PATCH 2/2] ensure table is not system table for offline, online, setTabletAvailability client-side --- .../accumulo/core/clientImpl/TableOperationsImpl.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index 14910080440..e322f5cbd26 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -1509,15 +1509,13 @@ private void changeTableState(String tableName, boolean wait, TableState newStat switch (newState) { case OFFLINE: op = TFateOperation.TABLE_OFFLINE; - if (tableName.equals(SystemTables.METADATA.tableName()) - || tableName.equals(SystemTables.ROOT.tableName())) { + if (SystemTables.containsTableName(tableName)) { throw new AccumuloException("Cannot set table to offline state"); } break; case ONLINE: op = TFateOperation.TABLE_ONLINE; - if (tableName.equals(SystemTables.METADATA.tableName()) - || tableName.equals(SystemTables.ROOT.tableName())) { + if (SystemTables.containsTableName(tableName)) { // Don't submit a Fate operation for this, these tables can only be online. return; } @@ -2241,6 +2239,10 @@ private void validatePropertiesToSet(Map opts, Map public void setTabletAvailability(String tableName, Range range, TabletAvailability availability) throws AccumuloSecurityException, AccumuloException { EXISTING_TABLE_NAME.validate(tableName); + if (SystemTables.containsTableName(tableName)) { + throw new AccumuloException("Cannot set set tablet availability for table " + tableName); + } + checkArgument(range != null, "range is null"); checkArgument(availability != null, "tabletAvailability is null");