Skip to content

Commit 5eebb53

Browse files
bokelleyclaude
andauthored
fix(reporting-storage): refuse a ledger upgraded to storage contract 2 with the real remedy (#1857)
* fix(reporting-storage): refuse a ledger upgraded to storage contract 2 with the real remedy adcp-reporting-storage 0.2.0b1+ upgrades a 0.1.x ledger in place, widening the configuration, obligation-period and status-checkpoint keys with offering_id. A 0.1.x process that keeps running through a rolling deploy, or restarts after it, used to fail in ways that hid the cause: - create_schema() raised "Reporting caller ownership requires a stopped-worker maintenance migration", with a hint to run migrate_legacy_reporting. That migration doesn't apply here and would move reporting state into quarantine. - commit_obligation raised LedgerConflictError LEDGER_WRITE_FAILED ("integrity rule"), and put_configuration and the status checkpoint write raised a raw InvalidColumnReference (42P10): their ON CONFLICT targets no longer match a unique index. Refuse an upgraded ledger (reporting_configurations has offering_id) before any DDL with ReportingSchemaAheadError, an operator-action error that startup re-raises unwrapped. Map 42P10 on those writes to the same remedy: obligation writes raise LedgerConflictError code REPORTING_SCHEMA_AHEAD, and configuration and checkpoint writes raise ReportingSchemaAheadError. The remedy is to run adcp>=10.0.0b7 with adcp-reporting-storage>=0.2.0b1 everywhere; downgrading means restoring the pre-upgrade backup; do not run migrate_legacy_reporting. The packaged SQL's own check now raises the same message, for operators who apply the DDL themselves. Refs #1831 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(public-api): record ReportingSchemaAheadError in the public API snapshot Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(reporting-storage): skip the contract-2 DDL test where psycopg is absent Import psycopg only after isolated_reporting_pool has checked it is installed, as the native suite runs without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5beaaed commit 5eebb53

7 files changed

Lines changed: 246 additions & 42 deletions

File tree

‎packages/adcp-reporting-storage/src/adcp_reporting_storage/__init__.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@
168168
from adcp_reporting_storage.migration import (
169169
ReportingOwnershipMigrationError as ReportingOwnershipMigrationError,
170170
)
171+
from adcp_reporting_storage.migration import (
172+
ReportingSchemaAheadError as ReportingSchemaAheadError,
173+
)
171174
from adcp_reporting_storage.migration import (
172175
backfill_legacy_reporting as backfill_legacy_reporting,
173176
)
@@ -252,6 +255,7 @@
252255
# Caller-ownership migration
253256
"ReportingOwnershipBackfill": ("migration", "ReportingOwnershipBackfill"),
254257
"ReportingOwnershipMigrationError": ("migration", "ReportingOwnershipMigrationError"),
258+
"ReportingSchemaAheadError": ("migration", "ReportingSchemaAheadError"),
255259
"backfill_legacy_reporting": ("migration", "backfill_legacy_reporting"),
256260
"legacy_generation_digest": ("migration", "legacy_generation_digest"),
257261
"migrate_legacy_reporting": ("migration", "migrate_legacy_reporting"),

‎packages/adcp-reporting-storage/src/adcp_reporting_storage/ledger/pg.py‎

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@
159159
ReportingRowStorageV1,
160160
ReportingRowStoreError,
161161
)
162+
from adcp_reporting_storage.migration import schema_ahead_error
162163

