keith-turner commented on code in PR #5715:
URL: https://github.com/apache/accumulo/pull/5715#discussion_r2201797447


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##########
@@ -1123,7 +1123,7 @@ private Map<String,String> 
modifyPropertiesUnwrapped(String tableName,
     } catch (AccumuloException ae) {
       Throwable cause = ae.getCause();
       if (cause instanceof TableNotFoundException) {
-        throw new TableNotFoundException(null, tableName, null, ae);
+        throw (TableNotFoundException) cause;
       }

Review Comment:
   > This was causing a weird issue that was hard to track down. Seems more 
correct to cast the original exception instead of creating a new one, and this 
fixed an issue I was seeing.
   
   What was the problem you were seeing?
   
   Tried making the following change locally to this branch to see if we could 
add the ignored exception as a suppressed exception.  Also printed the stack 
trace just to see what it looks like when running the IT.
   
   ```
   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 46ced154e3..fdd706c48c 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,6 +1123,8 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
        } catch (AccumuloException ae) {
          Throwable cause = ae.getCause();
          if (cause instanceof TableNotFoundException) {
   +        cause.addSuppressed(ae);
   +        cause.printStackTrace();
            throw (TableNotFoundException) cause;
          }
          throw ae;
   
   ```
   
   It looks like the following.  Its not pretty, but it has all the 
information.  It includes the fact that the thread went through 
`org.apache.accumulo.core.rpc.clients.TServerClient.execute(TServerClient.java:189)`
 which would be lost w/o adding the suppressed.  It seems to do a good job of 
dealing the redundant information in the supressed exception.   So if we are 
going throw the cause then maybe adding the original as a supressed exception 
on the cause would avoid losing information.
   
   ```
   org.apache.accumulo.core.client.TableNotFoundException: Table 
ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0.1 does not exist 
(Namespace ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0 does 
not exist.)
        at 
org.apache.accumulo.core.rpc.clients.TServerClient.execute(TServerClient.java:187)
        at 
org.apache.accumulo.core.rpc.clients.ClientServiceThriftClient.execute(ClientServiceThriftClient.java:53)
        at 
org.apache.accumulo.core.clientImpl.TableOperationsImpl.tryToModifyProperties(TableOperationsImpl.java:1057)
        at 
org.apache.accumulo.core.clientImpl.TableOperationsImpl.modifyProperties(TableOperationsImpl.java:1095)
        at 
org.apache.accumulo.core.clientImpl.TableOperationsImpl.modifyPropertiesUnwrapped(TableOperationsImpl.java:1122)
        at 
org.apache.accumulo.core.clientImpl.TableOperationsImpl.setLocalityGroups(TableOperationsImpl.java:1222)
        at 
org.apache.accumulo.test.NamespacesIT_SimpleSuite.lambda$verifyTableOperationsExceptions$77(NamespacesIT_SimpleSuite.java:1134)
        at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:53)
        at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:35)
        at org.junit.jupiter.api.Assertions.assertThrows(Assertions.java:3128)
        at 
org.apache.accumulo.test.NamespacesIT_SimpleSuite.assertNoTableNoNamespace(NamespacesIT_SimpleSuite.java:1214)
        at 
org.apache.accumulo.test.NamespacesIT_SimpleSuite.verifyTableOperationsExceptions(NamespacesIT_SimpleSuite.java:1134)
        at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at 
org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:775)
        at 
org.junit.platform.commons.support.ReflectionSupport.invokeMethod(ReflectionSupport.java:479)
        at 
org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
        at 
org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
        at 
org.junit.jupiter.api.AssertTimeoutPreemptively.lambda$submitTask$3(AssertTimeoutPreemptively.java:95)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at 
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)
        Suppressed: org.apache.accumulo.core.client.AccumuloException: 
org.apache.accumulo.core.client.TableNotFoundException: Table 
ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0.1 does not exist 
(Namespace ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0 does 
not exist.)
                at 
org.apache.accumulo.core.rpc.clients.TServerClient.execute(TServerClient.java:189)
                ... 24 more
        Caused by: [CIRCULAR REFERENCE: 
org.apache.accumulo.core.client.TableNotFoundException: Table 
ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0.1 does not exist 
(Namespace ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0 does 
not exist.)]
   Caused by: org.apache.accumulo.core.client.NamespaceNotFoundException: 
Namespace ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0.1 does 
not exist
        ... 25 more
   Caused by: ThriftTableOperationException(tableId:null, 
tableName:ns_NamespacesIT_SimpleSuite_verifyTableOperationsExceptions0.1, 
op:null, type:NAMESPACE_NOTFOUND, description:null)
        at 
org.apache.accumulo.core.clientImpl.thrift.ClientService$getVersionedTableProperties_result$getVersionedTableProperties_resultStandardScheme.read(ClientService.java:34339)
        at 
org.apache.accumulo.core.clientImpl.thrift.ClientService$getVersionedTableProperties_result$getVersionedTableProperties_resultStandardScheme.read(ClientService.java:34315)
        at 
org.apache.accumulo.core.clientImpl.thrift.ClientService$getVersionedTableProperties_result.read(ClientService.java:34245)
        at org.apache.thrift.TServiceClient.receiveBase(TServiceClient.java:93)
        at 
org.apache.accumulo.core.clientImpl.thrift.ClientService$Client.recv_getVersionedTableProperties(ClientService.java:904)
        at 
org.apache.accumulo.core.clientImpl.thrift.ClientService$Client.getVersionedTableProperties(ClientService.java:889)
        at 
org.apache.accumulo.core.clientImpl.TableOperationsImpl.lambda$tryToModifyProperties$14(TableOperationsImpl.java:1058)
        at 
org.apache.accumulo.core.rpc.clients.TServerClient.execute(TServerClient.java:172)
        ... 24 more
   ```
   
   



-- 
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

Reply via email to