Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion e2e/consumermodule/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ require (
github.com/aws/smithy-go v1.27.7 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6 // indirect
github.com/block/pg-sprite v0.2.0 // indirect
github.com/block/pg-sprite v0.3.1 // indirect
github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c // indirect
github.com/bradleyfalzon/ghinstallation/v2 v2.18.0 // indirect
github.com/cenkalti/backoff/v5 v5.0.3 // indirect
Expand Down
4 changes: 2 additions & 2 deletions e2e/consumermodule/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6 h1:GvubwsqXHanJkhBotCs4XdEmSnwzhHQe7DVGrn+NFok=
github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6/go.mod h1:KEo73lbxXs9cFlq+x3Z35UqGg3MTxAPfjDOR/ob/iik=
github.com/block/pg-sprite v0.2.0 h1:H6w/MNJf1rc7XtdVEI0Sq63I2+MkiifPgkS3qfZ9Rz8=
github.com/block/pg-sprite v0.2.0/go.mod h1:vZxHdTMrCOPAYgswveB7PSjOaOuRgnDLGRw6WoOizRg=
github.com/block/pg-sprite v0.3.1 h1:l2w3aAFql+RvKnXoE7aHh1xx0Umj6pRsnjhsSUeEB0g=
github.com/block/pg-sprite v0.3.1/go.mod h1:vZxHdTMrCOPAYgswveB7PSjOaOuRgnDLGRw6WoOizRg=
github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c h1:Gdd1vWs0UKLlvq84+4wMveyls7QkFqhQDpF21KR2cIA=
github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c/go.mod h1:Lg97/e4zr2X3AQXRUrDVAQZOVqFDZ09px503h4v/Yss=
github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea h1:t9VROoN/aCzwwh7Ap90TLijZz3rZHZ6HUgWymC+yVS8=
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.44.5
github.com/aws/aws-sdk-go-v2/service/sts v1.43.3
github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6
github.com/block/pg-sprite v0.2.0
github.com/block/pg-sprite v0.3.1
github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c
github.com/bradleyfalzon/ghinstallation/v2 v2.18.0
github.com/charmbracelet/bubbles v1.0.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6 h1:GvubwsqXHanJkhBotCs4XdEmSnwzhHQe7DVGrn+NFok=
github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6/go.mod h1:KEo73lbxXs9cFlq+x3Z35UqGg3MTxAPfjDOR/ob/iik=
github.com/block/pg-sprite v0.2.0 h1:H6w/MNJf1rc7XtdVEI0Sq63I2+MkiifPgkS3qfZ9Rz8=
github.com/block/pg-sprite v0.2.0/go.mod h1:vZxHdTMrCOPAYgswveB7PSjOaOuRgnDLGRw6WoOizRg=
github.com/block/pg-sprite v0.3.1 h1:l2w3aAFql+RvKnXoE7aHh1xx0Umj6pRsnjhsSUeEB0g=
github.com/block/pg-sprite v0.3.1/go.mod h1:vZxHdTMrCOPAYgswveB7PSjOaOuRgnDLGRw6WoOizRg=
github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c h1:Gdd1vWs0UKLlvq84+4wMveyls7QkFqhQDpF21KR2cIA=
github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c/go.mod h1:Lg97/e4zr2X3AQXRUrDVAQZOVqFDZ09px503h4v/Yss=
github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea h1:t9VROoN/aCzwwh7Ap90TLijZz3rZHZ6HUgWymC+yVS8=
Expand Down
107 changes: 75 additions & 32 deletions pkg/engine/postgres/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,17 +177,21 @@ func (e *Engine) runOptimisticApply(ctx context.Context, conn targetConn, change
}

var invalidErr *executor.InvalidIndexError
if errors.As(err, &invalidErr) {
// An invalid index — pre-existing or a build's own unrecovered
// leftover — is operational: an operator clears it and a retry can
// succeed. Checked before the refusal and budget arms because the
// verdict wraps the build failure that produced it (a budget-
// cancelled build leaves its own invalid index), and that inner
// cause must not be read as the outcome — the index the operator
// clears is. The detail is built from the typed identifiers and
// verdict code, never the wrapped build or cleanup errors, which
// may carry raw server text; the full cause lands in the server
// log below it.
if errors.As(err, &invalidErr) && !invalidErr.Code().Permanent() {
// An invalid index an operator can clear — a build's own leftover,
// abandoned debris, another backend's build to wait out, or a
// builder this role cannot observe — is operational: once it is
// cleared a retry can succeed. Checked before the refusal and budget
// arms because the verdict wraps the build failure that produced it
// (a budget-cancelled build leaves its own invalid index), and that
// inner cause must not be read as the outcome — the index the
// operator clears is. The permanent verdicts (the name is occupied
// on another table, or by an index the server will not drop
// concurrently) fall through to classifyRefusal: retrying unchanged
// reproduces them. The detail is built from the typed identifiers
// and verdict code, never the wrapped build or cleanup errors, which
// may carry raw server text; the full cause lands in the server log
// below it.
logger.Error("PostgreSQL concurrent index build left or found an invalid index",
"namespace", change.namespace, "table", change.table,
"index_schema", invalidErr.Schema, "index", invalidErr.Index, "error", err)
Expand Down Expand Up @@ -256,10 +260,13 @@ func classifyApplyFailure(err error, table string) applyFailure {

// invalidIndexDetail renders the operator-facing next step for an
// invalid-index verdict, matching pg-sprite's own ownership standard: a drop
// is named only when the entry is proven this build's own leftover. A
// pre-existing invalid entry may be another actor's still-running build, and
// an unproven verdict may sit on a healthy index — both get investigation
// steps, never a statement to run. Only the typed identifiers are
// is named only where the executor proved the entry is a failed build's
// debris on the target table — this build's own leftover, or an abandoned
// entry with no builder. A build still in flight says wait; an entry on
// another table, one the server will not drop concurrently, one whose
// builder this role cannot see, and an unproven verdict get investigation
// steps, never a statement to run — the index under the name may be healthy
// or may be exactly what it is meant to be. Only the typed identifiers are
// interpolated, never the wrapped build or cleanup errors, which may carry
// raw server text.
func invalidIndexDetail(invalidErr *executor.InvalidIndexError) string {
Expand All @@ -268,8 +275,16 @@ func invalidIndexDetail(invalidErr *executor.InvalidIndexError) string {
switch invalidErr.Code() {
case executor.CodeInvalidIndexOwnLeftover:
advice = fmt.Sprintf("this build left its own invalid index %s on the target; drop the invalid index, then retry", name)
case executor.CodeInvalidIndexPreexisting:
advice = fmt.Sprintf("an invalid index %s already occupies the name on the target and may be another actor's build still in progress; check pg_stat_activity before any recovery, then retry", name)
case executor.CodeInvalidIndexAbandoned:
advice = fmt.Sprintf("an abandoned invalid index %s occupies the name on the target table with no backend building it; confirm it is still invalid with no builder, drop the invalid index, then retry", name)
case executor.CodeInvalidIndexBuildInFlight:
advice = fmt.Sprintf("an invalid index %s occupies the name and backend %d is still building it; wait for that build to finish or fail, then retry", name, invalidErr.BuilderPID)
case executor.CodeInvalidIndexBuilderUnobservable:
advice = fmt.Sprintf("an invalid index %s occupies the name on the target table and the engine role cannot observe whether a backend is building it; check pg_stat_progress_create_index with a role granted pg_read_all_stats before any recovery, then retry", name)
case executor.CodeInvalidIndexOtherTable:
advice = fmt.Sprintf("an invalid index %s already occupies the name on a different table%s; this change cannot claim it — rename the index in the schema file and re-plan, or clear the entry through that table's own change", name, invalidIndexTableSuffix(invalidErr))
case executor.CodeInvalidIndexNotDroppable:
advice = fmt.Sprintf("an invalid index %s occupies the name and is a partitioned table's index, an index partition, or a constraint's index rather than a failed build's leftover; an operator must resolve it on the target, or rename the index in the schema file and re-plan", name)
default:
// CodeInvalidIndexUnproven and any future verdict fail safe with
// investigation steps: the index under the name may be healthy.
Expand All @@ -278,6 +293,16 @@ func invalidIndexDetail(invalidErr *executor.InvalidIndexError) string {
return sanitizeReasonText(advice)
}

// invalidIndexTableSuffix names the table the invalid index sits on when the
// catalog inspection saw it; the verdict carries no table when the state
// could not be inspected.
func invalidIndexTableSuffix(invalidErr *executor.InvalidIndexError) string {
if invalidErr.Table == "" {
return ""
}
return fmt.Sprintf(" (%q)", invalidErr.Table)
}

// refusal is a typed apply outcome that retrying cannot fix: the schema
// change, the target table, or role provisioning must change first.
type refusal struct {
Expand Down Expand Up @@ -349,15 +374,21 @@ func refusalForCause(err error, table string) *refusal {
}
return &refusal{reason: "insufficient-privileges", detail: detail}
}
// An invalid-index verdict is operational even when the build failure it
// wraps would classify as a refusal on its own — a budget-cancelled
// concurrent build leaves its own invalid index, and the index the
// operator clears is the outcome, not the inner budget exhaustion.
// Declined before the budget arm so the nested cause can never shadow
// the verdict.
// An invalid-index verdict is decided by its own code, never by the build
// failure it wraps — a budget-cancelled concurrent build leaves its own
// invalid index, and the index the operator clears is the outcome, not
// the inner budget exhaustion. Decided before the budget arm so the
// nested cause can never shadow the verdict. Only the permanent members
// of the family refuse: the name is occupied on another table, or by an
// index the server will not drop concurrently, so retrying unchanged
// reproduces the verdict. Every other member is operational.
var invalidErr *executor.InvalidIndexError
if errors.As(err, &invalidErr) {
return nil
r, _ := refusalForOutcome(invalidErr.Code(), table)
if r != nil {
r.detail = invalidIndexDetail(invalidErr)
}
return r
}
var budgetErr *executor.BudgetError
if errors.As(err, &budgetErr) && budgetErr.Cause == executor.CauseStatement {
Expand Down Expand Up @@ -444,14 +475,26 @@ func refusalForOutcome(code executor.Code, table string) (*refusal, bool) {
// apply fails closed until an operator has inspected the target.
return &refusal{reason: "engine-invariant-violation",
detail: fmt.Sprintf("the engine's safety invariants did not hold while changing table %q; inspect the target and server logs before re-running", table)}, true
case executor.CodeBudgetLockExceeded, executor.CodeCancelledExternally,
executor.CodeInvalidIndexOwnLeftover, executor.CodeInvalidIndexPreexisting,
executor.CodeInvalidIndexUnproven, executor.CodePoolTooSmall,
executor.CodeExecutionFailed:
// Operational outcomes: a bounded lock race, an external stop, an
// invalid-index state an operator clears, engine pool sizing, or a
// failure outside the typed set. A retry can succeed once
// conditions change, so none is a permanent refusal.
case executor.CodeInvalidIndexOtherTable, executor.CodeInvalidIndexNotDroppable:
// The permanent members of the invalid-index family: the requested
// name is held by an entry this change can never clear — an invalid
// index on a different table, or one the server will not drop
// concurrently (a partitioned table's index, an index partition, a
// constraint's index). Retrying unchanged reproduces the verdict.
// The typed-verdict path replaces this detail with the code's own
// advice; this mapping keeps the vocabulary total.
return &refusal{reason: "invalid-index-occupied",
detail: fmt.Sprintf("an invalid index already occupies a name the change to %q needs and is not a failed build's leftover; rename the index in the schema file and re-plan, or resolve the entry on the target", table)}, true
case executor.CodeBudgetLockExceeded, executor.CodeCancelledByCaller,
executor.CodeCancelledExternally, executor.CodeInvalidIndexOwnLeftover,
executor.CodeInvalidIndexAbandoned, executor.CodeInvalidIndexBuildInFlight,
executor.CodeInvalidIndexBuilderUnobservable, executor.CodeInvalidIndexUnproven,
executor.CodePoolTooSmall, executor.CodeExecutionFailed:
// Operational outcomes: a bounded lock race, the caller's own
// context ending or an external stop, an invalid-index state an
// operator clears or waits out, engine pool sizing, or a failure
// outside the typed set. A retry can succeed once conditions
// change, so none is a permanent refusal.
return nil, true
}
return nil, false
Expand Down
92 changes: 83 additions & 9 deletions pkg/engine/postgres/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,42 @@ func TestClassifyRefusal(t *testing.T) {
name: "external cancellation is operational",
err: fmt.Errorf("execute: %w", executor.ErrCancelledExternally),
},
{
name: "the caller's own cancellation is operational",
err: fmt.Errorf("execute: %w", executor.ErrCancelledByCaller),
},
{
name: "invalid index on another table is a refusal that renders the typed advice",
err: fmt.Errorf("execute: %w", &executor.InvalidIndexError{
Schema: "public",
Index: "users_ref_idx",
Table: "shipments",
Cleanup: executor.ErrInvalidIndexOnOtherTable,
}),
wantReason: "invalid-index-occupied",
wantDetail: []string{`"public"."users_ref_idx"`, `"shipments"`, "re-plan"},
wantNotDetail: []string{"drop the invalid index"},
},
{
name: "non-droppable invalid index is a refusal even when it wraps a statement-budget cause",
err: fmt.Errorf("execute: %w", &executor.InvalidIndexError{
Schema: "public",
Index: "users_pkey",
Table: "users",
Build: &executor.BudgetError{Cause: executor.CauseStatement, Budget: time.Second},
Cleanup: executor.ErrInvalidIndexNotDroppable,
}),
wantReason: "invalid-index-occupied",
wantDetail: []string{`"public"."users_pkey"`, "constraint's index", "operator must resolve"},
wantNotDetail: []string{"budget", "drop the invalid index"},
},
{
name: "abandoned invalid index is operational",
err: fmt.Errorf("execute: %w", &executor.InvalidIndexError{
Schema: "public", Index: "users_ref_idx", Table: "users",
Cleanup: executor.ErrAbandonedInvalidIndex,
}),
},
{
name: "partitioned-parent admission refusal renders the typed sentence",
err: fmt.Errorf("admit statement for partitioned PostgreSQL table %q: %w", "users",
Expand Down Expand Up @@ -268,11 +304,14 @@ func TestRetryPathFitsUnderApplyCeiling(t *testing.T) {
}

// TestInvalidIndexDetailMatchesVerdictOwnership pins the advice ladder to
// the verdict code: a drop is named only for the build's own proven
// leftover; a pre-existing entry gets an in-progress-build check; an
// unproven verdict gets catalog inspection because the index may be healthy.
// Every branch names the index and none renders the wrapped build or
// cleanup errors, which may carry raw server text.
// the verdict code: a drop is named only where the executor proved the entry
// is a failed build's debris on the target table — this build's own leftover
// or an abandoned entry; a build in flight says wait and names the builder;
// an entry on another table, one the server will not drop concurrently, one
// whose builder the role cannot see, and an unproven verdict get
// investigation steps because the index may be healthy or intentional. Every
// branch names the index and none renders the wrapped build or cleanup
// errors, which may carry raw server text.
func TestInvalidIndexDetailMatchesVerdictOwnership(t *testing.T) {
rawServerText := errors.New("ERROR: deadline exceeded at host db-internal-1.example.com")
tests := []struct {
Expand All @@ -289,12 +328,47 @@ func TestInvalidIndexDetailMatchesVerdictOwnership(t *testing.T) {
wantNotDetail: []string{"db-internal-1"},
},
{
name: "pre-existing entry gets an in-progress-build check, never a drop",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx",
Build: rawServerText, Cleanup: executor.ErrPreexistingInvalidIndex},
wantDetail: []string{`"public"."big_ref_idx"`, "another actor's build", "pg_stat_activity"},
name: "abandoned entry names the drop after a re-check",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx", Table: "orders",
Cleanup: executor.ErrAbandonedInvalidIndex},
wantDetail: []string{`"public"."big_ref_idx"`, "abandoned", "no backend building it", "drop the invalid index", "retry"},
wantNotDetail: []string{"db-internal-1"},
},
{
name: "build in flight says wait and names the builder, never a drop",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx", Table: "orders",
BuilderPID: 4242, Cleanup: executor.ErrInvalidIndexBuildInFlight},
wantDetail: []string{`"public"."big_ref_idx"`, "backend 4242", "still building it", "wait"},
wantNotDetail: []string{"drop the invalid index", "db-internal-1"},
},
{
name: "unobservable builder gets a privileged progress check, never a drop",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx", Table: "orders",
Cleanup: executor.ErrInvalidIndexBuilderUnobservable},
wantDetail: []string{`"public"."big_ref_idx"`, "cannot observe", "pg_stat_progress_create_index", "pg_read_all_stats"},
wantNotDetail: []string{"drop the invalid index", "db-internal-1"},
},
{
name: "entry on another table names that table and a re-plan, never a drop",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx", Table: "shipments",
Cleanup: executor.ErrInvalidIndexOnOtherTable},
wantDetail: []string{`"public"."big_ref_idx"`, "different table", `"shipments"`, "re-plan"},
wantNotDetail: []string{"drop the invalid index", "retry", "db-internal-1"},
},
{
name: "entry on another table with no inspected table name still re-plans",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx",
Cleanup: executor.ErrInvalidIndexOnOtherTable},
wantDetail: []string{`"public"."big_ref_idx"`, "different table;", "re-plan"},
wantNotDetail: []string{"drop the invalid index", `("")`},
},
{
name: "non-droppable entry is left to an operator, never a drop",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx", Table: "orders",
Cleanup: executor.ErrInvalidIndexNotDroppable},
wantDetail: []string{`"public"."big_ref_idx"`, "constraint's index", "operator must resolve", "re-plan"},
wantNotDetail: []string{"drop the invalid index", "retry", "db-internal-1"},
},
{
name: "unproven verdict gets catalog inspection, never a drop",
err: &executor.InvalidIndexError{Schema: "public", Index: "big_ref_idx",
Expand Down
Loading
Loading