163164
if TYPE_CHECKING:
164165
from psycopg_pool import AsyncConnectionPool
@@ -1028,40 +1029,46 @@ async def put_configuration(self, configuration: ReportingConfiguration) -> None
10281029
digest = _fingerprint(payload)
10291030
async with self._connection() as connection, connection.transaction():
10301031
await self._lock_account(connection, key.account_id)
1031-
inserted_cursor = await connection.execute(
1032-
"INSERT INTO reporting_configurations"
1033-
" (delivery_config_id, delivery_config_version, account_id,"
1034-
" report_definition_id, reporting_profile, feed_purpose, required_finality,"
1035-
" account_timezone, schedule, media_buy_ids, activated_at, deactivated_at,"
1036-
" automated_recovery_seconds, status_retention_days, definition,"
1037-
" authoritative_party, content_sha256, consumer_id, quarantined)"
1038-
" VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s::jsonb, %s::jsonb, %s, %s, %s, %s,"
1039-
" %s::jsonb, %s, %s, %s, %s)"
1040-
" ON CONFLICT (account_id, consumer_id, delivery_config_id, delive"
1041-
"ry_config_version) DO NOTHING"
1042-
" RETURNING delivery_config_id",
1043-
(
1044-
key.delivery_config_id,
1045-
key.delivery_config_version,
1046-
key.account_id,
1047-
configuration.report_definition_id,
1048-
configuration.reporting_profile,
1049-
configuration.feed_purpose,
1050-
configuration.required_finality,
1051-
configuration.account_timezone,
1052-
_json(payload["schedule"]),
1053-
_json(sorted(configuration.media_buy_ids)),
1054-
configuration.activated_at,
1055-
configuration.deactivated_at,
1056-
configuration.automated_recovery_window.total_seconds(),
1057-
configuration.status_retention_days,
1058-
_json(payload["definition"]) if payload["definition"] else None,
1059-
configuration.authoritative_party,
1060-
digest,
1061-
configuration.consumer_id,
1062-
configuration.quarantined,
1063-
),
1064-
)
1032+
try:
1033+
inserted_cursor = await connection.execute(
1034+
"INSERT INTO reporting_configurations"
1035+
" (delivery_config_id, delivery_config_version, account_id,"
1036+
" report_definition_id, reporting_profile, feed_purpose, required_finality,"
1037+
" account_timezone, schedule, media_buy_ids, activated_at, deactivated_at,"
1038+
" automated_recovery_seconds, status_retention_days, definition,"
1039+
" authoritative_party, content_sha256, consumer_id, quarantined)"
1040+
" VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s::jsonb, %s::jsonb, %s, %s, %s, %s,"
1041+
" %s::jsonb, %s, %s, %s, %s)"
1042+
" ON CONFLICT (account_id, consumer_id, delivery_config_id, delive"
1043+
"ry_config_version) DO NOTHING"
1044+
" RETURNING delivery_config_id",
1045+
(
1046+
key.delivery_config_id,
1047+
key.delivery_config_version,
1048+
key.account_id,
1049+
configuration.report_definition_id,
1050+
configuration.reporting_profile,
1051+
configuration.feed_purpose,
1052+
configuration.required_finality,
1053+
configuration.account_timezone,
1054+
_json(payload["schedule"]),
1055+
_json(sorted(configuration.media_buy_ids)),
1056+
configuration.activated_at,
1057+
configuration.deactivated_at,
1058+
configuration.automated_recovery_window.total_seconds(),
1059+
configuration.status_retention_days,
1060+
_json(payload["definition"]) if payload["definition"] else None,
1061+
configuration.authoritative_party,
1062+
digest,
1063+
configuration.consumer_id,
1064+
configuration.quarantined,
1065+
),
1066+
)
1067+
except Exception as error:
1068+
ahead = schema_ahead_error(error)
1069+
if ahead is not None:
1070+
raise ahead from error
1071+
raise
10651072
inserted = await inserted_cursor.fetchone()
10661073
# Check *after* the insert. ON CONFLICT waits for a concurrent
10671074
# winner; a fresh READ COMMITTED statement sees its retained
@@ -3493,6 +3500,9 @@ def _translate_integrity_error(error: Exception) -> LedgerConflictError:
34933500
Matching the index name rather than the driver's prose: the message text
34943501
varies by Postgres version and locale, the index name does not.
34953502
"""
3503+
ahead = schema_ahead_error(error)
3504+
if ahead is not None:
3505+
return LedgerConflictError("REPORTING_SCHEMA_AHEAD", str(ahead))
34963506
text = str(error)
34973507
if "reporting_revisions_one_official" in text:
34983508
return LedgerConflictError(

‎packages/adcp-reporting-storage/src/adcp_reporting_storage/ledger/reporting_ledger_account_generations.sql‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ BEGIN
2929
] THEN
3030
RETURN;
3131
END IF;
32+
-- adcp-reporting-storage 0.2+ widens the key with offering_id (storage
33+
-- contract 2). This release cannot run on it, and the caller-ownership
34+
-- remedy below would be wrong advice.
35+
IF 'offering_id' = ANY(primary_key_columns) THEN
36+
RAISE EXCEPTION 'adcp-reporting-storage 0.2 or later has upgraded this reporting ledger to storage contract 2; this release implements storage contract 1'
37+
USING HINT = 'Run adcp>=10.0.0b7 with adcp-reporting-storage>=0.2.0b1 on every reporting process. Downgrading is unsupported: restore the backup taken before the upgrade. Do not run migrate_legacy_reporting.';
38+
END IF;
3239
RAISE EXCEPTION 'Reporting caller ownership requires a stopped-worker maintenance migration'
3340
USING HINT = 'Use migrate_legacy_reporting; retain legacy state in quarantine and backfill authoritative ownership.';
3441

‎packages/adcp-reporting-storage/src/adcp_reporting_storage/migration.py‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
__all__ = [
2727
"ReportingOwnershipBackfill",
2828
"ReportingOwnershipMigrationError",
29+
"ReportingSchemaAheadError",
2930
"backfill_legacy_reporting",
3031
"legacy_generation_digest",
3132
"migrate_legacy_reporting",
@@ -43,6 +44,41 @@ class ReportingOwnershipMigrationError(ReportingOperatorActionError):
4344
"""
4445

