mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-08 22:21:23 +00:00
fix: Mitigate extension host crash from Qdrant batch deletes
This commit is contained in:
parent
fed603a927
commit
92a05d7be6
4 changed files with 253 additions and 30 deletions
|
|
@ -203,6 +203,8 @@ export class FileWatcher implements IFileWatcher {
|
|||
}
|
||||
} catch (error) {
|
||||
overallBatchError = error as Error
|
||||
// Log the full, potentially multi-line, aggregated error message
|
||||
console.error(`[FileWatcher] Failed to delete points for ${allPathsToClearFromDB.size} files:`, error)
|
||||
for (const path of pathsToExplicitlyDelete) {
|
||||
batchResults.push({ path, status: "error", error: error as Error })
|
||||
processedCountInBatch++
|
||||
|
|
|
|||
|
|
@ -309,11 +309,16 @@ export class DirectoryScanner implements IDirectoryScanner {
|
|||
`[DirectoryScanner] Failed to delete points for ${uniqueFilePaths.length} files before upsert in workspace ${scanWorkspace}:`,
|
||||
deleteError,
|
||||
)
|
||||
// Re-throw the error with workspace context
|
||||
throw new Error(
|
||||
`Failed to delete points for ${uniqueFilePaths.length} files. Workspace: ${scanWorkspace}. ${deleteError instanceof Error ? deleteError.message : String(deleteError)}`,
|
||||
{ cause: deleteError },
|
||||
)
|
||||
// Log the error and call onError callback if it exists, but continue processing
|
||||
if (onError) {
|
||||
onError(
|
||||
new Error(
|
||||
`Failed to delete points for ${uniqueFilePaths.length} files. Workspace: ${scanWorkspace}. ${deleteError instanceof Error ? deleteError.message : String(deleteError)}`,
|
||||
{ cause: deleteError },
|
||||
),
|
||||
)
|
||||
}
|
||||
// Continue processing instead of throwing
|
||||
}
|
||||
}
|
||||
// --- End Deletion Step ---
|
||||
|
|
|
|||
|
|
@ -9,10 +9,15 @@ import { DEFAULT_MAX_SEARCH_RESULTS, DEFAULT_SEARCH_MIN_SCORE } from "../../cons
|
|||
vitest.mock("@qdrant/js-client-rest")
|
||||
vitest.mock("crypto")
|
||||
vitest.mock("../../../../utils/path")
|
||||
vitest.mock("path", () => ({
|
||||
...vitest.importActual("path"),
|
||||
sep: "/",
|
||||
}))
|
||||
vitest.mock("path", async () => {
|
||||
const actual = await vitest.importActual("path")
|
||||
return {
|
||||
...actual,
|
||||
sep: "/",
|
||||
resolve: vitest.fn((root, filePath) => `${root}/${filePath}`),
|
||||
normalize: vitest.fn((path) => path),
|
||||
}
|
||||
})
|
||||
|
||||
const mockQdrantClientInstance = {
|
||||
getCollection: vitest.fn(),
|
||||
|
|
@ -1291,4 +1296,155 @@ describe("QdrantVectorStore", () => {
|
|||
expect(callArgs.score_threshold).toBe(DEFAULT_SEARCH_MIN_SCORE)
|
||||
})
|
||||
})
|
||||
describe("deletePointsByMultipleFilePaths", () => {
|
||||
const CHUNK_SIZE = 100 // As defined in the implementation
|
||||
const MAX_RETRIES = 3 // As defined as MAX_BATCH_RETRIES in constants
|
||||
|
||||
beforeEach(() => {
|
||||
// Mock timers to control retry logic in tests
|
||||
vitest.useFakeTimers()
|
||||
vitest.spyOn(console, "warn").mockImplementation(() => {})
|
||||
vitest.spyOn(console, "error").mockImplementation(() => {})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vitest.useRealTimers()
|
||||
})
|
||||
|
||||
it("should handle an empty filePaths array gracefully", async () => {
|
||||
await vectorStore.deletePointsByMultipleFilePaths([])
|
||||
expect(mockQdrantClientInstance.delete).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should delete a small batch of file paths in a single call", async () => {
|
||||
const filePaths = ["src/file1.ts", "src/file2.ts"]
|
||||
mockQdrantClientInstance.delete.mockResolvedValue({} as any)
|
||||
|
||||
await vectorStore.deletePointsByMultipleFilePaths(filePaths)
|
||||
|
||||
expect(mockQdrantClientInstance.delete).toHaveBeenCalledTimes(1)
|
||||
const callArgs = mockQdrantClientInstance.delete.mock.calls[0][1]
|
||||
expect(callArgs.filter.should).toHaveLength(2)
|
||||
})
|
||||
|
||||
it("should split a large batch of file paths into multiple chunks", async () => {
|
||||
const filePaths = Array.from({ length: 250 }, (_, i) => `src/file${i + 1}.ts`)
|
||||
mockQdrantClientInstance.delete.mockResolvedValue({} as any)
|
||||
|
||||
await vectorStore.deletePointsByMultipleFilePaths(filePaths)
|
||||
|
||||
expect(mockQdrantClientInstance.delete).toHaveBeenCalledTimes(3) // 250 paths / 100 per chunk = 3 chunks
|
||||
expect(mockQdrantClientInstance.delete.mock.calls[0][1].filter.should).toHaveLength(CHUNK_SIZE)
|
||||
expect(mockQdrantClientInstance.delete.mock.calls[1][1].filter.should).toHaveLength(CHUNK_SIZE)
|
||||
expect(mockQdrantClientInstance.delete.mock.calls[2][1].filter.should).toHaveLength(50)
|
||||
})
|
||||
|
||||
it("should retry a failing chunk and succeed on the third attempt", async () => {
|
||||
const filePaths = ["src/fail-then-succeed.ts"]
|
||||
const deleteError = new Error("Qdrant unavailable")
|
||||
|
||||
mockQdrantClientInstance.delete
|
||||
.mockRejectedValueOnce(deleteError)
|
||||
.mockRejectedValueOnce(deleteError)
|
||||
.mockResolvedValue({} as any)
|
||||
|
||||
const deletePromise = vectorStore.deletePointsByMultipleFilePaths(filePaths)
|
||||
|
||||
// Advance timers for each retry (INITIAL_RETRY_DELAY_MS = 500)
|
||||
await vitest.advanceTimersByTimeAsync(500) // First retry delay (500ms)
|
||||
await vitest.advanceTimersByTimeAsync(1000) // Second retry delay (500 * 2 = 1000ms)
|
||||
|
||||
await deletePromise
|
||||
|
||||
expect(mockQdrantClientInstance.delete).toHaveBeenCalledTimes(3)
|
||||
expect(console.warn).toHaveBeenCalledTimes(2) // Warnings for the two failed attempts
|
||||
expect(console.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining("attempt 1/3. Retrying in 500ms..."),
|
||||
deleteError.message,
|
||||
)
|
||||
expect(console.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining("attempt 2/3. Retrying in 1000ms..."),
|
||||
deleteError.message,
|
||||
)
|
||||
})
|
||||
|
||||
it("should throw an aggregated error if one chunk fails after all retries", async () => {
|
||||
const filePaths = Array.from({ length: 150 }, (_, i) => `src/file${i + 1}.ts`)
|
||||
const deleteError = new Error("Persistent failure")
|
||||
|
||||
// First chunk fails consistently, second chunk succeeds
|
||||
mockQdrantClientInstance.delete
|
||||
.mockRejectedValueOnce(deleteError)
|
||||
.mockRejectedValueOnce(deleteError)
|
||||
.mockRejectedValueOnce(deleteError)
|
||||
.mockResolvedValue({} as any)
|
||||
|
||||
const deletePromise = vectorStore.deletePointsByMultipleFilePaths(filePaths)
|
||||
|
||||
// Advance timers for all retries of the first chunk (INITIAL_RETRY_DELAY_MS = 500)
|
||||
await vitest.advanceTimersByTimeAsync(500) // First retry delay (500ms)
|
||||
await vitest.advanceTimersByTimeAsync(1000) // Second retry delay (500 * 2 = 1000ms)
|
||||
await vitest.advanceTimersByTimeAsync(2000) // Third retry delay (500 * 4 = 2000ms)
|
||||
|
||||
await expect(deletePromise).rejects.toThrow(
|
||||
"Failed to delete 100 file paths across 1 chunks. Chunks failed: 1. First error: Persistent failure",
|
||||
)
|
||||
|
||||
expect(mockQdrantClientInstance.delete).toHaveBeenCalledTimes(MAX_RETRIES + 1) // 3 retries for chunk 1 + 1 success for chunk 2
|
||||
expect(console.error).toHaveBeenCalledTimes(1)
|
||||
expect(console.error).toHaveBeenCalledWith(
|
||||
`Failed to delete chunk 1 after ${MAX_RETRIES} attempts:`,
|
||||
deleteError.message,
|
||||
)
|
||||
|
||||
// Ensure all timers are run to completion
|
||||
await vitest.runAllTimersAsync()
|
||||
})
|
||||
|
||||
it("should throw an aggregated error if all chunks fail", async () => {
|
||||
const filePaths = Array.from({ length: 220 }, (_, i) => `src/file${i + 1}.ts`)
|
||||
const deleteError = new Error("Total cluster failure")
|
||||
|
||||
// All chunks fail
|
||||
mockQdrantClientInstance.delete.mockRejectedValue(deleteError)
|
||||
|
||||
const deletePromise = vectorStore.deletePointsByMultipleFilePaths(filePaths)
|
||||
|
||||
// Advance timers for all retries of all chunks (INITIAL_RETRY_DELAY_MS = 500)
|
||||
// Chunk 1 retries (3 attempts with exponential backoff)
|
||||
await vitest.advanceTimersByTimeAsync(500) // First retry delay (500ms)
|
||||
await vitest.advanceTimersByTimeAsync(1000) // Second retry delay (500 * 2 = 1000ms)
|
||||
await vitest.advanceTimersByTimeAsync(2000) // Third retry delay (500 * 4 = 2000ms)
|
||||
// Chunk 2 retries
|
||||
await vitest.advanceTimersByTimeAsync(500) // First retry delay (500ms)
|
||||
await vitest.advanceTimersByTimeAsync(1000) // Second retry delay (500 * 2 = 1000ms)
|
||||
await vitest.advanceTimersByTimeAsync(2000) // Third retry delay (500 * 4 = 2000ms)
|
||||
// Chunk 3 retries
|
||||
await vitest.advanceTimersByTimeAsync(500) // First retry delay (500ms)
|
||||
await vitest.advanceTimersByTimeAsync(1000) // Second retry delay (500 * 2 = 1000ms)
|
||||
await vitest.advanceTimersByTimeAsync(2000) // Third retry delay (500 * 4 = 2000ms)
|
||||
|
||||
await expect(deletePromise).rejects.toThrow(
|
||||
"Failed to delete 220 file paths across 3 chunks. Chunks failed: 1, 2, 3. First error: Total cluster failure",
|
||||
)
|
||||
|
||||
expect(mockQdrantClientInstance.delete).toHaveBeenCalledTimes(MAX_RETRIES * 3) // 3 chunks * 3 retries each
|
||||
expect(console.error).toHaveBeenCalledTimes(3)
|
||||
expect(console.error).toHaveBeenCalledWith(
|
||||
`Failed to delete chunk 1 after ${MAX_RETRIES} attempts:`,
|
||||
deleteError.message,
|
||||
)
|
||||
expect(console.error).toHaveBeenCalledWith(
|
||||
`Failed to delete chunk 2 after ${MAX_RETRIES} attempts:`,
|
||||
deleteError.message,
|
||||
)
|
||||
expect(console.error).toHaveBeenCalledWith(
|
||||
`Failed to delete chunk 3 after ${MAX_RETRIES} attempts:`,
|
||||
deleteError.message,
|
||||
)
|
||||
|
||||
// Ensure all timers are run to completion
|
||||
await vitest.runAllTimersAsync()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -4,7 +4,12 @@ import * as path from "path"
|
|||
import { getWorkspacePath } from "../../../utils/path"
|
||||
import { IVectorStore } from "../interfaces/vector-store"
|
||||
import { Payload, VectorStoreSearchResult } from "../interfaces"
|
||||
import { DEFAULT_MAX_SEARCH_RESULTS, DEFAULT_SEARCH_MIN_SCORE } from "../constants"
|
||||
import {
|
||||
DEFAULT_MAX_SEARCH_RESULTS,
|
||||
DEFAULT_SEARCH_MIN_SCORE,
|
||||
MAX_BATCH_RETRIES,
|
||||
INITIAL_RETRY_DELAY_MS,
|
||||
} from "../constants"
|
||||
import { t } from "../../../i18n"
|
||||
|
||||
/**
|
||||
|
|
@ -333,29 +338,84 @@ export class QdrantVectorStore implements IVectorStore {
|
|||
return
|
||||
}
|
||||
|
||||
try {
|
||||
const workspaceRoot = getWorkspacePath()
|
||||
const normalizedPaths = filePaths.map((filePath) => {
|
||||
const absolutePath = path.resolve(workspaceRoot, filePath)
|
||||
return path.normalize(absolutePath)
|
||||
})
|
||||
const CHUNK_SIZE = 100
|
||||
const workspaceRoot = getWorkspacePath()
|
||||
const normalizedPaths = filePaths.map((filePath) => {
|
||||
const absolutePath = path.resolve(workspaceRoot, filePath)
|
||||
return path.normalize(absolutePath)
|
||||
})
|
||||
|
||||
const filter = {
|
||||
should: normalizedPaths.map((normalizedPath) => ({
|
||||
key: "filePath",
|
||||
match: {
|
||||
value: normalizedPath,
|
||||
},
|
||||
})),
|
||||
const failedChunks: { chunkIndex: number; paths: string[]; error: Error }[] = []
|
||||
|
||||
// Process paths in chunks
|
||||
for (let i = 0; i < normalizedPaths.length; i += CHUNK_SIZE) {
|
||||
const chunk = normalizedPaths.slice(i, i + CHUNK_SIZE)
|
||||
const chunkIndex = Math.floor(i / CHUNK_SIZE)
|
||||
|
||||
// Retry logic with exponential backoff
|
||||
let retryCount = 0
|
||||
let lastError: Error | null = null
|
||||
|
||||
while (retryCount < MAX_BATCH_RETRIES) {
|
||||
try {
|
||||
const filter = {
|
||||
should: chunk.map((normalizedPath) => ({
|
||||
key: "filePath",
|
||||
match: {
|
||||
value: normalizedPath,
|
||||
},
|
||||
})),
|
||||
}
|
||||
|
||||
await this.client.delete(this.collectionName, {
|
||||
filter,
|
||||
wait: true,
|
||||
})
|
||||
|
||||
// Success - break out of retry loop
|
||||
break
|
||||
} catch (error) {
|
||||
lastError = error instanceof Error ? error : new Error(String(error))
|
||||
retryCount++
|
||||
|
||||
if (retryCount < MAX_BATCH_RETRIES) {
|
||||
// Calculate exponential backoff delay
|
||||
const delay = INITIAL_RETRY_DELAY_MS * Math.pow(2, retryCount - 1)
|
||||
console.warn(
|
||||
`Failed to delete chunk ${chunkIndex + 1} (paths ${i + 1}-${Math.min(i + CHUNK_SIZE, normalizedPaths.length)}), ` +
|
||||
`attempt ${retryCount}/${MAX_BATCH_RETRIES}. Retrying in ${delay}ms...`,
|
||||
lastError.message,
|
||||
)
|
||||
|
||||
// Wait before retrying
|
||||
await new Promise((resolve) => setTimeout(resolve, delay))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await this.client.delete(this.collectionName, {
|
||||
filter,
|
||||
wait: true,
|
||||
})
|
||||
} catch (error) {
|
||||
console.error("Failed to delete points by file paths:", error)
|
||||
throw error
|
||||
// If all retries failed, log the error and continue to next chunk
|
||||
if (retryCount === MAX_BATCH_RETRIES && lastError) {
|
||||
console.error(
|
||||
`Failed to delete chunk ${chunkIndex + 1} after ${MAX_BATCH_RETRIES} attempts:`,
|
||||
lastError.message,
|
||||
)
|
||||
failedChunks.push({
|
||||
chunkIndex,
|
||||
paths: chunk,
|
||||
error: lastError,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// If any chunks failed, throw an aggregated error
|
||||
if (failedChunks.length > 0) {
|
||||
const totalFailedPaths = failedChunks.reduce((sum, chunk) => sum + chunk.paths.length, 0)
|
||||
const errorMessage =
|
||||
`Failed to delete ${totalFailedPaths} file paths across ${failedChunks.length} chunks. ` +
|
||||
`Chunks failed: ${failedChunks.map((c) => c.chunkIndex + 1).join(", ")}. ` +
|
||||
`First error: ${failedChunks[0].error.message}`
|
||||
|
||||
throw new Error(errorMessage)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue