jason810496 commented on code in PR #51134:
URL: https://github.com/apache/airflow/pull/51134#discussion_r2113768069
##########
airflow-core/src/airflow/models/serialized_dag.py:
##########
@@ -401,14 +402,22 @@ def write_dag(
# If Yes, does nothing
# If No or the DAG does not exists, updates / writes Serialized DAG to
DB
if min_update_interval is not None:
- if session.scalar(
- select(literal(True))
- .where(
- cls.dag_id == dag.dag_id,
- (timezone.utcnow() -
timedelta(seconds=min_update_interval)) < cls.last_updated,
+ try:
+ do_nothing = session.scalar(
+ select(literal(True))
+ .where(
+ cls.dag_id == dag.dag_id,
+ (timezone.utcnow() -
timedelta(seconds=min_update_interval)) < cls.created_at,
+ )
+ .select_from(cls)
+ .with_for_update(nowait=True)
+ )
+ if do_nothing:
+ return False
+ except OperationalError:
+ log.warning(
+ "Another Scheduler is already reparsing dag %s. Skipping
reparse request.", dag.dag_id
)
Review Comment:
nit:
```suggestion
except OperationalError as e:
log.warning(
"Another Scheduler is already reparsing dag %s. Skipping
reparse request. Details: %s", dag.dag_id, str(e)
)
```
##########
airflow-core/src/airflow/models/serialized_dag.py:
##########
@@ -401,14 +402,22 @@ def write_dag(
# If Yes, does nothing
# If No or the DAG does not exists, updates / writes Serialized DAG to
DB
if min_update_interval is not None:
- if session.scalar(
- select(literal(True))
- .where(
- cls.dag_id == dag.dag_id,
- (timezone.utcnow() -
timedelta(seconds=min_update_interval)) < cls.last_updated,
+ try:
+ do_nothing = session.scalar(
+ select(literal(True))
+ .where(
+ cls.dag_id == dag.dag_id,
+ (timezone.utcnow() -
timedelta(seconds=min_update_interval)) < cls.created_at,
Review Comment:
Why the `created_at` is used instead of `last_updated` ?
##########
airflow-core/src/airflow/models/serialized_dag.py:
##########
@@ -401,14 +402,22 @@ def write_dag(
# If Yes, does nothing
# If No or the DAG does not exists, updates / writes Serialized DAG to
DB
if min_update_interval is not None:
- if session.scalar(
- select(literal(True))
- .where(
- cls.dag_id == dag.dag_id,
- (timezone.utcnow() -
timedelta(seconds=min_update_interval)) < cls.last_updated,
+ try:
+ do_nothing = session.scalar(
+ select(literal(True))
+ .where(
+ cls.dag_id == dag.dag_id,
+ (timezone.utcnow() -
timedelta(seconds=min_update_interval)) < cls.created_at,
+ )
+ .select_from(cls)
+ .with_for_update(nowait=True)
Review Comment:
How about having comment for new row-level lock.
Also, I think we might need dialect-specific for the `with_for_update`
statement.
--
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]
