shunping opened a new pull request, #36166: URL: https://github.com/apache/beam/pull/36166
The current implementation of the `onFire` method for the `TriggerAfterEach` trigger in `strategy.go` has a side-effect that can cause multiple sub-triggers to be marked as finished within a single invocation. https://github.com/apache/beam/blob/d0e48e202403c2b1b96d8b170a6e575682332a31/sdks/go/pkg/beam/runners/prism/internal/engine/strategy.go#L305-L314 The `onFire` function iterates through its sub-triggers. When a sub-trigger (let's call it A) fires and completes, the loop continues to the next sub-trigger (B) and calls `B.onFire()` within the same execution. This is incorrect, as the `TriggerAfterEach` should only advance to the next sub-trigger on a subsequent onFire call. This behavior is not detected by the existing test suite because the sub-triggers used in the tests have conditions that prevent them from firing immediately. For example, TriggerElementCounts (below) will not fire if its element count has not been reached. https://github.com/apache/beam/blob/d0e48e202403c2b1b96d8b170a6e575682332a31/sdks/go/pkg/beam/runners/prism/internal/engine/strategy.go#L358-L366. However, there is an edge case if B is `TriggerAny{TriggerAlways{}}`. In this case, the call of `B.onFire()` will marked B as finished. This results in both A and B being marked as finished in one onFire call to the parent `TriggerAfterEach`, which is not the intended behavior -- 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: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
