-
Notifications
You must be signed in to change notification settings - Fork 16.4k
support retry multiplier parameter #56866
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.
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
support retry multiplier parameter #56866
Conversation
ashb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a new parameter for this, or could we allow retry_exponential_backoff to be set to an number instead to control it? (retry_exponential_backoff=True -> 2), retry_exponential_backoff=False -> 0, so its still falsey.
I am also tempted to say this should be a new ser dag version.
0BVer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
Show resolved
Hide resolved
|
One last moment idea... should we also add something to |
Co-authored-by: 김영준 <youngjun.kim@kurlycorp.com>
129ff5c to
bc657ea
Compare
|
Thanks for the suggestion, I looked into this but retry_exponential_backoff is a Dag operator parameter and not an airflow.cfg configuration setting therefore I think it wouldn't apply here. That said, I did find a bug that was addressed in bc657ea. When converting booleans to floats, float(True) returned 1.0 instead of the original default 2.0 multiplier. This would have completely broken exponential backoff for existing Dags using retry_exponential_backoff=True |
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM . @jscheffl ?
Ah, yeah 🤦 ypu are right is not a config entry :-D |
* support retry multiplier parameter * new param in serialization schema.json * fix test: retry multipler param matches schema default * replace back param with multipler * missed type hints * prek regen type hints * Update airflow-core/src/airflow/models/taskinstance.py Co-authored-by: 김영준 <youngjun.kim@kurlycorp.com> * newsfragment addition & backwards compatability * fix ci/cd rebase prek --------- Co-authored-by: 김영준 <youngjun.kim@kurlycorp.com>
* support retry multiplier parameter * new param in serialization schema.json * fix test: retry multipler param matches schema default * replace back param with multipler * missed type hints * prek regen type hints * Update airflow-core/src/airflow/models/taskinstance.py Co-authored-by: 김영준 <youngjun.kim@kurlycorp.com> * newsfragment addition & backwards compatability * fix ci/cd rebase prek --------- Co-authored-by: 김영준 <youngjun.kim@kurlycorp.com>
* support retry multiplier parameter * new param in serialization schema.json * fix test: retry multipler param matches schema default * replace back param with multipler * missed type hints * prek regen type hints * Update airflow-core/src/airflow/models/taskinstance.py Co-authored-by: 김영준 <youngjun.kim@kurlycorp.com> * newsfragment addition & backwards compatability * fix ci/cd rebase prek --------- Co-authored-by: 김영준 <youngjun.kim@kurlycorp.com>
Summary
Add configurable retry_delay_multiplier parameter to BaseOperator allowing users to customize the exponential backoff multiplier for task retries (previously hardcoded to 2).
Solution
Testing
Tested with breeze and unit tests with screenshots below:



Related Issues
Closes #56537