Skip to content

Commit 8e1d74a

Browse files
fix(dagster-postgres): use INSERT ON CONFLICT in initialize_concurrency_limit_to_default
Fixes #33552 The base-class `initialize_concurrency_limit_to_default` uses an INSERT- then-catch-IntegrityError-then-UPDATE pattern. On PostgreSQL, a failed INSERT aborts the entire transaction, so the fallback UPDATE fails with `InFailedSqlTransaction`. This crashes every run that uses `pool=` when the concurrency key already exists (race condition or retry). Override both `initialize_concurrency_limit_to_default` and `_reconcile_concurrency_limits_from_slots` in `PostgresEventLogStorage` to use `INSERT ... ON CONFLICT` — the same atomic upsert pattern already used by `store_asset_event` and `add_dynamic_partitions` in the same class. The legacy-schema path also moves `_allocate_concurrency_slots` inside the transaction context (fixing a pre-existing use-after-close). Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent e243b10 commit 8e1d74a

3 files changed

Lines changed: 140 additions & 0 deletions

File tree

python_modules/dagster/dagster_tests/storage_tests/utils/event_log_storage.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5863,6 +5863,41 @@ def test_changing_default_concurrency_key(
58635863

58645864
assert storage.get_concurrency_info("foo").slot_count == 0
58655865

5866+
def test_default_concurrency_idempotent(
5867+
self, storage: EventLogStorage, instance: DagsterInstance
5868+
):
5869+
"""Calling initialize_concurrency_limit_to_default twice with the same key must not
5870+
crash (regression test for PostgreSQL UniqueViolation / InFailedSqlTransaction)."""
5871+
assert storage
5872+
if not storage.supports_global_concurrency_limits:
5873+
pytest.skip("storage does not support global op concurrency")
5874+
5875+
if not self.can_set_concurrency_defaults():
5876+
pytest.skip("storage does not support setting global op concurrency defaults")
5877+
5878+
self.set_default_op_concurrency(instance, storage, 5)
5879+
5880+
assert storage.initialize_concurrency_limit_to_default("bar")
5881+
assert storage.get_concurrency_info("bar").slot_count == 5
5882+
5883+
# Second call with same default — must be idempotent, not crash
5884+
assert storage.initialize_concurrency_limit_to_default("bar")
5885+
assert storage.get_concurrency_info("bar").slot_count == 5
5886+
5887+
# Change the default and re-initialize — limit must propagate
5888+
self.set_default_op_concurrency(instance, storage, 3)
5889+
assert storage.initialize_concurrency_limit_to_default("bar")
5890+
assert storage.get_concurrency_info("bar").slot_count == 3
5891+
5892+
# Explicitly set via set_concurrency_slots (user-managed), then
5893+
# re-initialize from default — default must reclaim the row
5894+
storage.set_concurrency_slots("bar", 10)
5895+
assert storage.get_concurrency_info("bar").slot_count == 10
5896+
5897+
self.set_default_op_concurrency(instance, storage, 4)
5898+
assert storage.initialize_concurrency_limit_to_default("bar")
5899+
assert storage.get_concurrency_info("bar").slot_count == 4
5900+
58665901
def test_asset_checks(
58675902
self,
58685903
storage: EventLogStorage,

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/event_log.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
SqlEventLogStorageMetadata,
2020
SqlEventLogStorageTable,
2121
)
22+
from dagster._core.storage.event_log.schema import ConcurrencyLimitsTable, ConcurrencySlotsTable
2223
from dagster._core.storage.event_log.base import EventLogCursor
2324
from dagster._core.storage.event_log.migration import ASSET_KEY_INDEX_COLS
2425
from dagster._core.storage.event_log.polling_event_watcher import SqlPollingEventWatcher
@@ -314,6 +315,98 @@ def store_asset_event(self, event: EventLogEntry, event_id: int) -> None:
314315
query = query.on_conflict_do_nothing()
315316
conn.execute(query)
316317

318+
def _reconcile_concurrency_limits_from_slots(self) -> None:
319+
if not self.has_concurrency_limits_table:
320+
return
321+
322+
if not self._has_rows(ConcurrencySlotsTable) or self._has_rows(ConcurrencyLimitsTable):
323+
return
324+
325+
with self.index_transaction() as conn:
326+
rows = conn.execute(
327+
db_select(
328+
[
329+
ConcurrencySlotsTable.c.concurrency_key,
330+
db.func.count().label("count"),
331+
]
332+
)
333+
.where(
334+
ConcurrencySlotsTable.c.deleted == False, # noqa: E712
335+
)
336+
.group_by(
337+
ConcurrencySlotsTable.c.concurrency_key,
338+
)
339+
).fetchall()
340+
if rows:
341+
conn.execute(
342+
db_dialects.postgresql.insert(ConcurrencyLimitsTable)
343+
.values(
344+
[
345+
{
346+
"concurrency_key": row[0],
347+
"limit": row[1],
348+
}
349+
for row in rows
350+
]
351+
)
352+
.on_conflict_do_nothing(),
353+
)
354+
355+
def initialize_concurrency_limit_to_default(self, concurrency_key: str) -> bool:
356+
if not self.has_concurrency_limits_table:
357+
return False
358+
359+
self._reconcile_concurrency_limits_from_slots()
360+
361+
if not self.has_instance:
362+
return False
363+
364+
default_limit = self._instance.global_op_concurrency_default_limit
365+
has_default_pool_limit_col = self.has_default_pool_limit_col
366+
367+
if has_default_pool_limit_col:
368+
with self.index_transaction() as conn:
369+
if default_limit is None:
370+
conn.execute(
371+
ConcurrencyLimitsTable.delete().where(
372+
ConcurrencyLimitsTable.c.concurrency_key == concurrency_key,
373+
)
374+
)
375+
self._allocate_concurrency_slots(conn, concurrency_key, 0)
376+
else:
377+
stmt = db_dialects.postgresql.insert(ConcurrencyLimitsTable).values(
378+
concurrency_key=concurrency_key,
379+
limit=default_limit,
380+
using_default_limit=True,
381+
)
382+
stmt = stmt.on_conflict_do_update(
383+
index_elements=[ConcurrencyLimitsTable.c.concurrency_key],
384+
set_={
385+
"limit": stmt.excluded.limit,
386+
"using_default_limit": stmt.excluded.using_default_limit,
387+
},
388+
where=db.or_(
389+
ConcurrencyLimitsTable.c.limit != stmt.excluded.limit,
390+
ConcurrencyLimitsTable.c.using_default_limit
391+
!= stmt.excluded.using_default_limit,
392+
),
393+
)
394+
conn.execute(stmt)
395+
self._allocate_concurrency_slots(conn, concurrency_key, default_limit)
396+
return True
397+
398+
if default_limit is None:
399+
return False
400+
401+
with self.index_transaction() as conn:
402+
conn.execute(
403+
db_dialects.postgresql.insert(ConcurrencyLimitsTable)
404+
.values(concurrency_key=concurrency_key, limit=default_limit)
405+
.on_conflict_do_nothing(),
406+
)
407+
self._allocate_concurrency_slots(conn, concurrency_key, default_limit)
408+
return True
409+
317410
def add_dynamic_partitions(
318411
self, partitions_def_name: str, partition_keys: Sequence[str]
319412
) -> None:

python_modules/libraries/dagster-postgres/dagster_postgres_tests/test_event_log.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,18 @@ def event_log_storage(self, instance):
4545
assert isinstance(event_log_storage, PostgresEventLogStorage)
4646
yield event_log_storage
4747

48+
def can_set_concurrency_defaults(self):
49+
return True
50+
51+
def set_default_op_concurrency(self, instance, storage, limit):
52+
settings = dict(instance._settings) if instance._settings else {} # noqa: SLF001
53+
concurrency = dict(settings.get("concurrency", {}))
54+
pools = dict(concurrency.get("pools", {}))
55+
pools["default_limit"] = limit
56+
concurrency["pools"] = pools
57+
settings["concurrency"] = concurrency
58+
instance._settings = settings # noqa: SLF001
59+
4860
def can_wipe_asset_partitions(self) -> bool:
4961
return False
5062

0 commit comments

Comments
 (0)