You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "13MaxG (via GitHub)" <gi...@apache.org> on 2025/03/31 09:24:33 UTC
[PR] Fn Runner Watermark issue [beam]
13MaxG opened a new pull request, #34484:
URL: https://github.com/apache/beam/pull/34484
Proposing a solution to https://github.com/apache/beam/issues/26190 .
It appears the Flatten was not setting a watermark, which caused following steps not to execute.
The issue was returning errors on beam versions 2.39 onwards, and it potentially produced unstable results before 2.39.
There are meaningful TODOs mentioned:
`# TODO(robertwb): Possibly fuse multi-input flattens into one of the stages.`
`# TODO(pabloem): Consider case where there are various producers`
It makes parts of the code redundant, a refactoring could be considered.
For now, I would like to test if this change evaluate correctly.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "13MaxG (via GitHub)" <gi...@apache.org>.
13MaxG commented on code in PR #34484:
URL: https://github.com/apache/beam/pull/34484#discussion_r2025196301
##########
sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py:
##########
@@ -388,7 +388,8 @@ def create_stages(
translations.lift_combiners,
translations.expand_sdf,
translations.expand_gbk,
- translations.sink_flattens,
+ translations.fix_flatten_coders,
Review Comment:
The `fix_flatten_coders` was added to solve yaml unit tests.
It also mimics the `translations.standard_optimize_phases()` used by Portable Runner
Without it, the`YamlMappingTest::test_basic` yields
```
apache_beam.testing.util.BeamAssertException: Failed assert: [
Row(label='11a', isogeny='a'), Row(label='37a', isogeny='a'), Row(label='389a', isogeny='a')] == [BeamSchema_ccf257cb_1966_410e_8157_00cd826e7392(label='11a', isogeny='a'), BeamSchema_ccf257cb_1966_410e_8157_00cd826e7392(label='37a', isogeny='a'), BeamSchema_ccf257cb_1966_410e_8157_00cd826e7392(label='389a', isogeny='a')],
unexpected elements [BeamSchema_ccf257cb_1966_410e_8157_00cd826e7392(label='11a', isogeny='a'), BeamSchema_ccf257cb_1966_410e_8157_00cd826e7392(label='37a', isogeny='a'), BeamSchema_ccf257cb_1966_410e_8157_00cd826e7392(label='389a', isogeny='a')],
missing elements [Row(label='11a', isogeny='a'), Row(label='37a', isogeny='a'), Row(label='389a', isogeny='a')
]
```
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on code in PR #34484:
URL: https://github.com/apache/beam/pull/34484#discussion_r2025258852
##########
sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py:
##########
@@ -388,7 +388,8 @@ def create_stages(
translations.lift_combiners,
translations.expand_sdf,
translations.expand_gbk,
- translations.sink_flattens,
+ translations.fix_flatten_coders,
Review Comment:
It looks like the PreCommit YAML is still failing, can you please take a look>
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34484:
URL: https://github.com/apache/beam/pull/34484#issuecomment-2796745470
Reminder, please take a look at this pr: @claudevdm @damccorm
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #34484:
URL: https://github.com/apache/beam/pull/34484#discussion_r2027430192
##########
sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py:
##########
@@ -388,7 +388,8 @@ def create_stages(
translations.lift_combiners,
translations.expand_sdf,
translations.expand_gbk,
- translations.sink_flattens,
+ translations.fix_flatten_coders,
Review Comment:
I think this maybe doesn't work because fix_flatten_coders assumes that the flattens will eventually be dealt with by sink_flattens. That may be what is causing the failures?
##########
sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py:
##########
@@ -388,7 +388,8 @@ def create_stages(
translations.lift_combiners,
translations.expand_sdf,
translations.expand_gbk,
- translations.sink_flattens,
+ translations.fix_flatten_coders,
+ # translations.sink_flattens,
Review Comment:
I think it would be better to try to fix the underlying issue. This has the potential to introduce new encoding/decoding problems which it may be possible to generally avoid
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34484:
URL: https://github.com/apache/beam/pull/34484#issuecomment-2769789580
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @claudevdm for label python.
R: @damccorm for label build.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #34484:
URL: https://github.com/apache/beam/pull/34484#issuecomment-2797106704
waiting on author
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34484:
URL: https://github.com/apache/beam/pull/34484#issuecomment-2984057267
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34484:
URL: https://github.com/apache/beam/pull/34484#issuecomment-2765901720
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on code in PR #34484:
URL: https://github.com/apache/beam/pull/34484#discussion_r2025156322
##########
sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py:
##########
@@ -388,7 +388,8 @@ def create_stages(
translations.lift_combiners,
translations.expand_sdf,
translations.expand_gbk,
- translations.sink_flattens,
+ translations.fix_flatten_coders,
Review Comment:
Why was this added?
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34484:
URL: https://github.com/apache/beam/pull/34484#issuecomment-2962553788
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on code in PR #34484:
URL: https://github.com/apache/beam/pull/34484#discussion_r2025261569
##########
sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py:
##########
@@ -388,7 +388,8 @@ def create_stages(
translations.lift_combiners,
translations.expand_sdf,
translations.expand_gbk,
- translations.sink_flattens,
+ translations.fix_flatten_coders,
+ # translations.sink_flattens,
Review Comment:
To be clear, this PR isn't fixing the underlying issue, rather it is disabling the optimization?
Can we add a comment referencing why this is disabled, with reference to the bug?
Also I am not sure if the tradeoff between disabling this optimization is worth fixing this specific edge-case, maybe @damccorm has thoughts?
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fn Runner Watermark issue [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #34484: Fn Runner Watermark issue
URL: https://github.com/apache/beam/pull/34484
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org