Skip to content

Commit f0f46bb

Browse files
committed
fix(breg): keep role changes from widening a legacy ledger
A role change re-applies the active package, whose schema fingerprint an earlier release measured with the 1,024-statement ledger constraint. Widening that constraint during the role change made the measured catalog differ from the declared one and failed the activation. A fresh ledger now carries the 4,096-statement constraint from creation, and only a successor to a different package widens a legacy one. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
1 parent 1f84a9c commit f0f46bb

4 files changed

Lines changed: 196 additions & 53 deletions

File tree

‎crates/registry-breg/src/postgres/interlock.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2605,7 +2605,12 @@ async fn begin_successor_in(
26052605
.is_some();
26062606
if fresh {
26072607
verify_import_run_admission_state(transaction, current.activation_uuid()?).await?;
2608-
ensure_migration_statement_capacity(transaction).await?;
2608+
// A role change re-applies the active package, whose fingerprint
2609+
// covers the ledger constraint already in place; only a successor to
2610+
// a different package may widen it.
2611+
if target.package_digest != current.package_digest {
2612+
ensure_migration_statement_capacity(transaction).await?;
2613+
}
26092614
}
26102615
let changed = transaction
26112616
.execute(

‎crates/registry-breg/src/postgres/migration_ledger.rs‎

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ pub(crate) async fn install_migration_ledger(
300300
migration: &impl GenericClient,
301301
runtime_role: &SqlIdentifier,
302302
) -> Result<()> {
303-
let ledger_ddl =
303+
let ledger_ddl = format!(
304304
"CREATE TABLE IF NOT EXISTS registry_internal.registry_migrations (
305305
activation_id uuid PRIMARY KEY,
306306
apply_order bigint NOT NULL
@@ -309,10 +309,10 @@ pub(crate) async fn install_migration_ledger(
309309
CHECK (apply_order > 0),
310310
package_digest text NOT NULL
311311
CONSTRAINT registry_migrations_package_digest_sha256
312-
CHECK (package_digest ~ '^sha256:[0-9a-f]{64}$'),
312+
CHECK (package_digest ~ '^sha256:[0-9a-f]{{64}}$'),
313313
predecessor_package_digest text
314314
CONSTRAINT registry_migrations_predecessor_digest_sha256
315-
CHECK (predecessor_package_digest ~ '^sha256:[0-9a-f]{64}$'),
315+
CHECK (predecessor_package_digest ~ '^sha256:[0-9a-f]{{64}}$'),
316316
registry_revision text NOT NULL
317317
CONSTRAINT registry_migrations_registry_revision_nonempty
318318
CHECK (registry_revision <> ''),
@@ -372,6 +372,9 @@ pub(crate) async fn install_migration_ledger(
372372
),
373373
CONSTRAINT registry_migrations_applied_at_consistent CHECK (
374374
(outcome = 'applied') = (applied_at IS NOT NULL)
375+
),
376+
CONSTRAINT registry_migrations_statement_checksums_bounded_v2 CHECK (
377+
{statement_checksums_bounded}
375378
)
376379
);
377380
CREATE UNIQUE INDEX IF NOT EXISTS registry_migrations_one_open_activation_per_digest
@@ -404,9 +407,10 @@ pub(crate) async fn install_migration_ledger(
404407
)
405408
);
406409
REVOKE ALL ON TABLE registry_internal.registry_migrations FROM PUBLIC;
407-
REVOKE ALL ON TABLE registry_internal.registry_migration_steps FROM PUBLIC;";
408-
migration.batch_execute(ledger_ddl).await?;
409-
ensure_migration_statement_capacity(migration).await?;
410+
REVOKE ALL ON TABLE registry_internal.registry_migration_steps FROM PUBLIC;",
411+
statement_checksums_bounded = statement_checksums_bounded_check(),
412+
);
413+
migration.batch_execute(&ledger_ddl).await?;
410414
let revoke = RuntimeRevoke::detect(migration, runtime_role).await?;
411415
migration
412416
.batch_execute(&format!(
@@ -419,9 +423,28 @@ pub(crate) async fn install_migration_ledger(
419423
Ok(())
420424
}
421425

426+
/// The body of `registry_migrations_statement_checksums_bounded_v2`, shared
427+
/// by a fresh ledger and the successor that widens a legacy one, so both
428+
/// catalogs fingerprint the same.
429+
fn statement_checksums_bounded_check() -> String {
430+
format!(
431+
"COALESCE(array_ndims(statement_checksums), 1) = 1
432+
AND cardinality(statement_checksums) BETWEEN 0 AND {MAX_MIGRATION_STATEMENTS}
433+
AND array_position(statement_checksums, '') IS NULL
434+
AND (
435+
(migration_kind = 'metadata-only' AND cardinality(statement_checksums) = 0)
436+
OR (migration_kind = 'reviewed' AND cardinality(statement_checksums) = 0)
437+
OR (migration_kind IN ('compiled-additive', 'reviewed')
438+
AND cardinality(statement_checksums) BETWEEN 1 AND {MAX_MIGRATION_STATEMENTS})
439+
)"
440+
)
441+
}
442+
422443
/// Widens the legacy ledger statement bound inside the caller's
423-
/// migration-authorized transaction. The versioned constraint makes repeated
424-
/// startup, rehearsal, and successor retries no-ops after the first upgrade.
444+
/// migration-authorized transaction. Only a successor to a different package
445+
/// calls it: that package's fingerprint was measured on a fresh ledger, which
446+
/// already carries the versioned constraint. The versioned constraint makes
447+
/// successor retries and rehearsals no-ops after the first upgrade.
425448
pub(crate) async fn ensure_migration_statement_capacity(
426449
migration: &impl GenericClient,
427450
) -> Result<()> {
@@ -444,20 +467,11 @@ pub(crate) async fn ensure_migration_statement_capacity(
444467
) THEN
445468
ALTER TABLE registry_internal.registry_migrations
446469
ADD CONSTRAINT registry_migrations_statement_checksums_bounded_v2
447-
CHECK (
448-
COALESCE(array_ndims(statement_checksums), 1) = 1
449-
AND cardinality(statement_checksums) BETWEEN 0 AND {MAX_MIGRATION_STATEMENTS}
450-
AND array_position(statement_checksums, '') IS NULL
451-
AND (
452-
(migration_kind = 'metadata-only' AND cardinality(statement_checksums) = 0)
453-
OR (migration_kind = 'reviewed' AND cardinality(statement_checksums) = 0)
454-
OR (migration_kind IN ('compiled-additive', 'reviewed')
455-
AND cardinality(statement_checksums) BETWEEN 1 AND {MAX_MIGRATION_STATEMENTS})
456-
)
457-
);
470+
CHECK ({statement_checksums_bounded});
458471
END IF;
459472
END
460-
$migration_statement_capacity$;"
473+
$migration_statement_capacity$;",
474+
statement_checksums_bounded = statement_checksums_bounded_check(),
461475
))
462476
.await?;
463477
Ok(())

‎crates/registry-breg/tests/postgres_package/migration_capacity.rs‎

Lines changed: 154 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,69 @@ fn preassertion_review_source(
251251
}
252252
}
253253

254+
/// Puts back the 1,024-statement ledger constraint an earlier release
255+
/// installed, and returns the schema fingerprint of the catalog that leaves.
256+
async fn reinstall_legacy_statement_constraint(
257+
database: &TestDatabase,
258+
registry: &registry_breg::CompiledRegistry,
259+
) -> String {
260+
database
261+
.admin
262+
.batch_execute(
263+
"ALTER TABLE registry_internal.registry_migrations
264+
DROP CONSTRAINT registry_migrations_statement_checksums_bounded_v2;
265+
ALTER TABLE registry_internal.registry_migrations
266+
ADD CONSTRAINT registry_migrations_checksums_nonempty
267+
CHECK (
268+
COALESCE(array_ndims(statement_checksums), 1) = 1
269+
AND cardinality(statement_checksums) BETWEEN 0 AND 1024
270+
AND array_position(statement_checksums, '') IS NULL
271+
AND (
272+
(migration_kind = 'metadata-only' AND cardinality(statement_checksums) = 0)
273+
OR (migration_kind = 'reviewed' AND cardinality(statement_checksums) = 0)
274+
OR (migration_kind IN ('compiled-additive', 'reviewed')
275+
AND cardinality(statement_checksums) BETWEEN 1 AND 1024)
276+
)
277+
);",
278+
)
279+
.await
280+
.expect("fixture reinstalls the legacy statement constraint");
281+
let (migration, migration_task) = database.connect_migration().await;
282+
let legacy_fingerprint = managed_schema_fingerprint(
283+
&migration,
284+
&database.runtime_role,
285+
&ExpectedManagedCatalog::compiled(registry),
286+
)
287+
.await
288+
.expect("the legacy ledger catalog fingerprints");
289+
drop(migration);
290+
migration_task.abort();
291+
legacy_fingerprint
292+
}
293+
294+
/// The ledger statement constraints present: (v2, legacy).
295+
async fn ledger_statement_constraints(database: &TestDatabase) -> (bool, bool) {
296+
let row = database
297+
.admin
298+
.query_one(
299+
"SELECT
300+
EXISTS (
301+
SELECT 1 FROM pg_catalog.pg_constraint
302+
WHERE conrelid = 'registry_internal.registry_migrations'::regclass
303+
AND conname = 'registry_migrations_statement_checksums_bounded_v2'
304+
),
305+
EXISTS (
306+
SELECT 1 FROM pg_catalog.pg_constraint
307+
WHERE conrelid = 'registry_internal.registry_migrations'::regclass
308+
AND conname = 'registry_migrations_checksums_nonempty'
309+
)",
310+
&[],
311+
)
312+
.await
313+
.expect("ledger constraints are inspectable");
314+
(row.get(0), row.get(1))
315+
}
316+
254317
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
255318
async fn reviewed_flow_package_beyond_the_old_statement_limit_prepares_and_activates() {
256319
let source = serde_json::to_vec(&reviewed_flow_project(4, PROPOSAL_OPERATIONS))
@@ -345,37 +408,7 @@ async fn reviewed_successor_upgrades_the_legacy_ledger_and_applies_more_than_102
345408
.await
346409
.expect("prior package activates");
347410

348-
database
349-
.admin
350-
.batch_execute(
351-
"ALTER TABLE registry_internal.registry_migrations
352-
DROP CONSTRAINT registry_migrations_statement_checksums_bounded_v2;
353-
ALTER TABLE registry_internal.registry_migrations
354-
ADD CONSTRAINT registry_migrations_checksums_nonempty
355-
CHECK (
356-
COALESCE(array_ndims(statement_checksums), 1) = 1
357-
AND cardinality(statement_checksums) BETWEEN 0 AND 1024
358-
AND array_position(statement_checksums, '') IS NULL
359-
AND (
360-
(migration_kind = 'metadata-only' AND cardinality(statement_checksums) = 0)
361-
OR (migration_kind = 'reviewed' AND cardinality(statement_checksums) = 0)
362-
OR (migration_kind IN ('compiled-additive', 'reviewed')
363-
AND cardinality(statement_checksums) BETWEEN 1 AND 1024)
364-
)
365-
);",
366-
)
367-
.await
368-
.expect("fixture reinstalls the legacy statement constraint");
369-
let (migration, migration_task) = database.connect_migration().await;
370-
let legacy_fingerprint = managed_schema_fingerprint(
371-
&migration,
372-
&database.runtime_role,
373-
&ExpectedManagedCatalog::compiled(&prior),
374-
)
375-
.await
376-
.expect("the legacy ledger catalog fingerprints");
377-
drop(migration);
378-
migration_task.abort();
411+
let legacy_fingerprint = reinstall_legacy_statement_constraint(&database, &prior).await;
379412
assert_ne!(
380413
legacy_fingerprint, current.schema_fingerprint,
381414
"the constraint version belongs to the managed schema fingerprint"
@@ -545,3 +578,93 @@ async fn reviewed_successor_upgrades_the_legacy_ledger_and_applies_more_than_102
545578
assert_eq!(ledger, (ledger_statement_count as i32, true, false));
546579
database.cleanup().await;
547580
}
581+
582+
/// A role change re-applies the active package and runs no DDL, so a registry
583+
/// an earlier release activated keeps the ledger constraint the schema
584+
/// fingerprint of that package covers. Only a successor widens the ledger.
585+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
586+
async fn role_change_keeps_the_legacy_ledger_constraint_its_active_package_declares() {
587+
let source = serde_json::to_vec(&reviewed_flow_project(1, &[])).expect("fixture serializes");
588+
let project = parse_project_json(&source).expect("fixture project parses");
589+
let compiled = compile_project(&project, &[], CompileProfile::Production)
590+
.expect("fixture compiles in production");
591+
592+
let database = TestDatabase::create(1).await;
593+
let fingerprint = measure_fresh_compiled_schema(&database, &compiled).await;
594+
let prepared = prepare_package(capacity_request(
595+
source.clone(),
596+
fingerprint,
597+
None,
598+
PackageMigrationPlanInput::InitialCompiledDdl,
599+
))
600+
.expect("package prepares");
601+
let root = TempRoot::create();
602+
prepared
603+
.publish_to_directory(root.path())
604+
.expect("package publishes");
605+
let package = load_package(root.path(), &local_context()).expect("package verifies");
606+
let active = apply_package(
607+
&database,
608+
&package,
609+
ApplyPrecondition::InitialActivation,
610+
Duration::from_secs(30),
611+
Duration::from_secs(60),
612+
)
613+
.await
614+
.expect("package activates");
615+
616+
// The registry as an earlier release left it: the legacy ledger
617+
// constraint, and an active package whose declared schema fingerprint
618+
// was measured with that constraint.
619+
let legacy_fingerprint = reinstall_legacy_statement_constraint(&database, &compiled).await;
620+
let legacy_prepared = prepare_package(capacity_request(
621+
source,
622+
legacy_fingerprint.clone(),
623+
None,
624+
PackageMigrationPlanInput::InitialCompiledDdl,
625+
))
626+
.expect("the earlier release's package prepares");
627+
let legacy_root = TempRoot::create();
628+
legacy_prepared
629+
.publish_to_directory(legacy_root.path())
630+
.expect("the earlier release's package publishes");
631+
let legacy_package = load_package(legacy_root.path(), &local_context())
632+
.expect("the earlier release's package verifies");
633+
database
634+
.admin
635+
.batch_execute(&format!(
636+
"UPDATE registry_internal.registry_state
637+
SET active_package_digest = '{digest}', schema_fingerprint = '{legacy_fingerprint}'
638+
WHERE singleton AND active_activation_id = '{activation}'::uuid;
639+
UPDATE registry_internal.registry_migrations
640+
SET package_digest = '{digest}'
641+
WHERE activation_id = '{activation}'::uuid;",
642+
digest = legacy_package.package_digest(),
643+
activation = active.activation_id,
644+
))
645+
.await
646+
.expect("fixture records the earlier release's active package");
647+
let current = ExpectedRegistryIdentity {
648+
package_digest: legacy_package.package_digest().to_owned(),
649+
schema_fingerprint: legacy_fingerprint.clone(),
650+
..active
651+
};
652+
653+
// The re-apply serves with the migration role alone, so it is a role
654+
// change rather than a re-apply that leaves nothing to activate.
655+
let reapplied = apply_verified_package(ApplyVerifiedPackageRequest::new(
656+
&database.migration_config,
657+
&legacy_package,
658+
ActivationDeployment::new("local", INSTANCE, DATABASE),
659+
ApplyPrecondition::RoleChange { current: &current },
660+
ApplyRoles::new(&database.migration_role, &database.migration_role),
661+
ApplyTimeouts::new(Duration::from_secs(30), Duration::from_secs(60))
662+
.expect("role change timeouts are bounded"),
663+
database.activation_audit(),
664+
))
665+
.await
666+
.expect("the role change re-applies the package the earlier release activated");
667+
assert_eq!(reapplied.schema_fingerprint, legacy_fingerprint);
668+
assert_eq!(ledger_statement_constraints(&database).await, (false, true));
669+
database.cleanup().await;
670+
}

‎products/breg/README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ The first successor on a database with the legacy 1,024-statement ledger bound
217217
widens that constraint when maintenance begins. This engine catalog change
218218
remains if a later precondition or migration step fails. Correct the cause and
219219
retry the exact pinned target; if it cannot complete and reconciliation reports
220-
`unresolvable`, restore the pre-activation backup.
220+
`unresolvable`, restore the pre-activation backup. A role change re-applies the
221+
active package and leaves the ledger bound as that package declares it.
221222

222223
Authoring and migration authority are deliberately separate. An author or
223224
coding agent can edit configuration, inspect a diff, and run checks. Those

0 commit comments

Comments
 (0)