refactor(storage): optimize gateway-controller DB schema indexes#1407
refactor(storage): optimize gateway-controller DB schema indexes#1407renuka-fernando wants to merge 1 commit intowso2:mainfrom
Conversation
- Put gateway_id as leading column in all UNIQUE constraints (certificates, llm_provider_templates, api_keys) so implicit B-tree indexes support gateway-scoped prefix queries - Remove indexes redundant with UNIQUE constraint implicit indexes: idx_cert_name, idx_certificates_gateway_id, idx_template_handle, idx_llm_provider_templates_gateway_id, idx_api_key, idx_api_key_api - Replace single-column indexes with composite (gateway_id, col) indexes since the DB is shared across multiple gateways - Drop indexes on api_keys columns (expires_at, source, external_ref_id) not used in any WHERE clause in the codebase - Change api_keys PRIMARY KEY from (api_key, gateway_id) to uuid; enforce uniqueness via UNIQUE(gateway_id, artifact_uuid, api_key) - Standardize index names to use full table names consistently
WalkthroughThis PR updates the database schema for the gateway controller, consolidating indexes into composite indexes across multiple tables and restructuring the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
gateway/gateway-controller/pkg/storage/gateway-controller-db.sql (1)
135-139: Consider clarifying the schema comment about index coverage or add a dedicated index for defensive programming.The schema comment (lines 135-137) states that
UNIQUE(gateway_id, artifact_uuid, api_key)covers prefix queries on(gateway_id)and(gateway_id, artifact_uuid). However, queries likeGetAPIKeyByKey(line 1320:WHERE api_key = ? AND gateway_id = ?) andDeleteAPIKey(line 1500:WHERE api_key = ? AND gateway_id = ?) filter onapi_keywithout includingartifact_uuid, which means the UNIQUE constraint's index cannot efficiently serve these queries due to the intervening column.That said,
GetAPIKeyByKeydoes not appear to be used in production code (only in test mocks). Actual API key authentication occurs through xDS state-of-the-world snapshots, not direct database lookups. If these queries remain unused or are called infrequently, the optimization is not urgent. If they become part of a hot path in the future, consider adding:CREATE INDEX IF NOT EXISTS idx_api_keys_gateway_id_api_key ON api_keys(gateway_id, api_key);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/storage/gateway-controller-db.sql` around lines 135 - 139, The schema comment incorrectly implies the UNIQUE(gateway_id, artifact_uuid, api_key) index covers queries filtering by api_key and gateway_id; because api_key is the third column it won't efficiently serve WHERE api_key = ? AND gateway_id = ? (used by GetAPIKeyByKey and DeleteAPIKey). Fix by either updating the comment to explicitly state the limitation (that prefix coverage does not include queries that omit artifact_uuid) or add a defensive index CREATE INDEX IF NOT EXISTS idx_api_keys_gateway_id_api_key ON api_keys(gateway_id, api_key) so queries in GetAPIKeyByKey/DeleteAPIKey are covered; reference the UNIQUE constraint UNIQUE(gateway_id, artifact_uuid, api_key) and the queries GetAPIKeyByKey and DeleteAPIKey when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway/gateway-controller/pkg/storage/gateway-controller-db.sql`:
- Around line 129-131: The foreign key from application_api_keys (FOREIGN KEY
(api_key_id, gateway_id) REFERENCES api_keys(uuid, gateway_id)) is invalid
because api_keys no longer has a UNIQUE constraint across (uuid, gateway_id);
update the schema by adding a UNIQUE constraint on the parent table api_keys for
the column pair (uuid, gateway_id) (or alternatively change the FK to reference
only uuid if that matches your model) so the referenced columns are either the
PRIMARY KEY or a UNIQUE key; modify the api_keys table definition (where UNIQUE
(gateway_id, artifact_uuid, name), UNIQUE (gateway_id, artifact_uuid, api_key),
PRIMARY KEY (uuid) are defined) to include UNIQUE(uuid, gateway_id) to satisfy
SQLite FK requirements.
---
Nitpick comments:
In `@gateway/gateway-controller/pkg/storage/gateway-controller-db.sql`:
- Around line 135-139: The schema comment incorrectly implies the
UNIQUE(gateway_id, artifact_uuid, api_key) index covers queries filtering by
api_key and gateway_id; because api_key is the third column it won't efficiently
serve WHERE api_key = ? AND gateway_id = ? (used by GetAPIKeyByKey and
DeleteAPIKey). Fix by either updating the comment to explicitly state the
limitation (that prefix coverage does not include queries that omit
artifact_uuid) or add a defensive index CREATE INDEX IF NOT EXISTS
idx_api_keys_gateway_id_api_key ON api_keys(gateway_id, api_key) so queries in
GetAPIKeyByKey/DeleteAPIKey are covered; reference the UNIQUE constraint
UNIQUE(gateway_id, artifact_uuid, api_key) and the queries GetAPIKeyByKey and
DeleteAPIKey when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08888168-0ac1-40a2-b44f-7c20ff7294d3
📒 Files selected for processing (1)
gateway/gateway-controller/pkg/storage/gateway-controller-db.sql
| UNIQUE (gateway_id, artifact_uuid, name), | ||
| UNIQUE (gateway_id, artifact_uuid, api_key), | ||
| PRIMARY KEY (uuid) |
There was a problem hiding this comment.
Foreign key constraint at Line 220 will break due to removed UNIQUE(uuid, gateway_id).
The application_api_keys table has FOREIGN KEY (api_key_id, gateway_id) REFERENCES api_keys(uuid, gateway_id). SQLite requires FK parent columns to be either the PRIMARY KEY or have a UNIQUE constraint. With PRIMARY KEY (uuid) only, and no UNIQUE(uuid, gateway_id) or UNIQUE(gateway_id, uuid), this FK reference is now invalid.
🐛 Proposed fix: Add UNIQUE constraint for the FK reference
FOREIGN KEY (artifact_uuid) REFERENCES artifacts(uuid) ON DELETE CASCADE,
UNIQUE (gateway_id, artifact_uuid, name),
UNIQUE (gateway_id, artifact_uuid, api_key),
+ UNIQUE (gateway_id, uuid),
PRIMARY KEY (uuid)
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway/gateway-controller/pkg/storage/gateway-controller-db.sql` around
lines 129 - 131, The foreign key from application_api_keys (FOREIGN KEY
(api_key_id, gateway_id) REFERENCES api_keys(uuid, gateway_id)) is invalid
because api_keys no longer has a UNIQUE constraint across (uuid, gateway_id);
update the schema by adding a UNIQUE constraint on the parent table api_keys for
the column pair (uuid, gateway_id) (or alternatively change the FK to reference
only uuid if that matches your model) so the referenced columns are either the
PRIMARY KEY or a UNIQUE key; modify the api_keys table definition (where UNIQUE
(gateway_id, artifact_uuid, name), UNIQUE (gateway_id, artifact_uuid, api_key),
PRIMARY KEY (uuid) are defined) to include UNIQUE(uuid, gateway_id) to satisfy
SQLite FK requirements.
Purpose
The gateway-controller SQLite schema had several index-related issues:
gateway_idas the leading column. Since the DB is shared across multiple gateways, all queries scope bygateway_idfirst.sql_store.go).Approach
Removed indexes (redundant — covered by UNIQUE constraint implicit indexes)
idx_artifacts_gateway_id(gateway_id)artifactsUNIQUE(gateway_id, kind, handle)prefixidx_artifacts_kind(kind)artifactsUNIQUE(gateway_id, kind, handle)prefixidx_cert_name(name)certificatesUNIQUE(gateway_id, name)prefixidx_certificates_gateway_id(gateway_id)certificatesUNIQUE(gateway_id, name)prefixidx_template_handle(handle)llm_provider_templatesUNIQUE(gateway_id, handle)prefixidx_llm_provider_templates_gateway_id(gateway_id)llm_provider_templatesUNIQUE(gateway_id, handle)prefixidx_api_key(api_key)api_keysUNIQUE(gateway_id, artifact_uuid, api_key)prefixidx_api_key_api(artifact_uuid)api_keysUNIQUE(gateway_id, artifact_uuid, name)prefixRemoved indexes (unused — column never appears in a WHERE clause)
idx_api_key_expiry(expires_at)api_keysidx_api_key_source(source)api_keysidx_api_key_external_ref(external_ref_id)api_keysUpdated indexes (added
gateway_idas leading column)idx_artifacts_status(status)idx_artifacts_gateway_id_status(gateway_id, status)idx_cert_expiry(not_after)idx_certificates_gateway_id_expiry(gateway_id, not_after)idx_api_key_status(status)idx_api_keys_gateway_id_status(gateway_id, status)idx_created_by(created_by)idx_api_keys_gateway_id_created_by(gateway_id, created_by)Updated UNIQUE constraints (
gateway_idmoved to leading column)certificatesUNIQUE(name, gateway_id)UNIQUE(gateway_id, name)llm_provider_templatesUNIQUE(handle, gateway_id)UNIQUE(gateway_id, handle)api_keysUNIQUE(artifact_uuid, name, gateway_id)UNIQUE(gateway_id, artifact_uuid, name)Added UNIQUE constraint
api_keysUNIQUE(gateway_id, artifact_uuid, api_key)PRIMARY KEY change
api_keysPRIMARY KEY(api_key, gateway_id)PRIMARY KEY(uuid)The old composite PK on
(api_key, gateway_id)was semantically wrong — the natural identifier for a key record is itsuuid. Uniqueness of the api key value is now enforced viaUNIQUE(gateway_id, artifact_uuid, api_key).Related Issues
N/A
Checklist
Security checks