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

   Note: this PR was auto-generated by tooling. Before moving it out of draft, 
the author has done a full review of the PR, specifically evaluating it on the 
following criteria:
   
   - The PR is an appropriate fix for the issue (e.g. we're not changing a test 
to avoid a bug)
   - The PR is a simple fix to the issue (e.g. we're not adding infra when we 
could just modify a test)
   - The PR is not covering for other problems (e.g. if we're bumping timeouts, 
it is not because we're covering for a performance problem)
     
   Full details of the methodology of generation and review can be found here - 
https://docs.google.com/document/d/14lj7ECVov4T4EdwmIzNlWrRGE3lLDMOwX5xUMGzaLoc/edit?usp=sharing
   
   If you have concerns with the PR or with the underlying methodology used to 
generate this, please comment below - they will be addressed by the author not 
the tooling :)
   
   ## Summary
   
   - Annotates 
`GroupByKeyTest.BasicTests.testAfterProcessingTimeContinuationTriggerUsingState`
 with `@BeamParallelJunit4Runner.SerialTest` and increases `waitMillis` from 
`500` to `2000`.
   
   ## Root Cause
   
   In `PostCommit Java ValidatesRunner Direct JavaVersions` ([run 
43366517998](https://github.com/apache/beam/actions/runs/43366517998/job/131692083258)),
 `GroupByKeyTest$BasicTests > 
testAfterProcessingTimeContinuationTriggerUsingState` failed with:
   
   ```
   java.lang.AssertionError: Second Triggered 
sum/Values/Values/Map/ParMultiDo(Anonymous).output:
   Expected: iterable with items [<42>] in any order
        but: no item matches: <42> in []
   ```
   
   `testAfterProcessingTimeContinuationTriggerUsingState` cannot use 
`TestStream` processing-time advancement (because it runs across 
`ValidatesRunner` suites), so it holds the event-time watermark using a real 
wall-clock `Thread.sleep(waitMillis)` (`waitMillis = 500`) inside an `@OnTimer` 
callback (`onDelay`) while two sequential downstream `GroupByKey` transforms 
(`Triggered sum` and `Second Triggered sum`) fire 
`AfterProcessingTime.pastFirstElementInPane().plusDelayOf(Duration.millis(1))` 
early panes 
(`PAssert.that(triggeredSums).inEarlyGlobalWindowPanes().containsInAnyOrder(42)`).
   
   After `GroupByKeyTest.BasicTests` was migrated to 
`@RunWith(BeamParallelJunit4Runner.class)` (#40292) and 
`:runners:direct-java:validatesRunner` enabled `maxParallelForks = 4` with 
`beam.test.parallelThreads = 8` (#40324), up to 32 `DirectRunner` pipelines 
execute concurrently on a 4-vCPU runner. Under that CPU contention, propagating 
elements and firing processing-time timers across two consecutive `GroupByKey` 
stages can take longer than the 500ms wall-clock sleep in `onDelay`. When 
`onDelay` wakes up first, the input watermark advances to 
`TIMESTAMP_MAX_VALUE`, causing `Second Triggered sum` to emit its result in an 
`ON_TIME` pane rather than an `EARLY` pane.
   
   Annotating the test with `@BeamParallelJunit4Runner.SerialTest` and 
increasing `waitMillis` to `2000` prevents intra-class thread contention during 
the real-time sleep and provides sufficient headroom for both processing-time 
trigger stages to fire before the watermark is released.
   
   ## Verification
   
   - Verified locally with `./gradlew :sdks:java:core:spotlessCheck 
:runners:direct-java:validatesRunner --tests 
"org.apache.beam.sdk.transforms.GroupByKeyTest\$BasicTests.testAfterProcessingTimeContinuationTriggerUsingState"
 :sdks:java:core:checkstyleTest`.
   
   ### Verification
   - Triggered `beam_PostCommit_Java_ValidatesRunner_Direct_JavaVersions.yml` 
workflow run on branch: 
[`43379443238`](https://github.com/apache/beam/actions/runs/43379443238)
   


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