shunping commented on PR #40925:
URL: https://github.com/apache/beam/pull/40925#issuecomment-6665106909
> Same question in #40549, is there any example pipeline that could trigger
this issue?
>
> Defensive coding can be added anywhere, but given that
>
> * currently it's still failing in subsequent code path as well
> * and if it's not encountered in real scenario
>
> I'm leaning to leave core/hot code path as is unless a good reason and
value for it. Window instantiation is light but hot path if it's per-element
creation - see #40910
Thanks for sharing this.
I think `GlobalWindow` is different than `SlidingWindows` we changed here.
- `GlobalWindow` is built for each element via `GlobalWindows.assign()`.
Notice that `GlobalWinwindows` is `WindowFn` and `GlobalWindow` is `Window`.
- `SlidingWindows` (and `FixedWindows`/`Sessions`) is `WindowFn`, and it
built per-element `IntervalWindow`. The `SlidingWindows` instance is built once
and reused for every assign call.
Below the places where `SlidingWindows.__init__` is called:
- Building the pipeline
- Where: user code, e.g. beam.WindowInto(SlidingWindows(60, 10)),
yaml_provider.py, Nexmark queries
- Frequency: once per transform when the pipeline is built
- Converting proto → Python objects
- Where: SlidingWindows.from_runner_api_parameter, on the submission side
and on the SDK worker
- Frequency: once each time a windowing strategy is deserialized
- Worker bundle processor setup
- Where: create_assign_windows → Windowing.from_runner_api
- Frequency: once per bundle processor (these are cached and reused across
bundles)
So it is actually not on the core code path. Therefore, I think it is ok to
add the check to prevent invalid user input. WDYT? @Abacn
--
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]
