ctubbsii commented on code in PR #5433:
URL: https://github.com/apache/accumulo/pull/5433#discussion_r2045471939
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##########
@@ -1463,7 +1463,7 @@ public void offline(String tableName, boolean wait)
EXISTING_TABLE_NAME.validate(tableName);
TableId tableId = context.getTableId(tableName);
- List<ByteBuffer> args =
Arrays.asList(ByteBuffer.wrap(tableId.canonical().getBytes(UTF_8)));
+ List<ByteBuffer> args =
Arrays.asList(ByteBuffer.wrap(tableName.getBytes(UTF_8)));
Review Comment:
This changes the semantics of the RPC argument from that of a tableId to
that of a tableName. This makes this RPC incompatible across a bugfix release,
preventing rolling upgrades, which is something we generally support for bugfix
releases.
##########
server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java:
##########
@@ -410,7 +414,11 @@ public void executeFateOperation(TInfo tinfo, TCredentials
c, long opid, FateOpe
case TABLE_OFFLINE: {
TableOperation tableOp = TableOperation.OFFLINE;
validateArgumentCount(arguments, tableOp, 1);
- final var tableId = validateTableIdArgument(arguments.get(0), tableOp,
NOT_ROOT_TABLE_ID);
+ String tableName =
+ validateName(arguments.get(0), tableOp,
EXISTING_TABLE_NAME.and(NOT_BUILTIN_TABLE));
+
+ final TableId tableId =
+ ClientServiceHandler.checkTableId(manager.getContext(), tableName,
tableOp);
Review Comment:
The tableId was already resolved on the client side. Resolving it a second
time here, makes it possible for the client side to refer to a different table
than that of the server side if some table renames/creates happened
concurrently. It also creates an extra lookup that shouldn't be necessary.
The current code using the table ID was fine, but just needed to verify that
the TableId didn't refer to the metadata table. Other builtin tables may still
be available to go online/offline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
