ctubbsii commented on code in PR #5443: URL: https://github.com/apache/accumulo/pull/5443#discussion_r2023527081
########## server/manager/src/main/java/org/apache/accumulo/manager/tableOps/namespace/delete/DeleteNamespace.java: ########## @@ -41,7 +49,26 @@ public long isReady(long id, Manager environment) throws Exception { } @Override - public Repo<Manager> call(long tid, Manager environment) { + public Repo<Manager> call(long tid, Manager environment) + throws AcceptableThriftTableOperationException { + try { + // Namespaces.getTableIds(..) uses the following cache, clear the cache to force + // Namespaces.getTableIds(..) to read from zookeeper. + environment.getContext().clearTableListCache(); + // Since we have a write lock on the namespace id and all fate table operations get a read + // lock on the namespace id there is no need to worry about a fate operation concurrently + // changing table ids in this namespace. + List<TableId> tableIdsInNamespace = + Namespaces.getTableIds(environment.getContext(), namespaceId); + if (!tableIdsInNamespace.isEmpty()) { + throw new AcceptableThriftTableOperationException(null, null, TableOperation.DELETE, + TableOperationExceptionType.OTHER, Review Comment: Could we add a different TableOperationExceptionType instead of relying on OTHER? ########## core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java: ########## @@ -158,6 +163,12 @@ public void delete(String namespace) throws AccumuloException, AccumuloSecurityE try { doNamespaceFateOperation(FateOperation.NAMESPACE_DELETE, args, opts, namespace); + } catch (AccumuloException ae) { + if (ae.getMessage().contains(TABLES_EXISTS_IN_NAMESPACE_INDICATOR)) { + throw new NamespaceNotEmptyException(namespaceId.canonical(), namespace, null, ae); + } else { + throw ae; + } } catch (NamespaceExistsException e) { // should not happen throw new AssertionError(e); Review Comment: Instead of relying on the special String to detect this situation, you could instead use this existing NamespaceExistsException as a special case with the semantics of "namespace still exists (because it wasn't empty and couldn't be deleted)". -- 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