configstore: remove dead/footgun config-store surface (singletons, warehouse + org columns)#822
Merged
Merged
Conversation
Audit-driven cleanup of config-store surface that is persisted/served but never
read to drive runtime behavior. No user-visible behavior change.
Removed (confirmed dead by a full cross-repo audit incl. the Crossplane
compositions in posthog/charts):
- The four cluster-wide singleton config tables — GlobalConfig, DuckLakeConfig,
RateLimitConfig, QueryLogConfig — and their Snapshot fields, FirstOrCreate
seeds, snapshot loads, the admin GET/PUT /config/* API (handlers, routes,
apiStore methods, gormAPIStore impls), and the /settings dashboard page.
Nothing reads Snapshot.Global/DuckLake/RateLimit/QueryLog; effective config
comes from CLI flags/env (server.Config) and the per-org ManagedWarehouse
contract. (DuckLakeConfig was already marked legacy.)
- ManagedWarehouse dead columns: warehouse_database_{region,database_name,
username}, metadata_store_{engine,region}, worker_identity_service_account_name
(the worker SA name is computed, not read from config), warehouse_database_state
(warehouse_database has no provisioning sub-state), and the six per-component
*_status_message columns (the provisioner only writes the top-level
status_message). Kept warehouse_database endpoint/port (curated provision
response) and the per-component *_state fields that drive readiness.
- Runtime coordination dead columns: cp_instances.pod_uid/boot_id (already
encoded into the cp instance id), worker_records.pod_uid (never written),
org_connection_queue.canceled_at (cancel is a hard DELETE, never set) — plus
the ControlPlaneRuntimeTracker podUID/bootID fields and constructor params.
Migration & rollout safety:
- New goose migration 000004 drops the four config tables and the 13 dead
managed_warehouses columns (with a best-effort Down that recreates them).
- Runtime tables are AutoMigrate'd in a per-deployment schema, so the dead
runtime columns are dropped in autoMigrateRuntimeTables (schema-qualified,
DROP COLUMN IF EXISTS, idempotent). This runs before the first heartbeat —
critical because cp_instances.pod_uid/boot_id were NOT NULL, so a rollout that
stopped supplying them would otherwise fail every UpsertControlPlaneInstance.
Also updated: seed SQL (k8s + test fixtures), all affected unit/postgres tests
(including the migrations metadata-parity test, now asserting the config tables
are absent at version 4), and the README note about the former DuckLakeConfig
singleton.
Verified: `go build ./...`, `go build -tags kubernetes ./controlplane/... ./cmd/...`,
all kubernetes-tagged test binaries compile, and the configstore + admin
no-docker unit tests pass.
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: needs review Warnings
|
#821 (models explorer UI) merged to main after this branch was cut. Its models_api.go registered the four config-singleton models this branch deletes, so the merged tree failed to build (undefined: configstore.GlobalConfig etc). Merge main in and reconcile: - Remove the global/ducklake/ratelimit/querylog descriptors from models_api.go (and the now-unused modelGroupConfig group). - Drop them from models_api_test.go (wantKeys + the global-config redaction entry) and the harness models_explorer_api sidebar key list. - Drop the dead Settings nav link + the empty Config sidebar group from models.html, and the /settings row from admin/README.md (the /settings page and its config API were removed on this branch). Build (plain + kubernetes), all kubernetes test binaries, and the admin redaction test pass.
benben
changed the title
benben
added a commit
that referenced
this pull request
Jun 29, 2026
Removes four duckgres_orgs columns: - memory_budget, idle_timeout_s — dead: only copied to the snapshot OrgConfig + shown in admin OrgStatus; no behavioral reader. - worker_cpu_request, worker_memory_request — footgun: org_router applied the per-org value to the SHARED worker pool (last-org-wins pod sizing across all orgs). Per-org sizing stays via default_worker_cpu/memory + client GUCs; the deployment-wide K8sConfig.WorkerCPURequest/MemoryRequest (charts/env) is a separate, kept field. Drops the two SetWorkerResources footgun blocks from org_router; new migration 000005 drops the columns (Down re-adds); updates OrgConfig snapshot view, admin OrgStatus, seeds, and tests. Split out of #822 (which squash-merged before this landed): same configstore cleanup, new migration version since 000004 is already merged/applied.
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
1 participant
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.
Audit-driven removal of config-store schema/fields that are persisted/served but never read to drive behavior (or are outright footguns). No user-visible behavior change. Driven by a full cross-repo audit (control plane + the Crossplane compositions in posthog/charts).
Removed
Cluster-wide singleton config tables —
GlobalConfig,DuckLakeConfig,RateLimitConfig,QueryLogConfig: tables,Snapshotfields, seeds, snapshot loads, adminGET/PUT /config/*API, and the/settingsdashboard page. Effective config comes from CLI flags/env (server.Config) + the per-orgManagedWarehousecontract.ManagedWarehousedead columns (13) —warehouse_database_{region,database_name,username},metadata_store_{engine,region},worker_identity_service_account_name(worker SA is computed, not read),warehouse_database_state, and the 6 per-component*_status_messagecolumns (provisioner only writes the top-levelstatus_message). Keptwarehouse_databaseendpoint/port + the per-component*_statefields.Runtime coordination dead columns —
cp_instances.{pod_uid,boot_id}(already encoded in the instance id),worker_records.pod_uid(never written),org_connection_queue.canceled_at(cancel is a DELETE).Org dead/footgun columns (4) —
memory_budget,idle_timeout_s(dead: only copied to the snapshot + shown in admin status);worker_cpu_request,worker_memory_request(footgun:org_routerapplied the per-org value to the shared worker pool → last-org-wins pod sizing across all orgs). Legit per-org sizing stays viadefault_worker_*+ client GUCs; the deployment-wideK8sConfig.WorkerCPURequest/MemoryRequest(charts/env) is a different field, untouched.Migration / rollout safety
000004drops the 4 config tables + 13 warehouse columns + 4 org columns (Down recreates).autoMigrateRuntimeTables(DROP COLUMN IF EXISTS, idempotent, runs before first heartbeat —cp_instances.pod_uid/boot_idwere NOT NULL, so a rollout that stopped supplying them would otherwise fail every upsert).Reconciled with #821
The models-explorer (
models_api.go) dropped its 4 config-singleton descriptors + harness/test references.Verification
go build ./...,go build -tags kubernetes ./controlplane/... ./cmd/..., all test binaries compile (both tag sets), configstore + admin + control-plane unit tests pass. Postgres/e2e in CI.🤖 Generated with Claude Code