[GitHub] [accumulo] keith-turner commented on a change in pull request #2337: Fix stuck batch writer on OutOfMemoryError (#2331)

[GitBox <gi...@apache.org>]

keith-turner commented on a change in pull request #2337:
URL: https://github.com/apache/accumulo/pull/2337#discussion_r741034406



##########
File path: core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java
##########
@@ -763,7 +763,7 @@ public void run() {
               log.trace("{} - binning {} mutations", Thread.currentThread().getName(),
                   mutationsToSend.size());
               addMutations(mutationsToSend);
-            } catch (Exception e) {
+            } catch (Throwable e) {
               updateUnknownErrors("Error processing mutation set", e);

Review comment:
       If user code calls a library which creates a thread, it seems reasonable that the library should make a best effort attempt to get any errors that occur in that thread back to the user code.  Without this users can never automate their own responses to errors.  It seems like the argument against this is that it may not work therefore do not even make the attempt.
   
   When using Java standard libraries Throwable is caught.  Consider the following code, it will catch Throwable and get the error back to the user.
   
   ```java
        ExecutorService es = Executors.newFixedThreadPool(2);
        
        Callable<Void> callable = () -> {
          throw new Error("An error");
        };
        
        
        Future<Void> future = es.submit(callable);
        
        try {
         future.get();
       } catch (Exception e) {
         e.printStackTrace();
       } 
   ```
   
   The following stack trace is printed.
   
   ```
   java.util.concurrent.ExecutionException: java.lang.Error: An error
   	at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
   	at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
   	at org.apache.accumulo.core.clientImpl.TestErr.main(TestErr.java:21)
   Caused by: java.lang.Error: An error
   	at org.apache.accumulo.core.clientImpl.TestErr.lambda$0(TestErr.java:14)
   	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
   	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
   	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
   	at java.base/java.lang.Thread.run(Thread.java:832)
   ```




-- 
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-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org