-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Retry deadlocked transactions on deleting old rendered task fields #18616
Conversation
The query that deletes rendered old rendered task fields for MySQL can occasionally deadlock because it is unncesssary complex with a subquery (due to features missing in MySQL). This change adds DB retries to get rid of the deadlock (as is the recommended practice for MySQL). Fixes: apache#18512
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
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.
Whoops this is not working:
$ airflow db check
Traceback (most recent call last):
File "/usr/local/bin/airflow", line 33, in <module>
sys.exit(load_entry_point('apache-airflow', 'console_scripts', 'airflow')())
File "/opt/airflow/airflow/__main__.py", line 40, in main
args.func(args)
File "/opt/airflow/airflow/cli/cli_parser.py", line 47, in command
func = import_string(import_path)
File "/opt/airflow/airflow/utils/module_loading.py", line 32, in import_string
module = import_module(module_path)
File "/usr/local/lib/python3.6/importlib/__init__.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 994, in _gcd_import
File "<frozen importlib._bootstrap>", line 971, in _find_and_load
File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 678, in exec_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "/opt/airflow/airflow/cli/commands/db_command.py", line 24, in <module>
from airflow.utils import cli as cli_utils, db
File "/opt/airflow/airflow/utils/db.py", line 27, in <module>
from airflow.jobs.base_job import BaseJob # noqa: F401
File "/opt/airflow/airflow/jobs/__init__.py", line 19, in <module>
import airflow.jobs.backfill_job
File "/opt/airflow/airflow/jobs/backfill_job.py", line 29, in <module>
from airflow import models
File "/opt/airflow/airflow/models/__init__.py", line 30, in <module>
from airflow.models.renderedtifields import RenderedTaskInstanceFields
File "/opt/airflow/airflow/models/renderedtifields.py", line 36, in <module>
class RenderedTaskInstanceFields(Base):
File "/opt/airflow/airflow/models/renderedtifields.py", line 184, in RenderedTaskInstanceFields
@classmethod
File "/opt/airflow/airflow/utils/retries.py", line 96, in retry_db_transaction
return retry_decorator(_func)
File "/opt/airflow/airflow/utils/retries.py", line 55, in retry_decorator
func_params = signature(func).parameters
File "/usr/local/lib/python3.6/inspect.py", line 3065, in signature
return Signature.from_callable(obj, follow_wrapped=follow_wrapped)
File "/usr/local/lib/python3.6/inspect.py", line 2815, in from_callable
follow_wrapper_chains=follow_wrapped)
File "/usr/local/lib/python3.6/inspect.py", line 2193, in _signature_from_callable
raise TypeError('{!r} is not a callable object'.format(obj))
TypeError: <classmethod object at 0x7fd198edf9e8> is not a callable object
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
Hi -- thanks for resolving this issue. Will there be a patch release? |
We are going to release it in 2.2.0 Airflow release I believe. |
Thank you! We will look for that release. Much appreciated! |
Yup, it will be part of 2.2.0 to be released next week - RCs within next 2 days |
This is a follow-up on apache#18616 where we introduced retries on the occassional deadlocks when rendered task fields have been deleted by parallel threads (this is not a real deadlock, it's because MySQL locks too many things when queries are executed and will deadlock when one of those queries wait too much). Adding retry - while not perfect - should allow to handle the problem and significantly decrease the likelihood of such deadlocks. We can probably think about different approach for rendered fields, but for now retrying is - I think - acceptable short-term fix. Fixes: apache#32294 Fixes: apache#29687
This is a follow-up on #18616 where we introduced retries on the occassional deadlocks when rendered task fields have been deleted by parallel threads (this is not a real deadlock, it's because MySQL locks too many things when queries are executed and will deadlock when one of those queries wait too much). Adding retry - while not perfect - should allow to handle the problem and significantly decrease the likelihood of such deadlocks. We can probably think about different approach for rendered fields, but for now retrying is - I think - acceptable short-term fix. Fixes: #32294 Fixes: #29687
This is a follow-up on #18616 where we introduced retries on the occassional deadlocks when rendered task fields have been deleted by parallel threads (this is not a real deadlock, it's because MySQL locks too many things when queries are executed and will deadlock when one of those queries wait too much). Adding retry - while not perfect - should allow to handle the problem and significantly decrease the likelihood of such deadlocks. We can probably think about different approach for rendered fields, but for now retrying is - I think - acceptable short-term fix. Fixes: #32294 Fixes: #29687 (cherry picked from commit c8a3c11)
The query that deletes rendered old rendered task fields for MySQL
can occasionally deadlock because it is unncesssary complex with
a subquery (due to features missing in MySQL). This change
adds DB retries to get rid of the deadlock (as is the
recommended practice for MySQL).
Fixes: #18512
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.