ferruzzi commented on code in PR #70923:
URL: https://github.com/apache/airflow/pull/70923#discussion_r3973492699
##########
airflow-core/src/airflow/utils/db_cleanup.py:
##########
@@ -205,7 +219,22 @@ def readable_config(self):
_TableConfig(table_name="task_reschedule",
recency_column_name="start_date", dag_id_column_name="dag_id"),
_TableConfig(table_name="xcom", recency_column_name="timestamp",
dag_id_column_name="dag_id"),
_TableConfig(table_name="_xcom_archive", recency_column_name="timestamp",
dag_id_column_name="dag_id"),
- _TableConfig(table_name="callback_request",
recency_column_name="created_at"),
+ _TableConfig(
+ table_name="callback",
+ recency_column_name="created_at",
+ extra_columns=["state"],
Review Comment:
I'm surprised this doesn't need `id` in the `extra_columns`, it should be a
required column.
##########
airflow-core/src/airflow/utils/db_cleanup.py:
##########
@@ -205,7 +219,22 @@ def readable_config(self):
_TableConfig(table_name="task_reschedule",
recency_column_name="start_date", dag_id_column_name="dag_id"),
_TableConfig(table_name="xcom", recency_column_name="timestamp",
dag_id_column_name="dag_id"),
_TableConfig(table_name="_xcom_archive", recency_column_name="timestamp",
dag_id_column_name="dag_id"),
- _TableConfig(table_name="callback_request",
recency_column_name="created_at"),
+ _TableConfig(
+ table_name="callback",
+ recency_column_name="created_at",
+ extra_columns=["state"],
+ # A callback that can still run owns its deadline row through an ON
DELETE CASCADE
+ # foreign key, so purging one would silently drop a deadline that has
not fired yet.
+ # Dag-processor callbacks carry no state and are deleted as they are
dispatched; any
+ # that outlive the retention window were orphaned and no deadline
references them.
+ extra_filters=[
+ or_(
+ column("state").in_(sorted(TERMINAL_STATES)),
Review Comment:
This may be backwards, isn't it? Don't we want to check if
`column("state").not_in(sorted(ACTIVE_STATES)) OR column("state").is_(None),`?
If a deadline is in SCHEDULED and its run is deleted, then it's orphaned as
well and needs to be pruned, no? It would also mean renaming or adding new
states later become pruned by default which would have prevented this issue.
--
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]
