Skip to content

Conversation

@robertwb
Copy link
Contributor

This will allow runners to act more quickly on failures rather than wait for all workers to die before the exiting the container.


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.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit 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
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI.

This will allow runners to act more quickly on failures
rather than wait for all workers to die before the exiting
the container.
@codecov
Copy link

codecov bot commented Feb 14, 2023

Codecov Report

Merging #25473 (e999995) into master (d20d0b0) will decrease coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #25473      +/-   ##
==========================================
- Coverage   72.96%   72.79%   -0.17%     
==========================================
  Files         745      749       +4     
  Lines       99174    99543     +369     
==========================================
+ Hits        72362    72465     +103     
- Misses      25446    25712     +266     
  Partials     1366     1366              
Flag Coverage Δ
python 82.18% <ø> (-0.30%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdks/python/apache_beam/ml/inference/utils.py 92.15% <0.00%> (-7.85%) ⬇️
sdks/python/apache_beam/utils/interactive_utils.py 95.12% <0.00%> (-2.44%) ⬇️
...ks/python/apache_beam/runners/worker/data_plane.py 87.57% <0.00%> (-1.70%) ⬇️
...python/apache_beam/runners/worker/worker_status.py 75.33% <0.00%> (-1.34%) ⬇️
...eam/runners/portability/fn_api_runner/execution.py 92.49% <0.00%> (-0.64%) ⬇️
sdks/python/apache_beam/runners/direct/executor.py 96.46% <0.00%> (-0.55%) ⬇️
sdks/python/apache_beam/runners/common.py 88.33% <0.00%> (-0.27%) ⬇️
...ks/python/apache_beam/runners/worker/sdk_worker.py 89.08% <0.00%> (-0.17%) ⬇️
sdks/python/apache_beam/io/filesystem.py 88.59% <0.00%> (-0.09%) ⬇️
...hon/apache_beam/runners/worker/bundle_processor.py 93.49% <0.00%> (-0.06%) ⬇️
... and 26 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link

@antonbobkov antonbobkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

// DoFns throwing exceptions.
errorCount += 1
if errorCount < 4 {
log.Printf("Python (worker %v) exited: %v", workerId, err)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps add more information in the logs about retries/failures. Maybe something along the lines of "Python (worker %v) exited: %v, retrying process." and "Python (worker %v) exited: %v, out of retries: restarting SDK container."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Done.

@robertwb
Copy link
Contributor Author

PTAL

@robertwb robertwb merged commit 8cfee7d into apache:master Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants