From b83740b68475ba76c157b7485e4da98c1f25054a Mon Sep 17 00:00:00 2001 From: vedarolap <304927126+vedarolap@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:56:46 +0000 Subject: [PATCH 1/3] Fix CREATE TYPE race that can crash the worker on first boot On a fresh Postgres volume, the worker and UI API both call metadata.create_all() at startup. SQLAlchemy's enum creation is check-then-create rather than atomic, so both processes can see the job_status enum as missing and both issue CREATE TYPE, crashing the loser with a UniqueViolation. Serialize schema creation behind a Postgres advisory lock so only one process creates the schema at a time. Fixes #432 --- .../src/osprey/worker/lib/storage/postgres.py | 25 +++++++- .../worker/lib/storage/tests/test_postgres.py | 57 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 osprey_worker/src/osprey/worker/lib/storage/tests/test_postgres.py diff --git a/osprey_worker/src/osprey/worker/lib/storage/postgres.py b/osprey_worker/src/osprey/worker/lib/storage/postgres.py index b0f67045..ca386f6e 100644 --- a/osprey_worker/src/osprey/worker/lib/storage/postgres.py +++ b/osprey_worker/src/osprey/worker/lib/storage/postgres.py @@ -19,6 +19,10 @@ metadata = MetaData() Model = declarative_base(name='Model', metadata=metadata) +# Arbitrary key for the Postgres advisory lock guarding schema creation (see init_from_config). +# Any process taking this lock is guaranteed to be alone while it runs metadata.create_all(). +_SCHEMA_CREATE_LOCK_KEY = 87271 + if TYPE_CHECKING: SessionMaker = sessionmaker[Session] # type: ignore[type-var] else: @@ -36,6 +40,24 @@ def _get_or_init_session(database: str) -> SessionMaker: return sessions[database] +def create_schema(engine: sqlalchemy.engine.Engine) -> None: + """Create all tables/types defined in `metadata` against `engine`. + + Multiple processes (e.g. the worker and the UI API) can call this at the same time against + a fresh database. SQLAlchemy's enum creation is check-then-create rather than atomic, so on + a fresh volume both processes can see a type as missing and both issue CREATE TYPE, and the + loser crashes with a UniqueViolation. Take a Postgres advisory lock first so only one process + creates the schema at a time; by the time any other process acquires the lock, create_all's + own existence checks make it a no-op. + """ + with engine.connect() as connection: + connection.execute(sqlalchemy.text('SELECT pg_advisory_lock(:key)'), {'key': _SCHEMA_CREATE_LOCK_KEY}) + try: + metadata.create_all(engine) + finally: + connection.execute(sqlalchemy.text('SELECT pg_advisory_unlock(:key)'), {'key': _SCHEMA_CREATE_LOCK_KEY}) + + def init_from_config(database: str) -> None: def _init(config: Config) -> None: if not config['POSTGRES_HOSTS'].get(database): @@ -59,8 +81,7 @@ def _init(config: Config) -> None: temporary_ability_token, ) - # Create all tables defined in the metadata - metadata.create_all(new_engine) + create_schema(new_engine) CONFIG.instance().register_configuration_callback(_init) diff --git a/osprey_worker/src/osprey/worker/lib/storage/tests/test_postgres.py b/osprey_worker/src/osprey/worker/lib/storage/tests/test_postgres.py new file mode 100644 index 00000000..ee0e86ec --- /dev/null +++ b/osprey_worker/src/osprey/worker/lib/storage/tests/test_postgres.py @@ -0,0 +1,57 @@ +import threading + +import sqlalchemy +from osprey.worker.lib.singletons import CONFIG +from osprey.worker.lib.storage import postgres +from psycopg2.errors import DuplicateDatabase, InvalidCatalogName +from sqlalchemy.exc import ProgrammingError +from sqlalchemy_utils import create_database, drop_database + + +def test_create_schema_survives_concurrent_callers_on_a_fresh_database(): + """Regression test for issue #432: on a fresh database, two processes (e.g. the worker and + the UI API) both calling create_schema() at startup used to be able to race on `CREATE TYPE` + for the job_status enum, since SQLAlchemy's enum creation is check-then-create rather than + atomic. Both would see the type as missing, both would issue CREATE TYPE, and the loser would + crash with a UniqueViolation on `pg_type_typname_nsp_index`. + + Simulate two racing processes with two independent engines hitting a brand new database at + the same time, and assert neither raises. + """ + base_url = CONFIG.instance()['POSTGRES_HOSTS']['osprey_db'] + fresh_url = base_url.rsplit('/', 1)[0] + '/osprey_test_concurrent_schema_create' + + try: + drop_database(fresh_url) + except ProgrammingError as e: + if not isinstance(e.orig, InvalidCatalogName): + raise + try: + create_database(fresh_url) + except ProgrammingError as e: + if not isinstance(e.orig, DuplicateDatabase): + raise + + try: + errors: list[Exception] = [] + barrier = threading.Barrier(2) + + def _create_schema() -> None: + engine = sqlalchemy.create_engine(fresh_url) + try: + barrier.wait(timeout=5) + postgres.create_schema(engine) + except Exception as e: + errors.append(e) + finally: + engine.dispose() + + threads = [threading.Thread(target=_create_schema) for _ in range(2)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=10) + + assert not errors, errors + finally: + drop_database(fresh_url) From 0947e040254ee1c4db6f1f2de4fac6ee92c809a0 Mon Sep 17 00:00:00 2001 From: vedarolap <304927126+vedarolap@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:57:15 +0000 Subject: [PATCH 2/3] Add CHANGELOG entry for #436 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c2c7075..d55b51e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ For more information about each release including git tags and artifacts, see [R ### Fixed +- Serialize Postgres schema creation behind an advisory lock so the worker and UI API no longer race on `CREATE TYPE` for the `job_status` enum on a fresh volume, which could crash the worker on first boot ([#436](https://github.com/roostorg/osprey/pull/436) by [@vedarolap](https://github.com/vedarolap), closes [#432](https://github.com/roostorg/osprey/issues/432)) - Escape literal braces when parsing f-strings in the engine ([#347](https://github.com/roostorg/osprey/pull/347) by [@haileyok](https://github.com/haileyok)) - Tolerate malformed URI escapes in `EntityWithPopover` UI component ([#377](https://github.com/roostorg/osprey/pull/377) by [@julietshen](https://github.com/julietshen)) - Add retention limits to Kafka topics to prevent unbounded disk growth ([#249](https://github.com/roostorg/osprey/pull/249) by [@VINODvoid](https://github.com/VINODvoid)) From 4df609c5a9f6b5da6a1537d30983f3fe573a2e76 Mon Sep 17 00:00:00 2001 From: Cassidy James Date: Tue, 1 Sep 2026 16:33:26 -0600 Subject: [PATCH 3/3] CHANGELOG: move entry to the right place, make concise --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57360ed8..27b6e571 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ For more information about each release including git tags and artifacts, see [R - Osprey starts without GCP credentials instead of crashing: Pub/Sub publishing is now opt-in via `OSPREY_PUBSUB_ENABLED` (default off), so the default (no-GCP) deployment publishes nothing. When it is enabled but GCP credentials can't be resolved, a `make_publisher` factory logs a warning and degrades to a noop rather than crashing. The GCP-backed telemetry itself (analytics, webhooks, rules-visualizer experiment metadata) stays inert until GCP or a replacement is configured. **Required configuration change:** deployments that rely on Pub/Sub publishing must set `OSPREY_PUBSUB_ENABLED=true`, otherwise publishing is silently disabled ([#388](https://github.com/roostorg/osprey/pull/388) by [@julietshen](https://github.com/julietshen)) +### Fixed + +- Fix postgres worker crash on first boot ([#436](https://github.com/roostorg/osprey/pull/436) by [@vedarolap](https://github.com/vedarolap), closes [#432](https://github.com/roostorg/osprey/issues/432)) + ## [1.1.0] - 2026-07-22 ### Added @@ -44,7 +48,6 @@ For more information about each release including git tags and artifacts, see [R ### Fixed -- Serialize Postgres schema creation behind an advisory lock so the worker and UI API no longer race on `CREATE TYPE` for the `job_status` enum on a fresh volume, which could crash the worker on first boot ([#436](https://github.com/roostorg/osprey/pull/436) by [@vedarolap](https://github.com/vedarolap), closes [#432](https://github.com/roostorg/osprey/issues/432)) - Escape literal braces when parsing f-strings in the engine ([#347](https://github.com/roostorg/osprey/pull/347) by [@haileyok](https://github.com/haileyok)) - Tolerate malformed URI escapes in `EntityWithPopover` UI component ([#377](https://github.com/roostorg/osprey/pull/377) by [@julietshen](https://github.com/julietshen)) - Add retention limits to Kafka topics to prevent unbounded disk growth ([#249](https://github.com/roostorg/osprey/pull/249) by [@VINODvoid](https://github.com/VINODvoid))