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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
