ferruzzi commented on code in PR #66350:
URL: https://github.com/apache/airflow/pull/66350#discussion_r4051978185
##########
airflow-core/src/airflow/utils/db_cleanup.py:
##########
@@ -457,10 +465,37 @@ def _do_delete(
delete = source_table.delete().where(
and_(*[col == target_table.c[col.name] for col in
source_table.primary_key.columns])
)
+ # Re-apply skip_if_referenced on the DELETE to guard against a
race where a new
+ # referencing row is created after the archive INSERT committed
but before the DELETE
+ # runs. Without this the DELETE would violate the ON DELETE
RESTRICT FK and fail.
+ if skip_if_referenced:
+ pk_col = source_table.c[referenced_pk_column]
+ for referencing_table_name, fk_column in skip_if_referenced:
+ referencing = table(referencing_table_name,
column(fk_column))
+ delete = delete.where(
+ ~select(literal(1))
+ .select_from(referencing)
+ .where(referencing.c[fk_column] == pk_col)
+ .correlate(source_table)
+ .exists()
+ )
logger.debug("delete statement:\n%s", delete.compile())
- session.execute(delete)
+ deleted = cast("CursorResult", session.execute(delete)).rowcount
Review Comment:
@ramitkataria Good catch. I would rather reword the newsfragment than add
the IntegrityError catch, for two reasons. The smaller one is scope: this PR
has been narrowed a long way through so many revisions already and the
production change is down to the guard plus the warning. The bigger one is
that catching and continuing would not make "Fixed" true either, it would turn
the crash into a silent partial purge, which I think is exactly what you raise
in your other comment; the archive would still claim rows that are still live.
@jakubmatyszewski, could you reword `66350.bugfix.rst` along the lines of
"Reduces the window where airflow db clean fails with an IntegrityError ..."
rather than "Fixed..."?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
