Revert "fix: add missing indexes for top CPU-consuming queries (#23147)"#23402
Revert "fix: add missing indexes for top CPU-consuming queries (#23147)"#23402yuneng-jiang merged 1 commit intomainfrom
Conversation
This reverts commit 323b473.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR reverts commit Key changes:
Critical concern: The PR deletes the forward migration file but provides no new migration to drop the indexes from databases that already applied the original migration. Prisma tracks applied migrations in the Additionally, no explanation is provided for why the indexes are being reverted, nor is there any evidence (tests, benchmark data) that the original fix was harmful. Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| litellm-proxy-extras/litellm_proxy_extras/migrations/20260309115809_add_missing_indexes/migration.sql | Migration file deleted without a corresponding new migration to drop the indexes — leaves any database that already applied this migration in a schema-drift state. |
| litellm-proxy-extras/litellm_proxy_extras/schema.prisma | Removes @@index([key_alias]) from LiteLLM_VerificationToken and @@index([user, startTime]) from LiteLLM_SpendLogs; schema change is consistent with the deleted migration file but no drop migration accompanies it. |
| litellm/proxy/schema.prisma | Mirror of the schema.prisma change — removes the same two index definitions; no independent migration concern here as this file is not the migration source. |
| schema.prisma | Root schema.prisma mirror — removes the same two index definitions; consistent with the other schema files. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Original PR #23147\nAdds migration file\n+ schema index definitions] --> B[Migration Applied?\nDB has indexes\n_prisma_migrations records it]
A --> C[Migration NOT Applied\nDB has no indexes]
B --> D[This Revert PR #23402\nDeletes migration file\nRemoves schema index definitions\nNO drop migration added]
C --> D
D --> E{Was migration\nalready applied?}
E -- Yes --> F[❌ Schema Drift\nDB still has indexes\n_prisma_migrations still records it\nPrisma detects file-history mismatch]
E -- No --> G[✅ Clean state\nIndexes never created\nSchema consistent]
F --> H[Fix: Add new migration\nDROP INDEX CONCURRENTLY IF EXISTS\nLiteLLM_VerificationToken_key_alias_idx\nLiteLLM_SpendLogs_user_startTime_idx]
Last reviewed commit: 64ed29d
| @@index([startTime, request_id]) | ||
| @@index([end_user]) | ||
| @@index([session_id]) | ||
|
|
||
| // SELECT ... FROM "LiteLLM_SpendLogs" WHERE ("startTime" >= $1 AND "startTime" <= $2 AND "user" = $3) GROUP BY ... | ||
| @@index([user, startTime]) | ||
| } | ||
|
|
||
| // View spend, model, api_key per request |
There was a problem hiding this comment.
Missing revert migration to drop indexes
This PR deletes the migration file 20260309115809_add_missing_indexes/migration.sql and removes the index definitions from the schema, but does not add a new migration to DROP those indexes from databases where the original migration was already applied.
For any deployment that ran the original migration, Prisma's _prisma_migrations table will still record 20260309115809_add_missing_indexes as applied, but the file is now gone. This creates an irrecoverable migration state drift: Prisma will detect that the applied migration history no longer matches the migration files on disk and will refuse to run prisma migrate deploy / prisma migrate status correctly.
A proper revert should include a new migration file, e.g. 20260312_revert_missing_indexes/migration.sql, containing:
-- SkipTransactionBlock
-- Drop index added by 20260309115809_add_missing_indexes
DROP INDEX CONCURRENTLY IF EXISTS "LiteLLM_VerificationToken_key_alias_idx";
-- Drop index added by 20260309115809_add_missing_indexes
DROP INDEX CONCURRENTLY IF EXISTS "LiteLLM_SpendLogs_user_startTime_idx";Without this, any existing deployment is left in an inconsistent state. The same concern applies to litellm/proxy/schema.prisma and the root schema.prisma.
Revert "fix: add missing indexes for top CPU-consuming queries (#23147)"
This reverts commit 323b473.
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🚄 Infrastructure
Changes
This is incompatible with our current workflow for adjusting the schema
