fix(llm): skip the database-wide iterative_scan ALTER once it is in place
Some checks failed
CI / lint (push) Successful in 3m0s
CI / notebooks-smoke (push) Successful in 2m27s
Deploy / changes (push) Successful in 29s
CI / test (push) Failing after 7m13s
Infra CI / zotero (push) Successful in 32s
Infra CI / notebooks (push) Successful in 1m57s
Infra CI / docs (push) Successful in 53s
Infra CI / api (push) Successful in 1m43s
Infra CI / bcda (push) Successful in 2m19s
Infra CI / llm (push) Successful in 1m22s
Deploy / notebooks (push) Has been skipped
Deploy / zotero (push) Has been skipped
Deploy / docs (push) Has been skipped
Infra CI / mc (push) Successful in 29s
Deploy / api (push) Successful in 1m34s
Deploy / bcda (push) Successful in 2m16s
Deploy / mc (push) Has been skipped
Deploy / llm (push) Successful in 1m27s
Deploy / report (push) Successful in 16s
Comment Pull / comment-pull (push) Failing after 59s
Some checks failed
CI / lint (push) Successful in 3m0s
CI / notebooks-smoke (push) Successful in 2m27s
Deploy / changes (push) Successful in 29s
CI / test (push) Failing after 7m13s
Infra CI / zotero (push) Successful in 32s
Infra CI / notebooks (push) Successful in 1m57s
Infra CI / docs (push) Successful in 53s
Infra CI / api (push) Successful in 1m43s
Infra CI / bcda (push) Successful in 2m19s
Infra CI / llm (push) Successful in 1m22s
Deploy / notebooks (push) Has been skipped
Deploy / zotero (push) Has been skipped
Deploy / docs (push) Has been skipped
Infra CI / mc (push) Successful in 29s
Deploy / api (push) Successful in 1m34s
Deploy / bcda (push) Successful in 2m16s
Deploy / mc (push) Has been skipped
Deploy / llm (push) Successful in 1m27s
Deploy / report (push) Successful in 16s
Comment Pull / comment-pull (push) Failing after 59s
hnsw.iterative_scan = relaxed_order is already set on the llm database (pg_db_role_setting), but migrate re-issued ALTER DATABASE on every index run as the llm role, which may not, and postgres logged two permission ERRORs each time. migrate now checks the setting first. Verified live: the check sees it, a migrate run logs nothing.
This commit is contained in:
@@ -89,6 +89,12 @@ _METADATA_INDEX_DDL = [
|
||||
# — the API's pooled connections included — inherits it without having
|
||||
# to load the extension's GUCs first.
|
||||
ITERATIVE_SCAN_DDL = "ALTER DATABASE {db} SET hnsw.iterative_scan = 'relaxed_order'"
|
||||
_ITERATIVE_SCAN_SET_SQL = (
|
||||
"SELECT EXISTS (SELECT 1 FROM pg_db_role_setting s "
|
||||
"JOIN pg_database d ON d.oid = s.setdatabase "
|
||||
"WHERE d.datname = current_database() AND s.setrole = 0 "
|
||||
"AND 'hnsw.iterative_scan=relaxed_order' = ANY (s.setconfig))"
|
||||
)
|
||||
|
||||
|
||||
def migrate(engine: Engine) -> None:
|
||||
@@ -109,6 +115,10 @@ def ensure_iterative_scan(engine: Engine) -> None:
|
||||
try:
|
||||
with engine.begin() as conn:
|
||||
db = conn.execute(text("SELECT current_database()")).scalar()
|
||||
# Already in place (set once by a role that may): re-issuing it
|
||||
# as the llm role only logs a permission ERROR in postgres.
|
||||
if conn.execute(text(_ITERATIVE_SCAN_SET_SQL)).scalar():
|
||||
return
|
||||
conn.execute(text(ITERATIVE_SCAN_DDL.format(db=db)))
|
||||
except Exception as exc: # noqa: BLE001 — the llm role usually may not ALTER DATABASE
|
||||
# every connection also SETs it itself (llm.index.set_iterative_scan),
|
||||
|
||||
@@ -88,7 +88,15 @@ class TestIterativeScan:
|
||||
|
||||
engine = MagicMock()
|
||||
conn = engine.begin.return_value.__enter__.return_value
|
||||
conn.execute.return_value.scalar.return_value = "llm"
|
||||
|
||||
def answer(stmt, *a, **k):
|
||||
r = MagicMock()
|
||||
r.scalar.return_value = (
|
||||
False if "pg_db_role_setting" in str(stmt) else "llm"
|
||||
)
|
||||
return r
|
||||
|
||||
conn.execute.side_effect = answer
|
||||
migrate(engine)
|
||||
sql = [str(c.args[0]) for c in conn.execute.call_args_list]
|
||||
assert any(
|
||||
@@ -96,6 +104,26 @@ class TestIterativeScan:
|
||||
for q in sql
|
||||
)
|
||||
|
||||
def test_already_set_database_wide_issues_no_alter(self):
|
||||
# The llm role may not ALTER DATABASE; re-issuing it on every index
|
||||
# run only logged two permission ERRORs in postgres per run.
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from llm.migrate import ensure_iterative_scan
|
||||
|
||||
engine = MagicMock()
|
||||
conn = engine.begin.return_value.__enter__.return_value
|
||||
|
||||
def answer(stmt, *a, **k):
|
||||
r = MagicMock()
|
||||
r.scalar.return_value = True if "pg_db_role_setting" in str(stmt) else "llm"
|
||||
return r
|
||||
|
||||
conn.execute.side_effect = answer
|
||||
ensure_iterative_scan(engine)
|
||||
sql = [str(c.args[0]) for c in conn.execute.call_args_list]
|
||||
assert not any("ALTER DATABASE" in q for q in sql)
|
||||
|
||||
def test_failure_is_a_warning_not_an_error(self, caplog):
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
|
||||
Reference in New Issue
Block a user