This is an automated email from the ASF dual-hosted git repository. krathbun pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push: new 0cf01e3365 Fix broken NamespacesIT, table ops exceptions changes (#5715) 0cf01e3365 is described below commit 0cf01e3365b2493e84c9a3e243a26bf8acec80d4 Author: Kevin Rathbun <krath...@apache.org> AuthorDate: Fri Jul 25 09:36:12 2025 -0400 Fix broken NamespacesIT, table ops exceptions changes (#5715) * Fix broken IT: NamespacesIT * TServerClient.execute() and executeVoid() exception changes: Previously: * Thrown to caller: ThriftTableOperationException wrapped in an AccumuloException Now: * If the ThriftTableOperationException is the result of the table or namespace not existing, throw to caller: TableNotFoundException wrapped in an AccumuloException * If the ThriftTableOperationException occurs for any other reason, throw to caller: ThriftTableOperationException wrapped in an AccumuloException Callers in TableOperationsImpl, NamespaceOperationsImpl, and SecurityOperationsImpl: * would previously handle the ThriftTableOperationException wrapped in an AccumuloException, throwing a more appropriate exception to the user (e.g., TableNotFoundException, NamespaceNotFoundException, AccumuloSecurityException) if the cause was the table or namespace not existing. Would throw the ThriftTableOperationException wrapped in an AccumuloException otherwise. * now handle the TableNotFoundException wrapped in an AccumuloException, throwing a more appropriate exception to the user (e.g., TableNotFoundException, NamespaceNotFoundException, AccumuloSecurityException) if needed. Throws the ThriftTableOperationException wrapped in an AccumuloException if it is not a TableNotFoundException. Effect of this change on InstanceOperationsImpl: * Callers in InstanceOperationsImpl call execute and executeVoid directly, throwing the exception as-is to the user. So, this does change functionality here as previously a user would see an AccumuloException which wraps a ThriftTableOperationException for any cause of the TTOE (table doesn't exist, namespace doesn't exist, other). Now, users will see an AccumuloException which wraps a TableNotFoundException if the cause was due to the namespace or table not existing; will see an Accu [...] --- .../core/clientImpl/NamespaceOperationsImpl.java | 45 +++++++++--------- .../core/clientImpl/SecurityOperationsImpl.java | 55 +++++++++++++--------- .../core/clientImpl/TableOperationsImpl.java | 50 +++++++------------- .../accumulo/core/rpc/clients/TServerClient.java | 29 ++++++++++++ .../accumulo/test/NamespacesIT_SimpleSuite.java | 1 + 5 files changed, 105 insertions(+), 75 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java index 34094cd2c2..1792208687 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java @@ -49,8 +49,6 @@ import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.client.admin.TableOperations; import org.apache.accumulo.core.clientImpl.thrift.SecurityErrorCode; import org.apache.accumulo.core.clientImpl.thrift.TVersionedProperties; -import org.apache.accumulo.core.clientImpl.thrift.TableOperationExceptionType; -import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException; import org.apache.accumulo.core.data.NamespaceId; import org.apache.accumulo.core.data.constraints.Constraint; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; @@ -299,13 +297,14 @@ public class NamespaceOperationsImpl extends NamespaceOperationsHelper { } catch (AccumuloSecurityException e) { throw e; } catch (AccumuloException e) { - Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) { - throw new NamespaceNotFoundException(ttoe); + Throwable eCause = e.getCause(); + if (eCause instanceof TableNotFoundException) { + Throwable tnfeCause = eCause.getCause(); + if (tnfeCause instanceof NamespaceNotFoundException) { + var nnfe = (NamespaceNotFoundException) tnfeCause; + nnfe.addSuppressed(e); + throw nnfe; } - throw e; } throw e; } catch (Exception e) { @@ -322,13 +321,14 @@ public class NamespaceOperationsImpl extends NamespaceOperationsHelper { return ThriftClientTypes.CLIENT.execute(context, client -> client .getNamespaceProperties(TraceUtil.traceInfo(), context.rpcCreds(), namespace)); } catch (AccumuloException e) { - Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) { - throw new NamespaceNotFoundException(ttoe); + Throwable eCause = e.getCause(); + if (eCause instanceof TableNotFoundException) { + Throwable tnfeCause = eCause.getCause(); + if (tnfeCause instanceof NamespaceNotFoundException) { + var nnfe = (NamespaceNotFoundException) tnfeCause; + nnfe.addSuppressed(e); + throw nnfe; } - throw e; } throw e; } catch (Exception e) { @@ -356,14 +356,17 @@ public class NamespaceOperationsImpl extends NamespaceOperationsHelper { return ThriftClientTypes.CLIENT.execute(context, client -> client.checkNamespaceClass(TraceUtil.traceInfo(), context.rpcCreds(), namespace, className, asTypeName)); - } catch (AccumuloSecurityException | AccumuloException e) { - Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) { - throw new NamespaceNotFoundException(ttoe); + } catch (AccumuloSecurityException e) { + throw e; + } catch (AccumuloException e) { + Throwable eCause = e.getCause(); + if (eCause instanceof TableNotFoundException) { + Throwable tnfeCause = eCause.getCause(); + if (tnfeCause instanceof NamespaceNotFoundException) { + var nnfe = (NamespaceNotFoundException) tnfeCause; + nnfe.addSuppressed(e); + throw nnfe; } - throw e; } throw e; } catch (Exception e) { diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java index 23114926ef..d074b1544e 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/SecurityOperationsImpl.java @@ -27,6 +27,7 @@ import java.util.Set; import org.apache.accumulo.core.client.AccumuloException; import org.apache.accumulo.core.client.AccumuloSecurityException; +import org.apache.accumulo.core.client.NamespaceNotFoundException; import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.client.admin.DelegationTokenConfig; import org.apache.accumulo.core.client.admin.SecurityOperations; @@ -64,17 +65,22 @@ public class SecurityOperationsImpl implements SecurityOperations { throws AccumuloException, AccumuloSecurityException { try { ThriftClientTypes.CLIENT.executeVoid(context, client -> exec.execute(client)); - } catch (AccumuloSecurityException | AccumuloException e) { - Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - // recast missing table - if (ttoe.getType() == TableOperationExceptionType.NOTFOUND) { - throw new AccumuloSecurityException(null, SecurityErrorCode.TABLE_DOESNT_EXIST); - } else if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) { - throw new AccumuloSecurityException(null, SecurityErrorCode.NAMESPACE_DOESNT_EXIST); - } else { - throw e; + } catch (AccumuloSecurityException e) { + throw e; + } catch (AccumuloException e) { + Throwable eCause = e.getCause(); + if (eCause instanceof TableNotFoundException) { + var tnfeCause = eCause.getCause(); + if (tnfeCause instanceof NamespaceNotFoundException) { + var ase = new AccumuloSecurityException(null, SecurityErrorCode.NAMESPACE_DOESNT_EXIST); + ase.addSuppressed(e); + throw ase; + } else if (tnfeCause instanceof ThriftTableOperationException + && ((ThriftTableOperationException) tnfeCause).getType() + == TableOperationExceptionType.NOTFOUND) { + var ase = new AccumuloSecurityException(null, SecurityErrorCode.TABLE_DOESNT_EXIST); + ase.addSuppressed(e); + throw ase; } } throw e; @@ -96,17 +102,22 @@ public class SecurityOperationsImpl implements SecurityOperations { throws AccumuloException, AccumuloSecurityException { try { return ThriftClientTypes.CLIENT.execute(context, client -> exec.execute(client)); - } catch (AccumuloSecurityException | AccumuloException e) { - Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - // recast missing table - if (ttoe.getType() == TableOperationExceptionType.NOTFOUND) { - throw new AccumuloSecurityException(null, SecurityErrorCode.TABLE_DOESNT_EXIST); - } else if (ttoe.getType() == TableOperationExceptionType.NAMESPACE_NOTFOUND) { - throw new AccumuloSecurityException(null, SecurityErrorCode.NAMESPACE_DOESNT_EXIST); - } else { - throw e; + } catch (AccumuloSecurityException e) { + throw e; + } catch (AccumuloException e) { + Throwable eCause = e.getCause(); + if (eCause instanceof TableNotFoundException) { + var tnfeCause = eCause.getCause(); + if (tnfeCause instanceof NamespaceNotFoundException) { + var ase = new AccumuloSecurityException(null, SecurityErrorCode.NAMESPACE_DOESNT_EXIST); + ase.addSuppressed(e); + throw ase; + } else if (tnfeCause instanceof ThriftTableOperationException + && ((ThriftTableOperationException) tnfeCause).getType() + == TableOperationExceptionType.NOTFOUND) { + var ase = new AccumuloSecurityException(null, SecurityErrorCode.TABLE_DOESNT_EXIST); + ase.addSuppressed(e); + throw ase; } } throw e; 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 ba8bba53f1..356fb6d129 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 @@ -1123,7 +1123,9 @@ public class TableOperationsImpl extends TableOperationsHelper { } catch (AccumuloException ae) { Throwable cause = ae.getCause(); if (cause instanceof TableNotFoundException) { - throw new TableNotFoundException(null, tableName, null, ae); + var tnfe = (TableNotFoundException) cause; + tnfe.addSuppressed(ae); + throw tnfe; } throw ae; } @@ -1181,16 +1183,10 @@ public class TableOperationsImpl extends TableOperationsHelper { .getTableConfiguration(TraceUtil.traceInfo(), context.rpcCreds(), tableName)); } catch (AccumuloException e) { Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - switch (ttoe.getType()) { - case NOTFOUND: - throw new TableNotFoundException(ttoe); - case NAMESPACE_NOTFOUND: - throw new TableNotFoundException(tableName, new NamespaceNotFoundException(ttoe)); - default: - throw e; - } + if (t instanceof TableNotFoundException) { + var tnfe = (TableNotFoundException) t; + tnfe.addSuppressed(e); + throw tnfe; } throw e; } catch (Exception e) { @@ -1208,16 +1204,10 @@ public class TableOperationsImpl extends TableOperationsHelper { .getTableProperties(TraceUtil.traceInfo(), context.rpcCreds(), tableName)); } catch (AccumuloException e) { Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - switch (ttoe.getType()) { - case NOTFOUND: - throw new TableNotFoundException(ttoe); - case NAMESPACE_NOTFOUND: - throw new TableNotFoundException(tableName, new NamespaceNotFoundException(ttoe)); - default: - throw e; - } + if (t instanceof TableNotFoundException) { + var tnfe = (TableNotFoundException) t; + tnfe.addSuppressed(e); + throw tnfe; } throw e; } catch (Exception e) { @@ -1798,18 +1788,14 @@ public class TableOperationsImpl extends TableOperationsHelper { return ThriftClientTypes.CLIENT.execute(context, client -> client.checkTableClass(TraceUtil.traceInfo(), context.rpcCreds(), tableName, className, asTypeName)); - } catch (AccumuloSecurityException | AccumuloException e) { + } catch (AccumuloSecurityException e) { + throw e; + } catch (AccumuloException e) { Throwable t = e.getCause(); - if (t instanceof ThriftTableOperationException) { - ThriftTableOperationException ttoe = (ThriftTableOperationException) t; - switch (ttoe.getType()) { - case NOTFOUND: - throw new TableNotFoundException(ttoe); - case NAMESPACE_NOTFOUND: - throw new TableNotFoundException(tableName, new NamespaceNotFoundException(ttoe)); - default: - throw e; - } + if (t instanceof TableNotFoundException) { + var tnfe = (TableNotFoundException) t; + tnfe.addSuppressed(e); + throw tnfe; } throw e; } catch (Exception e) { diff --git a/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java b/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java index e84a7beb34..3539b853e6 100644 --- a/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java +++ b/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java @@ -33,9 +33,12 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.apache.accumulo.core.client.AccumuloException; import org.apache.accumulo.core.client.AccumuloSecurityException; +import org.apache.accumulo.core.client.NamespaceNotFoundException; +import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.clientImpl.AccumuloServerException; import org.apache.accumulo.core.clientImpl.ClientContext; import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException; +import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException; import org.apache.accumulo.core.lock.ServiceLockData; import org.apache.accumulo.core.lock.ServiceLockData.ThriftService; import org.apache.accumulo.core.lock.ServiceLockPaths.AddressSelector; @@ -174,6 +177,19 @@ public interface TServerClient<C extends TServiceClient> { } catch (TTransportException tte) { LOG.debug("ClientService request failed " + server + ", retrying ... ", tte); sleepUninterruptibly(100, MILLISECONDS); + } catch (ThriftTableOperationException ttoe) { + TableNotFoundException tnfe; + switch (ttoe.getType()) { + case NOTFOUND: + tnfe = new TableNotFoundException(ttoe); + throw new AccumuloException(tnfe); + case NAMESPACE_NOTFOUND: + tnfe = new TableNotFoundException(ttoe.getTableName(), + new NamespaceNotFoundException(ttoe)); + throw new AccumuloException(tnfe); + default: + throw new AccumuloException(ttoe); + } } catch (TException e) { throw new AccumuloException(e); } finally { @@ -202,6 +218,19 @@ public interface TServerClient<C extends TServiceClient> { } catch (TTransportException tte) { LOG.debug("ClientService request failed " + server + ", retrying ... ", tte); sleepUninterruptibly(100, MILLISECONDS); + } catch (ThriftTableOperationException ttoe) { + TableNotFoundException tnfe; + switch (ttoe.getType()) { + case NOTFOUND: + tnfe = new TableNotFoundException(ttoe); + throw new AccumuloException(tnfe); + case NAMESPACE_NOTFOUND: + tnfe = new TableNotFoundException(ttoe.getTableName(), + new NamespaceNotFoundException(ttoe)); + throw new AccumuloException(tnfe); + default: + throw new AccumuloException(ttoe); + } } catch (TException e) { throw new AccumuloException(e); } finally { diff --git a/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java b/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java index c04a47cde6..4134d30fe7 100644 --- a/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java +++ b/test/src/main/java/org/apache/accumulo/test/NamespacesIT_SimpleSuite.java @@ -1160,6 +1160,7 @@ public class NamespacesIT_SimpleSuite extends SharedMiniClusterBase { assertNoNamespace(() -> ops.setProperty(namespace, "k", "v")); assertNoNamespace(() -> ops.testClassLoad(namespace, VersioningIterator.class.getName(), SortedKeyValueIterator.class.getName())); + assertNoNamespace(() -> ops.getNamespaceProperties(namespace)); // namespace operations that should throw a NamespaceExistsException assertNamespaceExists(() -> ops.create(Namespace.DEFAULT.name()));