fix: prevent unnecessary reindexing after system restart #7409
Conversation
- Add retry logic for Qdrant connection on initialization - Distinguish between connection failures and missing collections - Only clear cache when a new collection is actually created - Add comprehensive logging to help diagnose connection issues This fixes the issue where the vector database index was being recreated after every system restart, even when the collection already existed in the Docker container. Fixes #7408
dosubot
bot
added
size:L
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| let created = false | ||
| try { | ||
| // Add initial connection test with retry | ||
| await this.testConnection() |
There was a problem hiding this comment.
Is this intentional? The testConnection() call here might succeed, but getCollectionInfo() could still fail if Qdrant becomes unavailable between these calls. Could we combine these operations or add a comment explaining why this edge case is acceptable?
|
|
||
| // If we get here, all retries failed | ||
| throw new Error( | ||
| t("embeddings:vectorStore.qdrantConnectionFailed", { |
There was a problem hiding this comment.
The error message uses the same i18n key embeddings:vectorStore.qdrantConnectionFailed for both connection test failures and initialization failures. Would it be clearer to use a more specific key like embeddings:vectorStore.qdrantConnectionTestFailed here?
|
|
||
| private async getCollectionInfo(): Promise<Schemas["CollectionInfo"] | null> { | ||
| private async getCollectionInfo(retryCount: number = 0): Promise<Schemas["CollectionInfo"] | null> { | ||
| const maxRetries = 3 |
There was a problem hiding this comment.
The retry configuration (3 retries, exponential backoff) is hardcoded. Have we considered making these values configurable through settings for different deployment scenarios? Some users might need more aggressive retries for flaky networks.
| * Checks if an error message indicates a connection failure | ||
| */ | ||
| private isConnectionError(errorMessage: string): boolean { | ||
| const connectionErrorPatterns = [ |
There was a problem hiding this comment.
This connection error pattern list is comprehensive, but might miss cloud-specific errors like 'Service Unavailable' or rate limiting errors. Should we expand this list to cover more edge cases?
| const connectionErrorPatterns = [ | |
| const connectionErrorPatterns = [ | |
| "ECONNREFUSED", | |
| "ETIMEDOUT", | |
| "ENOTFOUND", | |
| "ENETUNREACH", | |
| "EHOSTUNREACH", | |
| "ECONNRESET", | |
| "fetch failed", | |
| "network", | |
| "connect", | |
| "service unavailable", | |
| "rate limit", | |
| "503", | |
| "429" | |
| ] |
| ) | ||
| await this.cacheManager.clearCacheFile() | ||
| } else { | ||
| console.log( |
There was a problem hiding this comment.
Great addition! The logging here clearly differentiates between new collection creation and reconnection scenarios, which will help with debugging. The cache preservation logic is exactly what was needed to fix the issue.
This PR fixes issue #7408 where the vector database index was being recreated after every system restart.
Problem
After a system restart, the extension incorrectly determined that the Qdrant collection didn't exist due to initial connection failures, causing a full reindex (3+ hours for large projects).
Solution
Testing
Impact
Eliminates unnecessary reindexing after system restarts, saving hours for users with large codebases.
Fixes #7408
Important
Fixes unnecessary reindexing by adding retry logic and better error handling for Qdrant connection in
qdrant-client.ts.getCollectionInfo()andtestConnection()inqdrant-client.ts.getCollectionInfo().orchestrator.ts.qdrant-client.tsandorchestrator.ts.qdrant-client.spec.ts.getCollectionsfor connection tests inqdrant-client.spec.ts.This description was created by
for d6a72da. You can customize this summary. It will automatically update as commits are pushed.