damondouglas opened a new pull request, #24669:
URL: https://github.com/apache/beam/pull/24669

   This PR closes #24664 by adding an explicit and ignored InterrptedException 
catch and moving the shutdownNow of sdkHarnessExecutor and serverExecutor in 
the finally clause.
   
   **What remains uncertain to me is why if we see an ExecutionException higher 
in the stack trace caused by an InterruptedException and this reduces my 
confidence that I 1) fixed #24664. 2) Worse I worry whether I introduced a new 
bug.  Nonetheless, please see my attempt to diagnose the root cause below.** 
   
   https://github.com/apache/beam/actions/runs/3690463659/jobs/6247487642 
reports the error stack trace:
   ```
   org.apache.beam.fn.harness.jmh.ProcessBundleBenchmarkTest > 
testStateWithCaching FAILED
       java.util.concurrent.ExecutionException at 
ProcessBundleBenchmarkTest.java:54
           Caused by: java.lang.RuntimeException at 
ProcessBundleBenchmark.java:175
               Caused by: java.lang.RuntimeException at 
UnboundedScheduledExecutorService.java:316
                   Caused by: java.lang.InterruptedException at 
FutureTask.java:418
   ```
   
   
[ProcessBundleBenchmarkTest.java:54](https://github.com/apache/beam/blob/master/sdks/java/harness/jmh/src/test/java/org/apache/beam/fn/harness/jmh/ProcessBundleBenchmarkTest.java#L54)
 occurs when the code invokes tearDown:
   ```
        StatefulTransform transform = new StatefulTransform();
        new ProcessBundleBenchmark().testStateWithCaching(transform);
   -->  transform.tearDown();
   ```
   
   Since `StatefulTransform` extends `SdkHarness`, we find the tearDown method 
code at 
[ProcessBundleBenchmark.java#L200-L201](https://github.com/apache/beam/blob/master/sdks/java/harness/jmh/src/main/java/org/apache/beam/fn/harness/jmh/ProcessBundleBenchmark.java#L200-L201).
  Noteably, an `//expected` code comment suggests that at tearDown, we expect 
an InterruptedException.  However, when sdkHarnessExecutorFuture.get(); is 
invoked, we still see `Caused by: java.lang.InterruptedException at 
FutureTask.java:418`
   ```
         try {
           sdkHarnessExecutorFuture.get();
         } catch (ExecutionException e) {
           if (e.getCause() instanceof RuntimeException
     ----> && e.getCause().getCause() instanceof InterruptedException) {
     ----> // expected
           } else {
             throw e;
           }
         }
   ```
   
   By refactoring as follows, we may have prevented the re-throwing of 
`InterruptedException`.
   ```
   catch (InterruptedException ignored) {
           // expected
         } catch (Exception e) {
           throw new RuntimeException(e);
         } finally {
           sdkHarnessExecutor.shutdownNow();
           serverExecutor.shutdownNow();
   }
   ```
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
    - [ ] Mention the appropriate issue in your description (for example: 
`addresses #123`), if applicable. This will automatically add a link to the 
pull request in the issue. If you would like the issue to automatically close 
on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more 
tips on [how to make review process 
smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
   
   To check the build health, please visit 
[https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   
------------------------------------------------------------------------------------------------
   [![Build python source distribution and 
wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python 
tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java 
tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go 
tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more 
information about GitHub Actions CI.
   


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

Reply via email to