fix(worker): probe pg_proc before calling optional schemas_with_pending_work()#1503
Merged
nicoloboschi merged 2 commits intomainfrom May 7, 2026
Merged
fix(worker): probe pg_proc before calling optional schemas_with_pending_work()#1503nicoloboschi merged 2 commits intomainfrom
nicoloboschi merged 2 commits intomainfrom
Conversation
…ng_work() (#1408) The poller called the optional PL/pgSQL routine `schemas_with_pending_work()` unconditionally on every cycle. When the routine isn't installed (the default for fresh deployments), Postgres logs a server-side `function does not exist` error every ~30s even though the Python code silently caught the exception. This adds a small `OptionalRoutines` registry/cache in `hindsight_api/engine/db/optional_routines.py` that probes `pg_proc` once on first lookup and memoises the result for the life of the process. The poller now calls the routine only when it's actually installed and falls back to the per-schema EXISTS path otherwise — without any spurious server-side errors. The registry also carries the canonical install SQL for each routine inline, so anyone touching the optimisation has a single source of truth (the previous docstring lived only on `_scan_active_schemas`). Tradeoffs: - Probe is permanently cached: installing the routine on a running cluster requires a worker restart. Acceptable because these routines are expected to be installed once at deploy time, and a probe-per-poll would defeat the optimisation. - Non-PG backends short-circuit to False without touching the DB.
…instead Hindsight never installs schemas_with_pending_work() — operators do. Keeping the SQL body in the API repo would drift from whatever is actually deployed and falsely imply ownership. Replace the install_sql field on OptionalRoutine with a contract docstring describing the expected signature, return shape, and semantic constraints, so any operator-supplied implementation is interchangeable as long as it matches. The test installs a minimal contract-satisfying stub locally rather than relying on a registry-supplied body.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
schemas_with_pending_work()unconditionally, which was logging a server-sidefunction does not existerror in Postgres every ~30s on fresh deployments where the optional PL/pgSQL routine isn't installed.hindsight_api/engine/db/optional_routines.pythat probespg_proconce per process and memoises the result. Single source of truth for the install SQL — anyone grepping the routine name lands on the canonical definition.WorkerPoller._scan_active_schemasthrough the registry: ifis_installed("schemas_with_pending_work")returns True it uses the server-side path, otherwise it runs the existing per-schema EXISTS fallback. Non-PG backends short-circuit to False without touching the DB.Notes
pg_proc). Module/abstraction uses that term so a futureCREATE PROCEDUREslots in without a rename.tenant_%schemas (broken for single-tenant). That's a property of the cloud Helm hook's SQL, not this code path — the per-schema EXISTS fallback already handles single-tenant correctly. Out of scope for this PR.Test plan
test_scan_uses_optional_routine_when_installedinstalls the routine, spies onPostgresConnection.fetch/fetchval, and asserts (a)pg_procwas probed, (b)schemas_with_pending_work()was invoked, (c) the per-schema EXISTS fallback did NOT run, (d) the cache recordsTrue. Drops the routine infinally.test_scan_finds_schemas_with_pending_work(uninstalled routine, fallback path) still passes.TestClaimBatchRotationclass (8 tests) passes locally../scripts/hooks/lint.shclean.