4546

47+
_CONTRACT_2_REMEDY = (
48+
"Run adcp>=10.0.0b7 with adcp-reporting-storage>=0.2.0b1 on every reporting process."
49+
" Downgrading is unsupported: to go back, restore the backup taken before the upgrade."
50+
" This is not a caller-ownership migration; do not run migrate_legacy_reporting."
51+
)
52+
# PostgreSQL: no unique index or constraint matches an ON CONFLICT target.
53+
_NO_MATCHING_CONFLICT_KEY = "42P10"
54+
55+
56+
class ReportingSchemaAheadError(ReportingOperatorActionError):
57+
"""adcp-reporting-storage 0.2 or later upgraded this ledger (storage contract 2).
58+
59+
This release implements storage contract 1: its generation, obligation
60+
and status keys name the columns before ``offering_id`` was added, so it
61+
can neither start on the upgraded ledger nor write to it. Service startup
62+
re-raises it unwrapped, so the operator sees the remedy.
63+
"""
64+
65+
66+
def schema_ahead_error(error: BaseException) -> ReportingSchemaAheadError | None:
67+
"""The contract-2 refusal for a write whose conflict key the upgrade widened.
68+
69+
A running process of this release meets an upgraded ledger as PostgreSQL
70+
42P10 on its ``ON CONFLICT`` writes, since the unique keys they name now
71+
include ``offering_id``. ``None`` for any other error.
72+
"""
73+
if getattr(error, "sqlstate", None) != _NO_MATCHING_CONFLICT_KEY:
74+
return None
75+
return ReportingSchemaAheadError(
76+
"this reporting write names a key that a newer adcp-reporting-storage (storage contract"
77+
" 2, with offering_id) has widened, so the ledger was upgraded while this process ran. "
78+
+ _CONTRACT_2_REMEDY
79+
)
80+
81+
4682
@dataclass(frozen=True)
4783
class ReportingOwnershipBackfill:
4884
"""Authoritative mapping with an explicit retained-only recovery decision.
@@ -83,9 +119,17 @@ async def require_owned_schema_or_empty(connection: Any) -> None:
83119
"SELECT EXISTS (SELECT 1 FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace"
84120
" WHERE n.nspname=current_schema() AND c.relname='reporting_configurations'),"
85121
" EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema=current_schema()"
86-
" AND table_name='reporting_configurations' AND column_name='consumer_id')"
122+
" AND table_name='reporting_configurations' AND column_name='consumer_id'),"
123+
" EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema=current_schema()"
124+
" AND table_name='reporting_configurations' AND column_name='offering_id')"
87125
)
88126
).fetchone()
127+
if row[2]:
128+
raise ReportingSchemaAheadError(
129+
"adcp-reporting-storage 0.2 or later has upgraded this reporting ledger to storage"
130+
" contract 2 (reporting_configurations has offering_id). This release implements"
131+
" storage contract 1 and cannot run against it. " + _CONTRACT_2_REMEDY
132+
)
89133
if row[0] and not row[1]:
90134
raise ReportingOwnershipMigrationError(
91135
"If all legacy tables are empty, call replace_empty_legacy_reporting. "

‎packages/adcp-reporting-storage/src/adcp_reporting_storage/outbox/status_pg.py‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
persist_replay_lifecycles_on,
5050
settle_snapshot_on,
5151
)
52+
from adcp_reporting_storage.migration import schema_ahead_error
5253
from adcp_reporting_storage.outbox.pg import PgReportingOutbox, database_now
5354

5455
_KEY = (
@@ -288,13 +289,20 @@ async def _lock_scopes_on(self, connection: Any, snapshot: ReportingStatusSnapsh
288289
# Insert placeholders before allocating random IDs. New and existing
289290
# typed scopes have exactly the same row-lock/notification boundary.
290291
for scope in projection_scopes(snapshot):
291-
await connection.execute(
292-
f"INSERT INTO reporting_status_scope_checkpoints ({_KEY}, scope, fingerprint," # nosec B608
293-
" snapshot, source_sequence, baseline, publishable, selector_writer_floor)"
294-
' VALUES (%s,%s,%s,%s,%s,%s,%s::jsonb,%s,\'{"health":"waiting"}\',0,FALSE,FALSE,2)'
295-
f" ON CONFLICT ({_KEY}) DO NOTHING", # nosec B608
296-
(*scope.checkpoint_key, json.dumps(asdict(scope)), "0" * 64),
297-
)
292+
try:
293+
await connection.execute(
294+
f"INSERT INTO reporting_status_scope_checkpoints ({_KEY}, scope, fingerprint," # nosec B608
295+
" snapshot, source_sequence, baseline, publishable, selector_writer_floor)"
296+
" VALUES (%s,%s,%s,%s,%s,%s,%s::jsonb,%s,"
297+
' \'{"health":"waiting"}\',0,FALSE,FALSE,2)'
298+
f" ON CONFLICT ({_KEY}) DO NOTHING", # nosec B608
299+
(*scope.checkpoint_key, json.dumps(asdict(scope)), "0" * 64),
300+
)
301+
except Exception as error:
302+
ahead = schema_ahead_error(error)
303+
if ahead is not None:
304+
raise ahead from error
305+
raise
298306
await (
299307
await connection.execute(
300308
"SELECT 1 FROM reporting_status_scope_checkpoints WHERE account_id=%s"
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
"""A ledger upgraded to storage contract 2 is refused clearly by this release (#1831).
2+
3+
adcp-reporting-storage 0.2.0b1+ upgrades a 0.1.x ledger in place: migration
4+
0002 adds ``offering_id`` and widens the configuration key and the obligation
5+
period key with it. If a process of this release keeps running during a
6+
rolling deploy, or restarts afterwards, it must name the actual problem and
7+
remedy rather than fail with a raw driver error or point at the unrelated
8+
caller-ownership migration.
9+
10+
``_widen`` applies the same key changes as 0002 on ``next``
11+
(``reporting_ledger_account_generations.sql``).
12+
"""
13+
14+
from __future__ import annotations
15+
16+
import pytest
17+
18+
from adcp.reporting.ledger import LedgerConflictError
19+
20+
from ._generation_support import configuration, isolated_reporting_pool, obligation_for
21+
22+
_WIDEN = """
23+
ALTER TABLE reporting_configurations ADD COLUMN offering_id TEXT COLLATE "C" NOT NULL DEFAULT '';
24+
ALTER TABLE reporting_configurations DROP CONSTRAINT reporting_configurations_pkey CASCADE;
25+
ALTER TABLE reporting_configurations ADD CONSTRAINT reporting_configurations_pkey
26+
PRIMARY KEY (account_id, consumer_id, delivery_config_id, delivery_config_version, offering_id);
27+
ALTER TABLE reporting_obligations ADD COLUMN offering_id TEXT COLLATE "C" NOT NULL DEFAULT '';
28+
DROP INDEX reporting_obligations_period_key;
29+
CREATE UNIQUE INDEX reporting_obligations_period_key
30+
ON reporting_obligations
31+
(account_id, consumer_id, delivery_config_id, delivery_config_version, offering_id,
32+
period_start, period_end);
33+
ALTER TABLE reporting_consumer_statuses ADD COLUMN offering_id TEXT COLLATE "C" NOT NULL DEFAULT '';
34+
"""
35+
36+
37+
async def _upgraded_store(pool):
38+
from adcp_reporting_storage.ledger.pg import PgReportingLedgerStore
39+
40+
store = PgReportingLedgerStore(pool=pool)
41+
await store.create_schema()
42+
await store.put_configuration(configuration("acct_a"))
43+
async with pool.connection() as connection:
44+
await connection.execute(_WIDEN)
45+
return store
46+
47+
48+
def _assert_remedy(message: str) -> None:
49+
assert "storage contract 2" in message
50+
assert "adcp>=10.0.0b7 with adcp-reporting-storage>=0.2.0b1" in message
51+
assert "restore the backup" in message
52+
assert "do not run migrate_legacy_reporting" in message
53+
54+
55+
def test_only_a_missing_conflict_key_is_a_contract_2_refusal() -> None:
56+
from adcp_reporting_storage.migration import schema_ahead_error
57+
58+
class _Error(Exception):
59+
def __init__(self, sqlstate: str) -> None:
60+
super().__init__(sqlstate)
61+
self.sqlstate = sqlstate
62+
63+
assert schema_ahead_error(_Error("42P10")) is not None
64+
assert schema_ahead_error(_Error("23505")) is None # unique violation
65+
assert schema_ahead_error(ValueError("no sqlstate")) is None
66+
67+
68+
@pytest.mark.asyncio
69+
async def test_create_schema_refuses_an_upgraded_ledger_and_changes_nothing() -> None:
70+
from adcp_reporting_storage import ReportingSchemaAheadError
71+
72+
async with isolated_reporting_pool(autocommit=True) as pool:
73+
store = await _upgraded_store(pool)
74+
async with pool.connection() as connection:
75+
before = await (
76+
await connection.execute(
77+
"SELECT pg_get_constraintdef(oid) FROM pg_constraint"
78+
" WHERE conname = 'reporting_configurations_pkey'"
79+
" AND connamespace = current_schema()::regnamespace"
80+
)
81+
).fetchall()
82+
with pytest.raises(ReportingSchemaAheadError) as refused:
83+
await store.create_schema()
84+
_assert_remedy(str(refused.value))
85+
async with pool.connection() as connection:
86+
after = await (
87+
await connection.execute(
88+
"SELECT pg_get_constraintdef(oid) FROM pg_constraint"
89+
" WHERE conname = 'reporting_configurations_pkey'"
90+
" AND connamespace = current_schema()::regnamespace"
91+
)
92+
).fetchall()
93+
assert after == before and "offering_id" in before[0][0]
94+
95+
96+
@pytest.mark.asyncio
97+
async def test_the_ddl_alone_names_contract_2_not_the_ownership_migration() -> None:
98+
"""For operators who apply the packaged SQL themselves."""
99+
from importlib.resources import files
100+
101+
ddl = (
102+
files("adcp_reporting_storage.ledger")
103+
.joinpath("reporting_ledger_account_generations.sql")
104+
.read_text()
105+
)
106+
async with isolated_reporting_pool(autocommit=True) as pool: # skips without psycopg
107+
from psycopg import errors
108+
109+
await _upgraded_store(pool)
110+
async with pool.connection() as connection:
111+
with pytest.raises(errors.RaiseException) as raised:
112+
await connection.execute(ddl)
113+
diagnostic = raised.value.diag
114+
assert "storage contract 2" in (diagnostic.message_primary or "")
115+
assert "Do not run migrate_legacy_reporting" in (diagnostic.message_hint or "")
116+
117+
118+
@pytest.mark.asyncio
119+
async def test_a_running_process_names_the_upgrade_on_every_widened_write() -> None:
120+
from adcp_reporting_storage import ReportingSchemaAheadError
121+
122+
async with isolated_reporting_pool(autocommit=True) as pool:
123+
store = await _upgraded_store(pool)
124+
with pytest.raises(ReportingSchemaAheadError) as refused:
125+
await store.put_configuration(configuration("acct_b"))
126+
_assert_remedy(str(refused.value))
127+
with pytest.raises(LedgerConflictError) as conflict:
128+
await store.commit_obligation(obligation_for(configuration("acct_a")))
129+
assert conflict.value.code == "REPORTING_SCHEMA_AHEAD"
130+
_assert_remedy(str(conflict.value))

‎tests/fixtures/public_api_snapshot.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8225,6 +8225,7 @@
82258225
"ReportingRowQuarantineReason": "union[value:'BINDING_MISMATCH',value:'ROWS_NOT_PORTABLE',value:'ROW_SEQUENCE_INVALID']",
82268226
"ReportingRowStorageOptionsV1": "adcp_reporting_storage.ledger.row_storage",
82278227
"ReportingRowStorageV1": "adcp_reporting_storage.ledger.row_storage",
8228+
"ReportingSchemaAheadError": "adcp_reporting_storage.migration",
82288229
"assert_gcs_reporting_row_bucket_policy_v1": "adcp_reporting_storage.ledger.row_storage_gcs",
82298230
"backfill_legacy_reporting": "adcp_reporting_storage.migration",
82308231
"create_azure_blob_reporting_row_object_provider_v1": "adcp_reporting_storage.ledger.row_storage_azure",

0 commit comments

Comments
 (0)