meatballspaghetti commented on code in PR #5451:
URL: https://github.com/apache/accumulo/pull/5451#discussion_r2047424532
##########
server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java:
##########
@@ -178,7 +172,16 @@ public void cloneTable(TableId srcTableId, TableId
tableId, String tableName,
PropUtil.removeProperties(context, TablePropKey.of(tableId),
propertiesToExclude);
}
- public void removeTable(TableId tableId) throws KeeperException,
InterruptedException {
+ public void removeTable(TableId tableId, NamespaceId namespaceId)
+ throws KeeperException, InterruptedException,
AcceptableThriftTableOperationException {
+ try {
Review Comment:
I think you are referring to how I separate the handling of the table
mapping (`TableMapping.put`) and the table actions (`TableManager.addTable`,`
.cloneTable`, etc). In `PopulateZookeeper`, `CloneZookeeper` etc., the reason I
have separate calls to add the table to the table mapping, and then to create
the table, is to account for initialization. In `ZookeeperInitializer`, I
construct the initial mapping with the built-in tables existing from the start;
so at that point when `TableManager.prepareNewTableState` is called for the
built-in tables, it won't touch the already-existing pairs in the map.
For `removeTable`, it just happens to be the case that I don't have to make
that separation. So I allowed both the removal of table from table mapping, and
the deletion of the table, to be done in this one method. This also allows just
1 method call to be needed within all of the `undo` methods.
I can see that it is inconsistent. But I think there will be similar
complexity in any way this is handled since I have to work around the map
initialization (I did the same for the namespace mapping).
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
