uranusjr commented on code in PR #62501:
URL: https://github.com/apache/airflow/pull/62501#discussion_r3502000641


##########
airflow-core/tests/unit/jobs/test_scheduler_job.py:
##########
@@ -5591,6 +5593,207 @@ def dict_from_obj(obj):
 
         assert created_run.creating_job_id == scheduler_job.id
 
+    @pytest.mark.need_serialized_dag
+    def test_create_dag_runs_asset_triggered_skips_stale_triggered_date(self, 
session, dag_maker):
+        asset = Asset(uri="test://asset-for-stale-trigger-date", 
name="asset-for-stale-trigger-date")
+        with dag_maker(dag_id="asset-consumer-stale-trigger-date", 
schedule=[asset], session=session):
+            pass
+        dag_model = dag_maker.dag_model
+        asset_id = session.scalar(select(AssetModel.id).where(AssetModel.uri 
== asset.uri))
+
+        queued_at = timezone.utcnow()
+        session.add(AssetDagRunQueue(target_dag_id=dag_model.dag_id, 
asset_id=asset_id, created_at=queued_at))
+        session.flush()
+
+        # Simulate another scheduler consuming ADRQ rows after we computed 
triggered_date_by_dag.
+        
session.execute(delete(AssetDagRunQueue).where(AssetDagRunQueue.target_dag_id 
== dag_model.dag_id))
+        session.flush()
+
+        scheduler_job = Job()
+        self.job_runner = SchedulerJobRunner(job=scheduler_job, 
executors=[self.null_exec])
+        self.job_runner._create_dag_runs_asset_triggered(
+            dag_models=[dag_model],
+            session=session,
+        )
+
+        # We do not create a new DagRun since the ADRQ has already been 
consumed
+        assert session.scalars(select(DagRun).where(DagRun.dag_id == 
dag_model.dag_id)).one_or_none() is None
+
+    @pytest.mark.need_serialized_dag
+    def test_create_dag_runs_asset_triggered_deletes_only_selected_adrq_rows(
+        self, session: Session, dag_maker
+    ):
+        asset_1 = Asset("ready-to-trigger-a-Dag-run")
+        asset_2 = Asset("should-still-exist-after-a-Dag-run-created")
+        with dag_maker(dag_id="asset-consumer-delete-selected", 
schedule=asset_1 | asset_2, session=session):
+            pass
+        dag_model = dag_maker.dag_model
+        asset_1_id = session.scalar(select(AssetModel.id).where(AssetModel.uri 
== asset_1.name))
+        asset_2_id = session.scalar(select(AssetModel.id).where(AssetModel.uri 
== asset_2.name))
+        session.add_all(
+            [
+                AssetEvent(
+                    asset_id=asset_1_id,
+                    timestamp=timezone.utcnow(),
+                ),
+                # The ADRQ that should triggers the Dag run creation
+                AssetDagRunQueue(
+                    asset_id=asset_1_id, target_dag_id=dag_model.dag_id, 
created_at=timezone.utcnow()
+                ),
+                AssetEvent(asset_id=asset_2_id, timestamp=timezone.utcnow()),
+                # The ADRQ that arrives after the Dag run creation but before 
ADRQ clean up
+                # This situation is simulated by _lock_only_selected_asset 
below
+                AssetDagRunQueue(
+                    asset_id=asset_2_id, target_dag_id=dag_model.dag_id, 
created_at=timezone.utcnow()
+                ),
+            ]
+        )
+        session.flush()
+
+        scheduler_job = Job()
+        self.job_runner = SchedulerJobRunner(job=scheduler_job, 
executors=[MockExecutor(do_update=False)])
+
+        def _lock_only_selected_asset(query, **_):
+            # Simulate SKIP LOCKED behavior where this scheduler can only 
consume one ADRQ row.
+            return query.where(AssetDagRunQueue.asset_id == asset_1_id)
+
+        with patch("airflow.jobs.scheduler_job_runner.with_row_locks", 
side_effect=_lock_only_selected_asset):
+            self.job_runner._create_dag_runs_asset_triggered(
+                dag_models=[dag_model],
+                session=session,
+            )
+
+        dr = session.scalars(select(DagRun).where(DagRun.dag_id == 
dag_model.dag_id)).one_or_none()
+        assert dr is not None
+
+        adrq_1 = session.scalars(
+            select(AssetDagRunQueue).where(
+                AssetDagRunQueue.target_dag_id == dag_model.dag_id,
+                AssetDagRunQueue.asset_id == asset_1_id,
+            )
+        ).one_or_none()
+        assert adrq_1 is None
+        adrq_2 = session.scalars(
+            select(AssetDagRunQueue).where(
+                AssetDagRunQueue.target_dag_id == dag_model.dag_id,
+                AssetDagRunQueue.asset_id == asset_2_id,
+            )
+        ).one_or_none()
+        assert adrq_2 is not None
+
+    @pytest.mark.need_serialized_dag
+    @pytest.mark.backend("postgres", "mysql")
+    def test_create_dag_runs_when_concurrent_asset_events_created(self, 
session: Session, dag_maker, caplog):
+        from concurrent.futures import ThreadPoolExecutor, as_completed
+
+        ASSET_EVENT_COUNT = 30
+        asset = Asset(name="test_asset")
+        with dag_maker(dag_id="consumer", schedule=asset, session=session):
+            pass
+        dag_model = dag_maker.dag_model
+        # Capture the dag_id as a plain string in the main thread. The worker 
threads must not
+        # touch this ORM object: it is bound to the main thread's session, 
which the loop below
+        # commits (and thus expires) concurrently, so any attribute access 
from a worker would
+        # load against another thread's session.
+        consumer_dag_id = dag_model.dag_id
+        with dag_maker(dag_id="asset-producer", start_date=timezone.utcnow(), 
session=session):
+            BashOperator(task_id="simulate-asset-outlet", bash_command="echo 
1")
+        dag_maker.create_dagrun(run_id="asset-producer-run")
+        asset_id = session.scalar(select(AssetModel.id).where(AssetModel.uri 
== asset.uri))
+        futures = []
+        consumed_asset_events = []
+        asset_event_metadata: list[tuple[int, datetime.datetime]] = []
+
+        def create_asset_events(sleep):
+            import time
+
+            from sqlalchemy import inspect
+
+            with create_session() as session:
+                # Re-fetch the DagModel in this thread's own session so all 
ORM access stays
+                # thread-local.
+                dag = session.get(DagModel, consumer_dag_id)
+                now = timezone.utcnow()
+                asset_manager = AssetManager()
+                asset_event = _create_asset_event(session=session, 
asset_id=asset_id, timestamp=now)
+                time.sleep(sleep)  # widen the race window between event 
creation and queueing
+                dialect_name = inspect(session.get_bind()).dialect.name
+                if dialect_name in ("postgresql", "sqlite"):
+                    
asset_manager._queue_dagruns_nonpartitioned_conflict_update(
+                        asset_id=asset_id,
+                        dags_to_queue=[dag],
+                        event=asset_event,
+                        session=session,
+                        dialect_name=dialect_name,
+                    )
+                elif dialect_name == "mysql":
+                    asset_manager._queue_dagruns_nonpartitioned_mysql(
+                        asset_id=asset_id, dags_to_queue=[dag], 
event=asset_event, session=session
+                    )
+
+            return asset_event.id, now.isoformat()
+
+        with (
+            ThreadPoolExecutor() as executor,
+            caplog.at_level(
+                "WARNING",
+                logger="airflow.jobs.scheduler_job_runner",
+            ),
+        ):
+            for i in range(ASSET_EVENT_COUNT):
+                # Deterministically alternate between fast (0s) and slow (1s) 
workers so the
+                # test reliably exercises both code paths without relying on 
RNG.
+                future = executor.submit(create_asset_events, i % 3)
+                futures.append(future)
+            scheduler_job = Job()
+            self.job_runner = SchedulerJobRunner(job=scheduler_job, 
executors=[MockExecutor(do_update=False)])
+            seen_dr_ids: set[int] = set()
+            for future in as_completed(futures, timeout=120):
+                asset_event_metadata.append(future.result())
+                self.job_runner._create_dag_runs_asset_triggered(
+                    dag_models=[dag_model],
+                    session=session,
+                )
+                session.commit()
+                all_drs = session.scalars(select(DagRun).where(DagRun.dag_id 
== dag_model.dag_id)).all()
+                for dr in all_drs:
+                    if dr.id not in seen_dr_ids:
+                        seen_dr_ids.add(dr.id)
+                        consumed_asset_events += dr.consumed_asset_events
+        total_consumed_asset_events = len(consumed_asset_events)
+        total_consume_asset_event_metadata = []
+        sorted_asset_event_metadata = sorted(asset_event_metadata, key=lambda 
e: e[1])
+        if total_consumed_asset_events != ASSET_EVENT_COUNT:
+            # found missing events, print them for debugging
+            consumed_asset_event_metadata = {
+                (event.id, event.timestamp.isoformat()) for event in 
consumed_asset_events
+            }
+            missing_asset_event_metadata = set(sorted_asset_event_metadata) - 
consumed_asset_event_metadata
+            print(f"Missing AssetEvent Metadata: 
{missing_asset_event_metadata}")

Review Comment:
   This and other print statements should be removed



-- 
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]

Reply via email to