PR #62501 made AssetManager._create_asset_event commit each AssetEvent in a short-lived independent session, so events no longer commit atomically. A failure/crash after the event commits, but before the caller's transaction commits, now leaves an orphaned event (no ADRQ to consume it) or a duplicate event (on task retry). #62501's own description acknowledges this tradeoff.
We should drop the side session (single atomic commit for event + ADRQ + TI state), and make _create_dag_runs_asset_triggered tolerant of commit ordering.
The main idea is to make ADQR reference the asset event that generated it directly with a fk, instead of implicitly by asset_id + timestamp as introduced in #62501. This would remove the need for short-lived sessions. However, not using a timestamp range has some blast radius we’ll need to address, similar to when we changed the DagRun references from using execution_date to a concrete fk. It should be doable though.
References
PR #62501 made
AssetManager._create_asset_eventcommit eachAssetEventin a short-lived independent session, so events no longer commit atomically. A failure/crash after the event commits, but before the caller's transaction commits, now leaves an orphaned event (no ADRQ to consume it) or a duplicate event (on task retry). #62501's own description acknowledges this tradeoff.We should drop the side session (single atomic commit for event + ADRQ + TI state), and make
_create_dag_runs_asset_triggeredtolerant of commit ordering.The main idea is to make ADQR reference the asset event that generated it directly with a fk, instead of implicitly by asset_id + timestamp as introduced in #62501. This would remove the need for short-lived sessions. However, not using a timestamp range has some blast radius we’ll need to address, similar to when we changed the DagRun references from using execution_date to a concrete fk. It should be doable though.
References