Skip to content

[BEAM-11377] Fix retry & cleanup issues. #14026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
pabloem merged 1 commit into from
Feb 24, 2021
Merged

[BEAM-11377] Fix retry & cleanup issues. #14026

pabloem merged 1 commit into from
Feb 24, 2021

Conversation

@tysonjh
Copy link
Contributor

@tysonjh tysonjh commented Feb 19, 2021

  • Attempt to reduce 'Connection Reset' errors with mvn wagon flags.
  • Fix shutdown process that fails when the job is not in state Running.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • 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.

Post-Commit Tests Status (on master branch)

Lang SDK Dataflow Flink Samza Spark Twister2
Go Build Status --- Build Status --- Build Status ---
Java Build Status Build Status
Build Status
Build Status		 Build Status
Build Status
Build Status
Build Status		 Build Status Build Status
Build Status
Build Status		 Build Status
Python Build Status
Build Status
Build Status		 Build Status
Build Status
Build Status		 Build Status
Build Status		 --- Build Status ---
XLang Build Status Build Status Build Status --- Build Status ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website Whitespace Typescript
Non-portable Build Status Build Status
Build Status
Build Status
Build Status		 Build Status Build Status Build Status Build Status
Portable --- Build Status --- --- --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

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

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 19, 2021

Run Dataflow PostRelease

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 19, 2021

Run Python_PVR_Flink PreCommit

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 19, 2021

Run Dataflow PostRelease

@codecov
Copy link

codecov bot commented Feb 20, 2021 •
edited
Loading

Codecov Report

Merging #14026 (baf927c) into master (31e1b84) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14026      +/-   ##
==========================================
+ Coverage   82.97%   82.99%   +0.01%     
==========================================
  Files         469      469              
  Lines       58294    58298       +4     
==========================================
+ Hits        48371    48382      +11     
+ Misses       9923     9916       -7
Impacted Files Coverage Δ
sdks/python/apache_beam/dataframe/frame_base.py 90.50% <0.00%> (-0.05%) ⬇️
...apache_beam/portability/api/beam_runner_api_pb2.py 100.00% <0.00%> (ø)
...hon/apache_beam/runners/worker/bundle_processor.py 93.95% <0.00%> (+0.12%) ⬆️
sdks/python/apache_beam/io/localfilesystem.py 92.42% <0.00%> (+0.75%) ⬆️
.../python/apache_beam/transforms/periodicsequence.py 98.24% <0.00%> (+1.75%) ⬆️
...che_beam/runners/interactive/interactive_runner.py 92.72% <0.00%> (+1.81%) ⬆️
.../python/apache_beam/testing/test_stream_service.py 93.18% <0.00%> (+4.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31e1b84...baf927c. Read the comment docs.

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 20, 2021

Run Dataflow PostRelease

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 20, 2021

Run Dataflow PostRelease

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 22, 2021

Run Dataflow PostRelease

[BEAM-11377] Fix retry & cleanup issues.
baf927c 
- Also fixes [BEAM-9970].
- Attempt to reduce 'Connection Reset' errors with mvn wagon flags.
- Fix shutdown process that fails when the job is not in state Running.
@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 22, 2021

Run Dataflow PostRelease

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 22, 2021

Run Java_Examples_Dataflow_Java11 PreCommit

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 22, 2021

Run Python PreCommit

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 22, 2021

Run PythonDocker PreCommit

@tysonjh
Copy link
Contributor Author

tysonjh commented Feb 22, 2021

R: @pabloem

pabloem
pabloem reviewed Feb 23, 2021
View reviewed changes
Copy link
Member

@pabloem pabloem left a comment

Choose a reason for hiding this comment

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

LGTM. Just one question.

release/src/main/groovy/mobilegaming-java-dataflow.groovy
Comment on lines +113 to +120
t.run("""RUNNING_JOB=`gcloud dataflow jobs list | grep ${jobName} | grep Running | cut -d' ' -f1`
if [ ! -z "\${RUNNING_JOB}" ]
then
gcloud dataflow jobs cancel \${RUNNING_JOB}
else
echo "Job '${jobName}' is not running."
fi
""")
Copy link
Member

@pabloem pabloem Feb 23, 2021

Choose a reason for hiding this comment

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

does it make sense to retry? are jobs here usually finished/cancelled, or pending?

Copy link
Contributor Author

@tysonjh tysonjh Feb 24, 2021

Choose a reason for hiding this comment

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

Jobs were finished/cancelled during my testing. I didn't see any in pending, there are a lot of sleeps/loops above but I suppose it could happen. We could search for 'Running|Pending' if that makes sense.

I'm not sure a retry for cancelling will be that helpful here. If the job isn't present in the list as Running, or Pending, it won't need cancelling right?

@tysonjh tysonjh requested a review from pabloem February 24, 2021 21:46
@pabloem pabloem merged commit 8bbdeef into apache:master Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pabloem pabloem Awaiting requested review from pabloem

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tysonjh @pabloem