diff --git a/api/server/services/Files/process.js b/api/server/services/Files/process.js index c4e7d307b3..8272e0ecfe 100644 --- a/api/server/services/Files/process.js +++ b/api/server/services/Files/process.js @@ -349,6 +349,7 @@ async function sweepExpiredFiles(options = {}) { return sweepExpiredFilesWithDeps(options, { getExpiredFiles: db.getExpiredFiles, processDeleteRequest, + deferExpiredFile: db.deferExpiredFile, logger, }); } diff --git a/api/server/services/Files/process.spec.js b/api/server/services/Files/process.spec.js index 14eaae5941..4cf45e3b57 100644 --- a/api/server/services/Files/process.spec.js +++ b/api/server/services/Files/process.spec.js @@ -172,6 +172,7 @@ jest.mock('~/models', () => ({ findFileById: jest.fn(), getConvo: jest.fn(), getExpiredFiles: jest.fn(), + deferExpiredFile: jest.fn(), addAgentResourceFile: jest.fn().mockResolvedValue({}), removeAgentResourceFiles: jest.fn(), removeAgentResourceFilesFromAllAgents: jest.fn(), @@ -1840,6 +1841,7 @@ describe('sweepExpiredFiles', () => { expect.objectContaining({ getExpiredFiles: db.getExpiredFiles, processDeleteRequest: expect.any(Function), + deferExpiredFile: db.deferExpiredFile, logger: expect.objectContaining({ error: expect.any(Function), info: expect.any(Function), diff --git a/packages/api/src/files/sweep.spec.ts b/packages/api/src/files/sweep.spec.ts index 4c5465f97e..0718c633f0 100644 --- a/packages/api/src/files/sweep.spec.ts +++ b/packages/api/src/files/sweep.spec.ts @@ -1,6 +1,12 @@ import { EModelEndpoint, FileSources } from 'librechat-data-provider'; import type { AppConfig } from '@librechat/data-schemas'; -import { getFileRetentionSweepInterval, startExpiredFileSweep, sweepExpiredFiles } from './sweep'; +import { + sweepExpiredFiles, + startExpiredFileSweep, + getExpiredFileRetryDelay, + getFileRetentionMaxAttempts, + getFileRetentionSweepInterval, +} from './sweep'; describe('expired file sweep helpers', () => { const logger = { @@ -9,14 +15,19 @@ describe('expired file sweep helpers', () => { error: jest.fn(), }; + let deferExpiredFile: jest.Mock; + beforeEach(() => { jest.clearAllMocks(); + deferExpiredFile = jest.fn().mockResolvedValue(undefined); delete process.env.FILE_RETENTION_SWEEP_INTERVAL_MS; + delete process.env.FILE_RETENTION_SWEEP_MAX_ATTEMPTS; }); afterEach(() => { jest.useRealTimers(); delete process.env.FILE_RETENTION_SWEEP_INTERVAL_MS; + delete process.env.FILE_RETENTION_SWEEP_MAX_ATTEMPTS; }); it('loads endpoint config and deletes expired OpenAI storage files', async () => { @@ -40,9 +51,11 @@ describe('expired file sweep helpers', () => { const result = await sweepExpiredFiles( { appConfig: {} as AppConfig, loadAppConfig, limit: 1 }, - { getExpiredFiles, processDeleteRequest, logger }, + { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }, ); + expect(getExpiredFiles).toHaveBeenCalledWith(1, { maxAttempts: 10 }); + expect(deferExpiredFile).not.toHaveBeenCalled(); expect(loadAppConfig).toHaveBeenCalledTimes(1); expect(processDeleteRequest).toHaveBeenCalledWith({ req: expect.objectContaining({ @@ -62,16 +75,107 @@ describe('expired file sweep helpers', () => { const result = await sweepExpiredFiles( { appConfig: {} as AppConfig, limit: 1 }, - { getExpiredFiles, processDeleteRequest, logger }, + { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }, ); expect(processDeleteRequest).not.toHaveBeenCalled(); + expect(deferExpiredFile).toHaveBeenCalledWith('orphaned-file', expect.any(Date)); expect(logger.warn).toHaveBeenCalledWith( '[sweepExpiredFiles] Skipping expired file without user: orphaned-file', ); expect(result).toEqual({ scanned: 1, deleted: 0, failed: 1 }); }); + it.each([ + ['a rejected delete', { failedFileIds: ['stuck-file'], deletedFileIds: [] }], + ['a delete that resolves nothing', { failedFileIds: [], deletedFileIds: [] }], + ])('defers the file after %s', async (_case, outcome) => { + const getExpiredFiles = jest + .fn() + .mockResolvedValue([{ file_id: 'stuck-file', user: 'user-1', deletionAttempts: 2 }]); + const processDeleteRequest = jest.fn().mockResolvedValue(outcome); + + const result = await sweepExpiredFiles( + { appConfig: {} as AppConfig, limit: 1 }, + { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }, + ); + + const [fileId, retryAt] = deferExpiredFile.mock.calls[0]; + expect(fileId).toBe('stuck-file'); + /* Third consecutive failure, so the backoff has doubled twice. */ + expect(retryAt.getTime() - Date.now()).toBeCloseTo(4 * 60 * 60 * 1000, -3); + expect(result).toEqual({ scanned: 1, deleted: 0, failed: 1 }); + }); + + it('defers the file when the delete throws', async () => { + const getExpiredFiles = jest + .fn() + .mockResolvedValue([{ file_id: 'stuck-file', user: 'user-1' }]); + const processDeleteRequest = jest.fn().mockRejectedValue(new Error('storage unreachable')); + + const result = await sweepExpiredFiles( + { appConfig: {} as AppConfig, limit: 1 }, + { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }, + ); + + expect(deferExpiredFile).toHaveBeenCalledWith('stuck-file', expect.any(Date)); + expect(result).toEqual({ scanned: 1, deleted: 0, failed: 1 }); + }); + + it('reports giving up once the file reaches the attempt cap', async () => { + process.env.FILE_RETENTION_SWEEP_MAX_ATTEMPTS = '3'; + const getExpiredFiles = jest + .fn() + .mockResolvedValue([{ file_id: 'stranded-file', user: 'user-1', deletionAttempts: 2 }]); + const processDeleteRequest = jest + .fn() + .mockResolvedValue({ deletedFileIds: [], failedFileIds: ['stranded-file'] }); + + await sweepExpiredFiles( + { appConfig: {} as AppConfig, limit: 1 }, + { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }, + ); + + expect(getExpiredFiles).toHaveBeenCalledWith(1, { maxAttempts: 3 }); + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('Giving up on expired file stranded-file after 3 failed deletions'), + ); + }); + + it('keeps sweeping the batch when recording a failure fails', async () => { + const getExpiredFiles = jest.fn().mockResolvedValue([ + { file_id: 'stuck-file', user: 'user-1' }, + { file_id: 'deletable-file', user: 'user-1' }, + ]); + const processDeleteRequest = jest + .fn() + .mockResolvedValueOnce({ deletedFileIds: [], failedFileIds: ['stuck-file'] }) + .mockResolvedValueOnce({ deletedFileIds: ['deletable-file'], failedFileIds: [] }); + deferExpiredFile.mockRejectedValue(new Error('mongo unreachable')); + + const result = await sweepExpiredFiles( + { appConfig: {} as AppConfig, limit: 2 }, + { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }, + ); + + expect(logger.error).toHaveBeenCalledWith( + '[sweepExpiredFiles] Error recording failed deletion of expired file stuck-file:', + expect.any(Error), + ); + expect(result).toEqual({ scanned: 2, deleted: 1, failed: 1 }); + }); + + it('caps the retry delay at a day and ignores unusable attempt limits', () => { + expect(getExpiredFileRetryDelay(1)).toBe(60 * 60 * 1000); + expect(getExpiredFileRetryDelay(4)).toBe(8 * 60 * 60 * 1000); + expect(getExpiredFileRetryDelay(64)).toBe(24 * 60 * 60 * 1000); + + expect(getFileRetentionMaxAttempts('0')).toBe(10); + expect(getFileRetentionMaxAttempts('2.5')).toBe(10); + expect(getFileRetentionMaxAttempts(' ')).toBe(10); + expect(getFileRetentionMaxAttempts('25')).toBe(25); + }); + it('falls back to the default interval for sub-millisecond values', () => { expect(getFileRetentionSweepInterval('0.5')).toBe(60 * 60 * 1000); }); diff --git a/packages/api/src/files/sweep.ts b/packages/api/src/files/sweep.ts index 4128fe3aa4..d3f7485de7 100644 --- a/packages/api/src/files/sweep.ts +++ b/packages/api/src/files/sweep.ts @@ -7,12 +7,16 @@ import { import type { AppConfig } from '@librechat/data-schemas'; const DEFAULT_FILE_RETENTION_SWEEP_INTERVAL_MS = 60 * 60 * 1000; +const DEFAULT_FILE_RETENTION_MAX_ATTEMPTS = 10; +const FILE_RETENTION_RETRY_BASE_MS = 60 * 60 * 1000; +const FILE_RETENTION_RETRY_MAX_MS = 24 * 60 * 60 * 1000; type ExpiredFile = { file_id: string; source?: string; user?: string | { toString?: () => string }; tenantId?: string; + deletionAttempts?: number; }; type SweepRequest = { @@ -46,11 +50,15 @@ type VersionedEndpointConfig = { }; type SweepDependencies = { - getExpiredFiles: (limit: number) => Promise; + getExpiredFiles: ( + limit: number, + options: { maxAttempts: number }, + ) => Promise; processDeleteRequest: (params: { req: SweepRequest; files: ExpiredFile[]; }) => Promise<{ deletedFileIds: string[]; failedFileIds: string[] }>; + deferExpiredFile: (file_id: string, deletionRetryAt: Date) => Promise; logger: SweepLogger; }; @@ -86,6 +94,37 @@ export function getFileRetentionSweepInterval( return value; } +/** + * Consecutive failures after which the sweep stops retrying a file. + * + * The record is kept so the reference survives for reconciliation, and + * raising this limit re-admits everything previously given up on — that is + * the supported way to resume once the underlying storage failure is fixed. + */ +export function getFileRetentionMaxAttempts( + maxAttempts: string | undefined = process.env.FILE_RETENTION_SWEEP_MAX_ATTEMPTS, +): number { + if (maxAttempts == null || maxAttempts.trim() === '') { + return DEFAULT_FILE_RETENTION_MAX_ATTEMPTS; + } + + const value = Number(maxAttempts); + if (!Number.isInteger(value) || value < 1) { + return DEFAULT_FILE_RETENTION_MAX_ATTEMPTS; + } + return value; +} + +/** + * Backoff before the next attempt, doubling from one sweep interval up to a + * day. `attempts` is the file's failure count including the one just + * recorded, so the first retry lands on the following sweep. + */ +export function getExpiredFileRetryDelay(attempts: number): number { + const exponent = Math.max(0, attempts - 1); + return Math.min(FILE_RETENTION_RETRY_BASE_MS * 2 ** exponent, FILE_RETENTION_RETRY_MAX_MS); +} + export function getExpiredFileEndpoint(source?: string): string { return source === FileSources.azure ? EModelEndpoint.azureAssistants : EModelEndpoint.assistants; } @@ -205,18 +244,45 @@ export async function resolveExpiredFileSweepConfig({ export async function sweepExpiredFiles( { appConfig, limit = 100, loadAppConfig }: ExpiredFileSweepOptions | undefined = {}, - { getExpiredFiles, processDeleteRequest, logger }: SweepDependencies, + { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }: SweepDependencies, ): Promise { - const files = (await getExpiredFiles(limit)) ?? []; + const maxAttempts = getFileRetentionMaxAttempts(); + const files = (await getExpiredFiles(limit, { maxAttempts })) ?? []; let resolvedAppConfig = appConfig; let deleted = 0; let failed = 0; + /* Every failure has to be recorded, not just counted: an unrecorded one + * leaves the file at the head of the next `expiredAt`-ordered batch, and + * a file that can never be deleted then crowds out every file that + * expired after it for the life of the deployment. */ + const recordFailure = async (file: ExpiredFile): Promise => { + failed++; + const attempts = (file.deletionAttempts ?? 0) + 1; + const retryAt = new Date(Date.now() + getExpiredFileRetryDelay(attempts)); + try { + await deferExpiredFile(file.file_id, retryAt); + } catch (error) { + logger.error( + `[sweepExpiredFiles] Error recording failed deletion of expired file ${file.file_id}:`, + error, + ); + return; + } + + if (attempts < maxAttempts) { + return; + } + logger.error( + `[sweepExpiredFiles] Giving up on expired file ${file.file_id} after ${attempts} failed deletions. Its backing storage was not removed; the record is kept so the reference survives for reconciliation. Raise FILE_RETENTION_SWEEP_MAX_ATTEMPTS to retry it.`, + ); + }; + for (const file of files) { const userId = typeof file.user === 'string' ? file.user : file.user?.toString?.(); if (!userId) { logger.warn(`[sweepExpiredFiles] Skipping expired file without user: ${file.file_id}`); - failed++; + await recordFailure(file); continue; } @@ -229,21 +295,21 @@ export async function sweepExpiredFiles( const req = createExpiredFileSweepRequest({ appConfig: resolvedAppConfig, file, userId }); const { deletedFileIds, failedFileIds } = await processDeleteRequest({ req, files: [file] }); if (failedFileIds.includes(file.file_id)) { - failed++; + await recordFailure(file); continue; } if (deletedFileIds.includes(file.file_id)) { deleted++; } else { - failed++; logger.error( `[sweepExpiredFiles] Delete request finished without resolving expired file ${file.file_id}`, ); + await recordFailure(file); } } catch (error) { - failed++; logger.error(`[sweepExpiredFiles] Error deleting expired file ${file.file_id}:`, error); + await recordFailure(file); } } diff --git a/packages/data-schemas/src/methods/file.spec.ts b/packages/data-schemas/src/methods/file.spec.ts index d4235b03fd..ef19ea43c9 100644 --- a/packages/data-schemas/src/methods/file.spec.ts +++ b/packages/data-schemas/src/methods/file.spec.ts @@ -380,10 +380,122 @@ describe('File Methods', () => { true, ); - const files = await fileMethods.getExpiredFiles(100, now); + const files = await fileMethods.getExpiredFiles(100, { now }); expect(files.map((file) => file.file_id)).toEqual([expiredFileId]); }); + + it('holds back files whose deletion backoff has not elapsed', async () => { + const userId = new mongoose.Types.ObjectId(); + const now = new Date('2030-01-01T00:00:00.000Z'); + const readyFileId = uuidv4(); + const backedOffFileId = uuidv4(); + + await fileMethods.createFile( + { + file_id: backedOffFileId, + user: userId, + filename: 'backed-off.txt', + filepath: '/uploads/backed-off.txt', + type: 'text/plain', + bytes: 100, + expiredAt: new Date('2029-12-01T00:00:00.000Z'), + deletionAttempts: 2, + deletionRetryAt: new Date('2030-01-01T00:00:01.000Z'), + }, + true, + ); + await fileMethods.createFile( + { + file_id: readyFileId, + user: userId, + filename: 'ready.txt', + filepath: '/uploads/ready.txt', + type: 'text/plain', + bytes: 100, + expiredAt: new Date('2029-12-31T00:00:00.000Z'), + deletionAttempts: 2, + deletionRetryAt: new Date('2029-12-31T23:00:00.000Z'), + }, + true, + ); + + const files = await fileMethods.getExpiredFiles(100, { now }); + + expect(files.map((file) => file.file_id)).toEqual([readyFileId]); + }); + + it('drops files that reached the attempt cap so they cannot starve the batch', async () => { + const userId = new mongoose.Types.ObjectId(); + const now = new Date('2030-01-01T00:00:00.000Z'); + const exhaustedFileId = uuidv4(); + const retriableFileId = uuidv4(); + + /* The exhausted file expired first, so without the cap it sorts to + * the front of every batch and a limit-sized run of them starves + * everything that expired afterwards. */ + await fileMethods.createFile( + { + file_id: exhaustedFileId, + user: userId, + filename: 'exhausted.txt', + filepath: '/uploads/exhausted.txt', + type: 'text/plain', + bytes: 100, + expiredAt: new Date('2029-01-01T00:00:00.000Z'), + deletionAttempts: 10, + deletionRetryAt: new Date('2029-06-01T00:00:00.000Z'), + }, + true, + ); + await fileMethods.createFile( + { + file_id: retriableFileId, + user: userId, + filename: 'retriable.txt', + filepath: '/uploads/retriable.txt', + type: 'text/plain', + bytes: 100, + expiredAt: new Date('2029-12-31T00:00:00.000Z'), + deletionAttempts: 9, + }, + true, + ); + + const capped = await fileMethods.getExpiredFiles(100, { now, maxAttempts: 10 }); + expect(capped.map((file) => file.file_id)).toEqual([retriableFileId]); + + const raised = await fileMethods.getExpiredFiles(100, { now, maxAttempts: 20 }); + expect(raised.map((file) => file.file_id)).toEqual([exhaustedFileId, retriableFileId]); + }); + }); + + describe('deferExpiredFile', () => { + it('increments the attempt count and stamps the next retry', async () => { + const userId = new mongoose.Types.ObjectId(); + const fileId = uuidv4(); + const retryAt = new Date('2030-02-01T00:00:00.000Z'); + + await fileMethods.createFile( + { + file_id: fileId, + user: userId, + filename: 'deferred.txt', + filepath: '/uploads/deferred.txt', + type: 'text/plain', + bytes: 100, + expiredAt: new Date('2029-12-31T00:00:00.000Z'), + }, + true, + ); + + await fileMethods.deferExpiredFile(fileId, retryAt); + await fileMethods.deferExpiredFile(fileId, retryAt); + + const [file] = (await fileMethods.getFiles({ file_id: fileId }))!; + expect(file.deletionAttempts).toBe(2); + expect(file.deletionRetryAt).toEqual(retryAt); + }); }); describe('getToolFilesByIds', () => { diff --git a/packages/data-schemas/src/methods/file.ts b/packages/data-schemas/src/methods/file.ts index b145ea65f9..19ffc1ca93 100644 --- a/packages/data-schemas/src/methods/file.ts +++ b/packages/data-schemas/src/methods/file.ts @@ -9,6 +9,12 @@ export type FileOwnerScope = { tenantId?: string | null; }; +export type ExpiredFileQueryOptions = { + now?: Date; + /** Consecutive-failure count at which a file stops being retried. */ + maxAttempts?: number; +}; + function withOwnerScope>( filter: T, ownerScope?: FileOwnerScope, @@ -35,7 +41,8 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { _sortOptions?: Record | null, selectFields?: Record | string | null, ) => Promise; - getExpiredFiles: (limit?: number, now?: Date) => Promise; + getExpiredFiles: (limit?: number, options?: ExpiredFileQueryOptions) => Promise; + deferExpiredFile: (file_id: string, deletionRetryAt: Date) => Promise; getToolFilesByIds: ( fileIds: string[], toolResourceSet?: Set, @@ -130,14 +137,51 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { return await query.sort(sortOptions).lean(); } - async function getExpiredFiles(limit = 100, now: Date = new Date()): Promise { + /** + * Expired files the sweep may attempt right now, oldest deadline first. + * + * Files whose storage deletion keeps failing are held back twice over: + * `deletionRetryAt` defers them until their backoff elapses, and + * `maxAttempts` drops them entirely once they have failed that many + * times. Without both, a permanently undeletable file sorts to the front + * of this bounded batch on every pass and starves every file that + * expired after it. Absent fields mean "never attempted", so records + * written before these fields existed remain eligible. + */ + async function getExpiredFiles( + limit = 100, + { now = new Date(), maxAttempts }: ExpiredFileQueryOptions = {}, + ): Promise { const File = mongoose.models.File as Model; - return await File.find({ expiredAt: { $ne: null, $lte: now } }) + const eligible: FilterQuery[] = [ + { $or: [{ deletionRetryAt: null }, { deletionRetryAt: { $lte: now } }] }, + ]; + if (maxAttempts != null) { + eligible.push({ + $or: [{ deletionAttempts: null }, { deletionAttempts: { $lt: maxAttempts } }], + }); + } + + return await File.find({ expiredAt: { $ne: null, $lte: now }, $and: eligible }) .sort({ expiredAt: 1 }) .limit(limit) .lean(); } + /** + * Records a failed sweep deletion and holds the file back until + * `deletionRetryAt`. The counter is incremented server-side so + * concurrent sweeps on separate nodes cannot overwrite each other's + * progress toward the give-up cap. + */ + async function deferExpiredFile(file_id: string, deletionRetryAt: Date): Promise { + const File = mongoose.models.File as Model; + await File.updateOne( + { file_id }, + { $inc: { deletionAttempts: 1 }, $set: { deletionRetryAt } }, + ).exec(); + } + /** * Retrieves tool files (files that are embedded or have a fileIdentifier) from an array of file IDs. * Note: execute_code files are handled separately by getCodeGeneratedFiles. @@ -679,6 +723,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { findFileById, getFiles, getExpiredFiles, + deferExpiredFile, getToolFilesByIds, getCodeGeneratedFiles, getUserCodeFiles, diff --git a/packages/data-schemas/src/schema/file.ts b/packages/data-schemas/src/schema/file.ts index 208ad67bdf..96668c43d3 100644 --- a/packages/data-schemas/src/schema/file.ts +++ b/packages/data-schemas/src/schema/file.ts @@ -152,6 +152,17 @@ const file: Schema = new Schema( * backing storage first, then removes this metadata record. */ type: Date, }, + deletionAttempts: { + /* Consecutive failed sweep deletions. Files at or past + * `FILE_RETENTION_SWEEP_MAX_ATTEMPTS` are excluded from the sweep + * query so a permanently undeletable file cannot starve the bounded + * batch; raising that limit re-admits them. */ + type: Number, + }, + deletionRetryAt: { + /* Earliest next sweep attempt, backed off from `deletionAttempts`. */ + type: Date, + }, }, { timestamps: true, diff --git a/packages/data-schemas/src/types/file.ts b/packages/data-schemas/src/types/file.ts index 8038512144..2a858e93bd 100644 --- a/packages/data-schemas/src/types/file.ts +++ b/packages/data-schemas/src/types/file.ts @@ -76,6 +76,18 @@ export interface IMongoFile extends Omit { }; expiresAt?: Date; expiredAt?: Date | null; + /** + * Consecutive failed retention-sweep deletions. The sweep backs off + * between attempts and stops retrying once this reaches the configured + * cap, so a file whose backing storage refuses deletion cannot occupy + * the sweep's bounded queue forever. Absent until the first failure. + */ + deletionAttempts?: number; + /** + * Earliest time the retention sweep may retry this file. Set alongside + * `deletionAttempts` on every failure; absent until the first one. + */ + deletionRetryAt?: Date; createdAt?: Date; updatedAt?: Date; tenantId?: string;