Uh oh!
There was an error while loading. Please reload this page.
Add IcebergTableSnapshotTrigger for event-driven scheduling - #71387
Conversation
9567f02 to
9cb12efCompareThe Iceberg provider ships a hook and nothing else, so there is no way to schedule a DAG on 'this table has new data'. Anyone wanting it writes their own trigger. Add a trigger that polls a table's branch head through the existing IcebergHook and emits an event when it advances, so a table commit can drive an AssetWatcher. Triggers sharing a catalog connection, branch and poll interval report the same shared_stream_key, so watching many tables in one catalog costs a single poll rather than one per table. Scoped to the trigger. Registering an iceberg:// asset URI scheme is left out: that depends on how Iceberg datasets are named in OpenLineage, which is still open, and a watcher works with any Asset name. Signed-off-by: 1fanwang <1fannnw@gmail.com>
9cb12ef to
c9ce1f7Compareserialize() is captured once when the trigger row is written, so last_seen_snapshot_id mutated on self is lost when the triggerer restarts. The trigger then saw the current head as new and emitted it again, scheduling a DAG run for a commit it had already reported. Keep the cursor in the asset_state_store watermark the triggerer injects for watcher triggers, and seed from the kwarg only on the first run. Read through getattr because that attribute postdates the Airflow versions this provider supports, and skip persistence when several assets watch one trigger, since there is then no single cursor to keep. Signed-off-by: 1fanwang <1fannnw@gmail.com>
ca881b1 to
4108f03CompareUh oh!
There was an error while loading. Please reload this page.
A non-None shared_stream_key sends the triggerer to filter_shared_stream instead of run(), and the base method raises because this trigger implements neither it nor open_shared_stream. The trigger never polled; every test passed because they call run() directly. Sharing was not reachable anyway: the key omits the table, so one poll could only serve a group by listing the whole catalog. Each trigger polls its own table, which is what run() already did. Also narrow the watermark guard. A state store backend raises ValueError from the same call as the several-assets case, so catching both hid a real failure and disabled the watermark silently. Cover the triggerer's dispatch rather than calling run() directly, so a reintroduced key fails instead of passing. Signed-off-by: 1fanwang <1fannnw@gmail.com>
244516f to
129ef13CompareA watcher outlives the table it watches, so a table created after the DAG is written is normal. load_table raised there, which killed the trigger and had the triggerer restart it about once a second until the table appeared. Treat an absent table like an absent branch and keep polling. Also read shared_stream_key through getattr in the tests, since it does not exist on the older Airflow versions this provider supports. Signed-off-by: 1fanwang <1fannnw@gmail.com>
5e6aab8 to
1174d01CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: 1fanwang <1fannnw@gmail.com>
b32fd50 to
e78097dCompareSigned-off-by: 1fanwang <1fannnw@gmail.com>
e78097d to
77b3854CompareUh oh!
There was an error while loading. Please reload this page.
| store = getattr(self, "asset_state_store", None) | ||
| if store is not None: | ||
| try: | ||
| stored = await asyncio.to_thread(store.get, WATERMARK_KEY) |
There was a problem hiding this comment.
Instead of reinventing the async behaviour in every trigger, we should migrate this to aget, aset etc once #72127 lands.
There was a problem hiding this comment.
Thanks! have the fix ready that we can circle back on once ^^ 72127 lands
| if AIRFLOW_V_3_0_PLUS: | ||
| from airflow.triggers.base import BaseEventTrigger, TriggerEvent | ||
| else: | ||
| from airflow.triggers.base import ( # type: ignore[assignment] | ||
| BaseTrigger as BaseEventTrigger, | ||
| TriggerEvent, | ||
| ) |
There was a problem hiding this comment.
We should avoid if/else based on version as much as possible, gets hard to manage conditional code over time. An improvement: #72140
There was a problem hiding this comment.
agreed, added my non binding approval there
vincbeck
commented
Aug 27, 2026
@1fanwang can you work on these comments in a separate PR please? |
Rationale for this change
The Iceberg provider ships a hook and nothing else, so there is no way to schedule a DAG on "this table has new data". Anyone who wants it writes their own trigger.
IcebergTableSnapshotTriggerpolls a table's branch head through the existingIcebergHookand emits an event when it advances, so a commit can drive anAssetWatcher:The event carries
snapshot_idandprevious_snapshot_id, so a task can scan the delta instead of the whole table.Polling is what Iceberg supports: the REST catalog spec defines no subscribe, webhook or event endpoint, and none is proposed.
Two behaviors worth calling out, both found by running it on a real Airflow rather than in tests:
asset_state_storewatermark, not on the instance.serialize()is captured once when the trigger row is written, so a value mutated duringrun()is lost on restart and the trigger re-reports a commit it already emitted. Read throughgetattr, since that attribute postdates the oldest Airflow this provider supports.A trigger watched by more than one asset gets one accessor per asset, where the watermark shorthand raises. It degrades to no watermark; #71460 adds what is needed to keep a cursor there.
Are these changes tested?
Unit tests — 19 passed
Cold start emits the current head; an unchanged table emits nothing; each commit emits one event carrying the snapshot it replaced; an absent branch or table is waited on; the watermark is restored on start and written per event; a state store failure is not mistaken for the several-assets case; the triggerer's dispatch reaches the poll loop; serialization round-trips.
Removing
triggers/iceberg.pyfails collection, so the tests cannot pass vacuously.Then on a running Airflow: Postgres metadata database, api-server, dag-processor, scheduler and triggerer as separate processes, a real Iceberg catalog behind a real
iceberg_defaultconnection, and a DAG scheduled on the watched asset. Nothing stubbed.On a running Airflow
Step 4 is the real
asset_state_storerow, written by the triggerer over the execution API rather than by a test double. Step 5 is why the watermark is not just a kwarg: killing the triggerer and bringing it back produced no second run for a commit already reported.An example DAG is included, following the Redis message-queue one.