diff --git a/.changelog/adapter-aware-workers-and-schedules.md b/.changelog/adapter-aware-workers-and-schedules.md deleted file mode 100644 index 87790e3..0000000 --- a/.changelog/adapter-aware-workers-and-schedules.md +++ /dev/null @@ -1,68 +0,0 @@ -# Adapter-Aware Workers and Schedules - -## New Features - -### Select an Adapter per Worker - -Workers can now listen on a specific registered Adapter with `worker.adapter`. When omitted, the -worker continues to use the queue manager's default Adapter. - -```typescript -const config = { - default: 'redis', - adapters: { - redis: redis(redisConfig), - database: knex(databaseConfig), - }, - worker: { - adapter: 'database', - concurrency: 5, - }, -} - -const worker = new Worker(config) -await worker.start(['default', 'emails']) -``` - -This makes it possible to run separate workers for queues stored by different Adapters. - -### Store and Access Schedules on a Specific Adapter - -Schedules can now select their owning Adapter with `.with()`: - -```typescript -await CleanupJob.schedule({ days: 30 }).id('daily-cleanup').with('redis').cron('0 0 * * *') -``` - -`Schedule.find()` and `Schedule.list()` accept an Adapter selector when accessing schedules outside -the default Adapter: - -```typescript -const schedule = await Schedule.find('daily-cleanup', { adapter: 'redis' }) -const schedules = await Schedule.list({ status: 'active' }, { adapter: 'redis' }) -``` - -A returned `Schedule` retains the selected Adapter for subsequent `pause()`, `resume()`, `delete()`, -and `trigger()` calls. Jobs dispatched by a schedule stay on the Adapter that owns that schedule. - -### Identify Jobs Dispatched by a Schedule - -Scheduled jobs now include their originating schedule ID in `JobData.scheduleId`. Jobs can access it -while executing through `this.context.scheduleId`: - -```typescript -async execute() { - console.log(this.context.scheduleId) -} -``` - -The value is `undefined` for jobs that were not dispatched by a schedule. - -## Upgrade Notes - -Start a Worker for every Adapter that owns schedules. A Worker only claims schedules and jobs from -its configured Adapter. - -When a schedule does not call `.with()`, its Adapter is resolved from the job's `adapter` option, -then from the Adapter configured for the job's queue, and finally from the queue manager default. -An explicit `.with()` always takes precedence. diff --git a/.changelog/consistent-job-runtime.md b/.changelog/consistent-job-runtime.md deleted file mode 100644 index efdcf4f..0000000 --- a/.changelog/consistent-job-runtime.md +++ /dev/null @@ -1,30 +0,0 @@ -# Consistent Job Dispatch and Execution - -## Improvements - -All job dispatch paths now apply the same routing and job options. This includes `dispatch()`, -`dispatchMany()`, manual schedule triggers, and schedules claimed by workers. - -The routing order is now consistent across these paths: - -1. Fluent overrides such as `.toQueue()` and `.with()` -2. Static `Job.options` -3. The Adapter configured for the selected queue -4. The queue manager's default Adapter - -Queue, Adapter, priority, custom job name, creation timestamp, and schedule provenance are therefore -preserved consistently regardless of how a job is dispatched. Static job options are resolved when -the fluent builder runs, so changes made between builder creation and execution are applied. - -The Sync adapter and Worker execution paths now also share the same job lifecycle behavior, -including context construction, dependency injection through `jobFactory`, execution wrappers, -timeouts, retries, failed hooks, and tracing. - -## Upgrade Notes - -A job routed to a queue with `queues..adapter` now uses that Adapter when neither `.with()` nor -`Job.options.adapter` selects another one. Previously, some dispatch paths could incorrectly fall -back to the queue manager's default Adapter. Verify that a Worker is running for every Adapter used -by queue configuration. - -Explicit fluent options continue to take precedence over static job options. diff --git a/.changelog/hot-reloading-jobs.md b/.changelog/hot-reloading-jobs.md deleted file mode 100644 index d23c314..0000000 --- a/.changelog/hot-reloading-jobs.md +++ /dev/null @@ -1,44 +0,0 @@ -# Hot Reloading Jobs - -## New Feature - -Workers can now execute the latest saved version of a job without restarting during development. - -Enable `hotReload` when initializing the queue manager. Jobs discovered from `locations` will then -be resolved from their module again before every execution. - -```typescript -await QueueManager.init({ - default: 'redis', - adapters: { - redis: redis({ host: 'localhost', port: 6379 }), - }, - locations: ['./app/jobs/**/*.ts'], - hotReload: process.env.NODE_ENV === 'development', -}) -``` - -Hot reload integrates with [Hot Hook](https://github.com/Julien-R44/hot-hook). The queue provides -the dynamic import boundary, while the application remains responsible for installing and -initializing Hot Hook. AdonisJS applications can use `node ace serve --hmr`; standalone worker -processes must initialize Hot Hook themselves. - -`Locator.registerFromGlob()` also accepts the option directly: - -```typescript -await Locator.registerFromGlob(['./app/jobs/**/*.ts'], { hotReload: true }) -``` - -## Upgrade Notes - -Hot reload is disabled by default and should only be enabled in development. - -Only jobs discovered from `locations` or registered with `Locator.registerFromGlob()` can be -reloaded. Jobs registered manually with `Locator.register()` do not have a module path to reload. - -Changes to the set of registered jobs still require a restart. This includes adding, deleting, -moving, or renaming a job, as well as changing its configured `name`. A job that is already running -keeps its current implementation; the next execution receives the updated version. - -Avoid import-time side effects in hot-reloaded job modules, since their module code can execute -again after an invalidation. diff --git a/.changelog/redis-schedule-due-index.md b/.changelog/redis-schedule-due-index.md new file mode 100644 index 0000000..d661a83 --- /dev/null +++ b/.changelog/redis-schedule-due-index.md @@ -0,0 +1,34 @@ +# Indexed Redis Schedule Claims + +## Performance Improvement + +The Redis adapter now maintains a `schedules::due` sorted-set index scored by each schedule's +`next_run_at`. Claiming the next due schedule uses this index instead of scanning every stored +schedule, so polling no longer grows linearly with the total schedule count. + +Schedule hashes remain the source of truth. Creating, updating, pausing, resuming, deleting, and +claiming schedules maintain the derived index, while claiming repairs stale entries when the hash +and index disagree. + +Schedule hash and index writes are atomic, and index consistency is preserved across concurrent +lifecycle changes. Cron finalization now rejects stale calculations when a schedule is paused, +deleted, or reconfigured while its next run is being calculated. Claiming also discards malformed +due scores instead of allowing one corrupt entry to block later schedules. + +## Upgrade Notes + +This change requires an explicit migration for existing Redis schedules. The `Adapter` contract now +includes an idempotent `migrate()` method; built-in adapters without migrations implement it as a +no-op, and custom adapters must implement it as well. + +Run the migration once during deployment, before starting workers or any process that creates or +updates schedules: + +```typescript +await QueueManager.init(config) +await QueueManager.use('redis').migrate() +``` + +Existing Redis schedules will not fire through the new index until the migration has completed. +The migration scans all schedules and should remain an explicit deployment step rather than run in +the worker polling loop. diff --git a/README.md b/README.md index 1fbcaaa..9c0bc37 100644 --- a/README.md +++ b/README.md @@ -334,6 +334,26 @@ const connection = new Redis({ host: 'localhost' }) const adapter = redis(connection) ``` +#### Migrating Redis schedules after an upgrade + +The Redis adapter uses a `schedules::due` sorted-set index to find due schedules. When upgrading +from a version that predates this index, run the adapter migration once during deployment, before +starting any workers: + +```typescript +import { QueueManager, Worker } from '@boringnode/queue' + +await QueueManager.init(config) +await QueueManager.use('redis').migrate() + +const worker = new Worker(config) +await worker.start(['default']) +``` + +The migration is idempotent and rebuilds the derived index from the canonical schedule hashes. +Existing Redis schedules will not fire through the new index until it has run. Do not run the +`O(number of schedules)` migration from the worker polling loop. + ### Knex (PostgreSQL, MySQL, SQLite) ```typescript diff --git a/src/contracts/adapter.ts b/src/contracts/adapter.ts index 547ff4b..4b42751 100644 --- a/src/contracts/adapter.ts +++ b/src/contracts/adapter.ts @@ -223,6 +223,16 @@ export interface Adapter { */ destroy(): Promise + /** + * Run adapter-specific migrations needed after a major version upgrade. + * + * This method is idempotent — it is always safe to call multiple times. + * Adapters that have no pending migrations return immediately. + * + * Call this once during your deployment process before starting workers. + */ + migrate(): Promise + /** * Create or update a schedule. * diff --git a/src/drivers/fake_adapter.ts b/src/drivers/fake_adapter.ts index 50810bb..0e79e44 100644 --- a/src/drivers/fake_adapter.ts +++ b/src/drivers/fake_adapter.ts @@ -404,6 +404,10 @@ export class FakeAdapter implements Adapter { return Promise.resolve() } + migrate(): Promise { + return Promise.resolve() + } + async upsertSchedule(config: ScheduleConfig): Promise { const id = config.id ?? randomUUID() const existing = this.#schedules.get(id) diff --git a/src/drivers/knex_adapter.ts b/src/drivers/knex_adapter.ts index 6f4e56e..7ceaf2b 100644 --- a/src/drivers/knex_adapter.ts +++ b/src/drivers/knex_adapter.ts @@ -88,6 +88,8 @@ export class KnexAdapter implements Adapter { } } + async migrate(): Promise {} + async pop(): Promise { return this.popFrom('default') } diff --git a/src/drivers/kysely_adapter.ts b/src/drivers/kysely_adapter.ts index e09504c..7ddbe59 100644 --- a/src/drivers/kysely_adapter.ts +++ b/src/drivers/kysely_adapter.ts @@ -122,6 +122,8 @@ export class KyselyAdapter implements Adapter { // The Kysely instance is always owned by the application. } + async migrate(): Promise {} + async pop(): Promise { return this.popFrom('default') } diff --git a/src/drivers/redis_adapter.ts b/src/drivers/redis_adapter.ts index 3ab152e..0c20c85 100644 --- a/src/drivers/redis_adapter.ts +++ b/src/drivers/redis_adapter.ts @@ -16,8 +16,10 @@ import { resolveRetention } from '../utils.js' import { encodeRedisJobPayloadOverlay, hydrateRedisJob } from './redis_job_storage.js' import { ACQUIRE_JOB_SCRIPT, + BACKFILL_SCHEDULE_DUE_INDEX_SCRIPT, CLAIM_SCHEDULE_SCRIPT, FINALIZE_JOB_SCRIPT, + FINALIZE_CRON_SCHEDULE_SCRIPT, GET_JOB_SCRIPT, PUSH_DEDUP_JOB_SCRIPT, PUSH_DELAYED_JOB_SCRIPT, @@ -26,11 +28,14 @@ import { REMOVE_JOB_SCRIPT, RENEW_JOBS_SCRIPT, RETRY_JOB_SCRIPT, + UPDATE_SCHEDULE_SCRIPT, + UPSERT_SCHEDULE_SCRIPT, } from './redis_scripts.js' const redisKey = 'jobs' const schedulesKey = 'schedules' const schedulesIndexKey = 'schedules::index' +const schedulesDueKey = 'schedules::due' type RedisConfig = Redis | RedisOptions function isRedisConnection(config?: RedisConfig): config is Redis { @@ -70,7 +75,6 @@ export class RedisAdapter implements Adapter { readonly #connection: Redis readonly #ownsConnection: boolean #workerId: string = '' - constructor(connection: Redis, ownsConnection: boolean = false) { this.#connection = connection this.#ownsConnection = ownsConnection @@ -433,11 +437,6 @@ export class RedisAdapter implements Adapter { const id = config.id ?? randomUUID() const now = Date.now() const scheduleKey = `${schedulesKey}::${id}` - const [existingRunCount, existingCreatedAt] = await this.#connection.hmget( - scheduleKey, - 'run_count', - 'created_at' - ) const scheduleData: Record = { id, @@ -445,8 +444,6 @@ export class RedisAdapter implements Adapter { payload: JSON.stringify(config.payload), timezone: config.timezone, status: 'active', - run_count: existingRunCount ?? '0', - created_at: existingCreatedAt ?? now.toString(), } if (config.cronExpression !== undefined) scheduleData.cron_expression = config.cronExpression @@ -455,13 +452,16 @@ export class RedisAdapter implements Adapter { if (config.to !== undefined) scheduleData.to_date = config.to.getTime().toString() if (config.limit !== undefined) scheduleData.run_limit = config.limit.toString() - // Upsert schedule and clear stale optional fields from previous config. - await this.#connection - .multi() - .hdel(scheduleKey, 'cron_expression', 'every_ms', 'from_date', 'to_date', 'run_limit') - .hset(scheduleKey, scheduleData) - .sadd(schedulesIndexKey, id) - .exec() + await this.#connection.eval( + UPSERT_SCHEDULE_SCRIPT, + 3, + scheduleKey, + schedulesIndexKey, + schedulesDueKey, + id, + now.toString(), + JSON.stringify(scheduleData) + ) return id } @@ -537,14 +537,30 @@ export class RedisAdapter implements Adapter { } if (updates.runCount !== undefined) data.run_count = updates.runCount.toString() - if (Object.keys(data).length > 0) { - await this.#connection.hset(scheduleKey, data) - } + if (Object.keys(data).length === 0) return + + await this.#connection.eval( + UPDATE_SCHEDULE_SCRIPT, + 2, + scheduleKey, + schedulesDueKey, + id, + JSON.stringify(data) + ) } async deleteSchedule(id: string): Promise { const scheduleKey = `${schedulesKey}::${id}` - await this.#connection.multi().del(scheduleKey).srem(schedulesIndexKey, id).exec() + await this.#connection + .multi() + .del(scheduleKey) + .srem(schedulesIndexKey, id) + .zrem(schedulesDueKey, id) + .exec() + } + + async migrate(): Promise { + await this.backfillDueIndex() } async claimDueSchedule(): Promise { @@ -552,7 +568,7 @@ export class RedisAdapter implements Adapter { const result = await this.#connection.eval( CLAIM_SCHEDULE_SCRIPT, 2, - schedulesIndexKey, + schedulesDueKey, `${schedulesKey}::`, now.toString() ) @@ -574,7 +590,6 @@ export class RedisAdapter implements Adapter { }) const nextRun = cron.next().toDate().getTime() - // Check limits before updating const runCount = Number.parseInt(data.run_count || '0', 10) + 1 const runLimit = data.run_limit ? Number.parseInt(data.run_limit, 10) : null const toDate = data.to_date ? Number.parseInt(data.to_date, 10) : null @@ -587,9 +602,14 @@ export class RedisAdapter implements Adapter { newNextRunAt = '' } - await this.#connection.hset( + await this.#connection.eval( + FINALIZE_CRON_SCHEDULE_SCRIPT, + 2, `${schedulesKey}::${data.id}`, - 'next_run_at', + schedulesDueKey, + data.id, + data.cron_expression, + data.config_revision || '', newNextRunAt.toString() ) } @@ -597,6 +617,16 @@ export class RedisAdapter implements Adapter { return this.#hashToScheduleData(data) } + async backfillDueIndex(): Promise { + return (await this.#connection.eval( + BACKFILL_SCHEDULE_DUE_INDEX_SCRIPT, + 3, + schedulesIndexKey, + schedulesDueKey, + `${schedulesKey}::` + )) as number + } + #hashToScheduleData(data: Record): ScheduleData { return { id: data.id, diff --git a/src/drivers/redis_scripts.ts b/src/drivers/redis_scripts.ts index 330c79c..5af310d 100644 --- a/src/drivers/redis_scripts.ts +++ b/src/drivers/redis_scripts.ts @@ -491,41 +491,234 @@ ${REDIS_JOB_STORAGE_LUA} }) ` +const SCHEDULE_DUE_INDEX_LUA = ` + local function sync_schedule_due_index(schedule_key, due_key, id) + local status = redis.call('HGET', schedule_key, 'status') + local next_run_at = redis.call('HGET', schedule_key, 'next_run_at') + local score = next_run_at and tonumber(next_run_at) or nil + + if status == 'active' and score then + redis.call('ZADD', due_key, score, id) + else + redis.call('ZREM', due_key, id) + end + end +` + /** - * Lua script for atomically claiming a due schedule. - * Iterates the schedule index server-side and claims the first due schedule. - * Returns the schedule data if claimed, nil otherwise. + * Atomically upserts schedule configuration while preserving runtime fields + * and synchronizing the derived due index from the resulting hash. */ -export const CLAIM_SCHEDULE_SCRIPT = ` - local schedules_index_key = KEYS[1] - local schedule_key_prefix = KEYS[2] - local now = tonumber(ARGV[1]) +export const UPSERT_SCHEDULE_SCRIPT = ` + local schedule_key = KEYS[1] + local schedules_index_key = KEYS[2] + local due_key = KEYS[3] + local id = ARGV[1] + local now = ARGV[2] + local schedule = cjson.decode(ARGV[3]) + +${SCHEDULE_DUE_INDEX_LUA} + + local run_count = redis.call('HGET', schedule_key, 'run_count') or '0' + local created_at = redis.call('HGET', schedule_key, 'created_at') or now + local config_revision = tonumber(redis.call('HGET', schedule_key, 'config_revision') or '0') + 1 + + redis.call( + 'HDEL', + schedule_key, + 'cron_expression', + 'every_ms', + 'from_date', + 'to_date', + 'run_limit' + ) + + for field, value in pairs(schedule) do + redis.call('HSET', schedule_key, field, value) + end + + redis.call( + 'HSET', + schedule_key, + 'run_count', + run_count, + 'created_at', + created_at, + 'config_revision', + tostring(config_revision) + ) + redis.call('SADD', schedules_index_key, id) + sync_schedule_due_index(schedule_key, due_key, id) + + return id +` + +/** + * Atomically updates schedule runtime fields and synchronizes the derived due + * index from the resulting canonical hash. + */ +export const UPDATE_SCHEDULE_SCRIPT = ` + local schedule_key = KEYS[1] + local due_key = KEYS[2] + local id = ARGV[1] + local updates = cjson.decode(ARGV[2]) +${SCHEDULE_DUE_INDEX_LUA} + + if redis.call('EXISTS', schedule_key) == 0 then + redis.call('ZREM', due_key, id) + return 0 + end + + for field, value in pairs(updates) do + redis.call('HSET', schedule_key, field, value) + end + + sync_schedule_due_index(schedule_key, due_key, id) + + return 1 +` + +/** + * Finalizes the JS-calculated next run for a cron claim only when the same + * claimed occurrence still owns the canonical hash. + */ +export const FINALIZE_CRON_SCHEDULE_SCRIPT = ` + local schedule_key = KEYS[1] + local due_key = KEYS[2] + local id = ARGV[1] + local expected_cron_expression = ARGV[2] + local expected_config_revision = ARGV[3] + local next_run_at = ARGV[4] + +${SCHEDULE_DUE_INDEX_LUA} + + if redis.call('EXISTS', schedule_key) == 0 then + redis.call('ZREM', due_key, id) + return 0 + end + + local status = redis.call('HGET', schedule_key, 'status') + local current_next_run_at = redis.call('HGET', schedule_key, 'next_run_at') + local cron_expression = redis.call('HGET', schedule_key, 'cron_expression') + local config_revision = redis.call('HGET', schedule_key, 'config_revision') or '' + + if status ~= 'active' + or current_next_run_at ~= '' + or cron_expression ~= expected_cron_expression + or config_revision ~= expected_config_revision then + sync_schedule_due_index(schedule_key, due_key, id) + return 0 + end + + local run_count = tonumber(redis.call('HGET', schedule_key, 'run_count') or '0') + local run_limit = tonumber(redis.call('HGET', schedule_key, 'run_limit') or '') + local to_date = tonumber(redis.call('HGET', schedule_key, 'to_date') or '') + local next_score = tonumber(next_run_at) + + if (run_limit and run_count >= run_limit) + or (to_date and next_score and next_score > to_date) then + next_run_at = '' + end + + redis.call('HSET', schedule_key, 'next_run_at', next_run_at) + sync_schedule_due_index(schedule_key, due_key, id) + + return 1 +` + +/** + * Atomically rebuilds the derived due index from canonical schedule hashes. + * This is an explicit O(N) migration and blocks concurrent Redis commands + * until the complete index reflects one consistent point in time. + */ +export const BACKFILL_SCHEDULE_DUE_INDEX_SCRIPT = ` + local schedules_index_key = KEYS[1] + local due_key = KEYS[2] + local schedule_key_prefix = KEYS[3] local ids = redis.call('SMEMBERS', schedules_index_key) + local count = 0 + + redis.call('DEL', due_key) for i = 1, #ids do - local schedule_key = schedule_key_prefix .. ids[i] + local id = ids[i] + local schedule_key = schedule_key_prefix .. id + local status = redis.call('HGET', schedule_key, 'status') + local next_run_at = redis.call('HGET', schedule_key, 'next_run_at') + local score = next_run_at and tonumber(next_run_at) or nil + + if status == 'active' and score then + redis.call('ZADD', due_key, score, id) + count = count + 1 + end + end + + return count +` + +/** + * Lua script for atomically claiming a due schedule using a sorted set index. + * + * Uses ZRANGEBYSCORE on schedules::due (scored by next_run_at) for O(log N) + * lookup instead of scanning all schedule hashes via SMEMBERS. + * + * Stale entries (paused, exhausted, deleted) are cleaned from the ZSET on + * sight so subsequent calls skip them. + * + * KEYS[1] = schedules::due (the ZSET) + * KEYS[2] = schedule key prefix (e.g. "schedules::") + * ARGV[1] = now (epoch milliseconds) + */ +export const CLAIM_SCHEDULE_SCRIPT = ` + local due_key = KEYS[1] + local prefix = KEYS[2] + local now = tonumber(ARGV[1]) + + while true do + local candidates = redis.call('ZRANGEBYSCORE', due_key, '-inf', tostring(now), 'LIMIT', 0, 1) + + if #candidates == 0 then + return nil + end + + local id = candidates[1] + local schedule_key = prefix .. id -- Get schedule data local data = redis.call('HGETALL', schedule_key) - if #data > 0 then + + -- Deleted schedule still in ZSET + if #data == 0 then + redis.call('ZREM', due_key, id) + else -- Convert HGETALL result to table local schedule = {} for j = 1, #data, 2 do schedule[data[j]] = data[j + 1] end - -- Check if schedule is due - if schedule.status == 'active' then - local next_run_at = tonumber(schedule.next_run_at) - - if next_run_at and next_run_at <= now then + -- Check if schedule is active + if schedule.status ~= 'active' then + redis.call('ZREM', due_key, id) + else + -- Hash is the source of truth for next_run_at. + -- If the ZSET score is stale, repair it and skip this candidate. + local hash_nra = schedule.next_run_at + local hash_score = hash_nra and tonumber(hash_nra) or nil + if not hash_score then + redis.call('ZREM', due_key, id) + elseif hash_score > now then + redis.call('ZADD', due_key, hash_score, id) + else local run_count = tonumber(schedule.run_count or '0') local run_limit = schedule.run_limit and tonumber(schedule.run_limit) or nil local to_date = schedule.to_date and tonumber(schedule.to_date) or nil -- Check limits - if not (run_limit and run_count >= run_limit) and not (to_date and now > to_date) then + if (run_limit and run_count >= run_limit) or (to_date and now > to_date) then + redis.call('ZREM', due_key, id) + else -- This schedule is claimable - atomically update it local new_run_count = run_count + 1 @@ -553,6 +746,13 @@ export const CLAIM_SCHEDULE_SCRIPT = ` 'last_run_at', tostring(now), 'run_count', tostring(new_run_count)) + -- Update or remove from ZSET + if new_next_run_at ~= '' then + redis.call('ZADD', due_key, tonumber(new_next_run_at), id) + else + redis.call('ZREM', due_key, id) + end + -- Return the schedule data (before update) as JSON return cjson.encode(schedule) end @@ -560,6 +760,4 @@ export const CLAIM_SCHEDULE_SCRIPT = ` end end end - - return nil ` diff --git a/src/drivers/sync_adapter.ts b/src/drivers/sync_adapter.ts index 2d707f5..298353f 100644 --- a/src/drivers/sync_adapter.ts +++ b/src/drivers/sync_adapter.ts @@ -117,6 +117,10 @@ export class SyncAdapter implements Adapter { return Promise.resolve() } + migrate(): Promise { + return Promise.resolve() + } + upsertSchedule(_config: ScheduleConfig): Promise { // No-op: schedules don't make sense for sync adapter // Return a fake ID so code doesn't break in dev diff --git a/tests/_mocks/memory_adapter.ts b/tests/_mocks/memory_adapter.ts index e1de1bf..9d21cad 100644 --- a/tests/_mocks/memory_adapter.ts +++ b/tests/_mocks/memory_adapter.ts @@ -312,6 +312,10 @@ export class MemoryAdapter implements Adapter { return Promise.resolve() } + migrate(): Promise { + return Promise.resolve() + } + async upsertSchedule(config: ScheduleConfig): Promise { const id = config.id ?? randomUUID() const existing = this.#schedules.get(id) diff --git a/tests/_utils/register_driver_test_suite.ts b/tests/_utils/register_driver_test_suite.ts index a8ba74c..5080067 100644 --- a/tests/_utils/register_driver_test_suite.ts +++ b/tests/_utils/register_driver_test_suite.ts @@ -21,6 +21,13 @@ interface DriverTestSuiteOptions { export function registerDriverTestSuite(options: DriverTestSuiteOptions) { const { test } = options + test('migrate should be safe to call repeatedly', async () => { + const adapter = await options.createAdapter() + + await adapter.migrate() + await adapter.migrate() + }) + test('popFrom should return null when queue is empty', async ({ assert }) => { const adapter = await options.createAdapter() adapter.setWorkerId('worker-1') diff --git a/tests/adapter.spec.ts b/tests/adapter.spec.ts index d4d147d..8914783 100644 --- a/tests/adapter.spec.ts +++ b/tests/adapter.spec.ts @@ -628,6 +628,649 @@ test.group('Adapter | Redis', (group) => { assert.isNull(await connection.hget(metadataKey, 'metadata-stalled-uuid-1')) assert.isNull(await adapter.getJob('metadata-stalled-uuid-1', queue)) }) + + test('migrate makes pre-existing schedules claimable from the due index', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + + // Simulate pre-upgrade schedule data: write hash + index directly, skip ZSET + const id = 'pre-existing-schedule' + const pastRunAt = (Date.now() - 5_000).toString() + await connection + .multi() + .hset(`schedules::${id}`, { + id, + name: 'LegacyJob', + payload: '{}', + status: 'active', + every_ms: '60000', + timezone: 'UTC', + next_run_at: pastRunAt, + last_run_at: '', + run_count: '0', + created_at: Date.now().toString(), + }) + .sadd('schedules::index', id) + .exec() + + // Without backfill, ZSET has no entry so claim returns null + const beforeBackfill = await adapter.claimDueSchedule() + assert.isNull(beforeBackfill) + + await adapter.migrate() + + const score = await connection.zscore('schedules::due', id) + assert.equal(Number(score), Number(pastRunAt)) + + const afterBackfill = await adapter.claimDueSchedule() + assert.isNotNull(afterBackfill) + assert.equal(afterBackfill!.id, id) + }) + + test('backfillDueIndex is idempotent', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + const nextRunAt = Date.now() + 30_000 + + await adapter.upsertSchedule({ + id: 'idempotent-schedule', + name: 'TestJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + await adapter.updateSchedule('idempotent-schedule', { + nextRunAt: new Date(nextRunAt), + }) + + await connection + .multi() + .del('schedules::due') + .zadd('schedules::due', Date.now() - 10_000, 'orphaned-schedule') + .exec() + + await adapter.backfillDueIndex() + const firstMembers = await connection.zrange('schedules::due', 0, -1, 'WITHSCORES') + + await adapter.backfillDueIndex() + const secondMembers = await connection.zrange('schedules::due', 0, -1, 'WITHSCORES') + + assert.deepEqual(firstMembers, ['idempotent-schedule', nextRunAt.toString()]) + assert.deepEqual(secondMembers, firstMembers) + }) + + test('backfillDueIndex rebuilds the index in one atomic Redis command', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + const id = 'atomic-backfill-schedule' + + await connection + .multi() + .hset(`schedules::${id}`, { + id, + status: 'active', + next_run_at: (Date.now() + 30_000).toString(), + }) + .sadd('schedules::index', id) + .exec() + + const { writes } = await withRedisWriteSpy({ + connection, + run: () => adapter.backfillDueIndex(), + }) + + assert.equal(writes, 1) + }) + + test('concurrent migration and schedule writes leave the due index canonical', async ({ + assert, + cleanup, + }) => { + const secondConnection = new Redis({ + host: process.env.REDIS_HOST || 'localhost', + port: Number.parseInt(process.env.REDIS_PORT || '6379', 10), + keyPrefix: KEY_PREFIX, + db: 15, + }) + cleanup(async () => { + await secondConnection.quit() + }) + + const adapter = new RedisAdapter(connection) + const secondAdapter = new RedisAdapter(secondConnection) + + for (let i = 0; i < 20; i++) { + const id = `migration-write-schedule-${i}` + const nextRunAt = Date.now() + 30_000 + i + + await Promise.all([ + adapter.backfillDueIndex(), + secondAdapter.upsertSchedule({ + id, + name: 'MigrationWriteJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }), + ]) + await Promise.all([ + adapter.backfillDueIndex(), + secondAdapter.updateSchedule(id, { nextRunAt: new Date(nextRunAt) }), + ]) + + assert.equal(Number(await connection.zscore('schedules::due', id)), nextRunAt) + } + }) + + test('stale ZSET score is self-healed during claim', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + const id = 'stale-score-schedule' + const futureRunAt = Date.now() + 60_000 + + await adapter.upsertSchedule({ + id, + name: 'StaleJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + await adapter.updateSchedule(id, { nextRunAt: new Date(futureRunAt) }) + + // Corrupt the ZSET score to a past value while hash still says future + await connection.zadd('schedules::due', Date.now() - 10_000, id) + + const claimed = await adapter.claimDueSchedule() + assert.isNull(claimed, 'should not claim when hash says schedule is not due yet') + + // ZSET score should have been repaired to match the hash + const repairedScore = await connection.zscore('schedules::due', id) + assert.equal(Number(repairedScore), futureRunAt) + }) + + test('schedule lifecycle keeps the due index aligned with canonical state', async ({ + assert, + }) => { + const adapter = new RedisAdapter(connection) + const id = 'lifecycle-schedule' + const firstRunAt = Date.now() + 30_000 + const pausedRunAt = firstRunAt + 30_000 + + await adapter.upsertSchedule({ + id, + name: 'LifecycleJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + await adapter.updateSchedule(id, { nextRunAt: new Date(firstRunAt) }) + assert.equal(Number(await connection.zscore('schedules::due', id)), firstRunAt) + + await adapter.updateSchedule(id, { + status: 'paused', + nextRunAt: new Date(pausedRunAt), + }) + assert.isNull(await connection.zscore('schedules::due', id)) + + await adapter.updateSchedule(id, { nextRunAt: new Date(pausedRunAt + 30_000) }) + assert.isNull(await connection.zscore('schedules::due', id)) + + await adapter.updateSchedule(id, { status: 'active' }) + assert.equal(Number(await connection.zscore('schedules::due', id)), pausedRunAt + 30_000) + + await adapter.updateSchedule(id, { nextRunAt: null }) + assert.isNull(await connection.zscore('schedules::due', id)) + + await adapter.updateSchedule(id, { nextRunAt: new Date(firstRunAt) }) + await adapter.deleteSchedule(id) + assert.isNull(await connection.zscore('schedules::due', id)) + }) + + test('schedule mutations update canonical state and its index in one command', async ({ + assert, + }) => { + const adapter = new RedisAdapter(connection) + const id = 'atomic-schedule-mutation' + + const { writes: upsertWrites } = await withRedisWriteSpy({ + connection, + run: () => + adapter.upsertSchedule({ + id, + name: 'AtomicJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }), + }) + assert.equal(upsertWrites, 1) + + const { writes: updateWrites } = await withRedisWriteSpy({ + connection, + run: () => adapter.updateSchedule(id, { nextRunAt: new Date(Date.now() + 30_000) }), + }) + assert.equal(updateWrites, 1) + }) + + test('concurrent resume and next-run updates leave the due index canonical', async ({ + assert, + cleanup, + }) => { + const secondConnection = new Redis({ + host: process.env.REDIS_HOST || 'localhost', + port: Number.parseInt(process.env.REDIS_PORT || '6379', 10), + keyPrefix: KEY_PREFIX, + db: 15, + }) + cleanup(async () => { + await secondConnection.quit() + }) + + const adapter = new RedisAdapter(connection) + const secondAdapter = new RedisAdapter(secondConnection) + const id = 'concurrent-resume-schedule' + + await adapter.upsertSchedule({ + id, + name: 'ConcurrentJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + + for (let i = 0; i < 20; i++) { + const nextRunAt = Date.now() + 30_000 + i + await adapter.updateSchedule(id, { status: 'paused' }) + + await Promise.all([ + adapter.updateSchedule(id, { nextRunAt: new Date(nextRunAt) }), + secondAdapter.updateSchedule(id, { status: 'active' }), + ]) + + const schedule = await adapter.getSchedule(id) + assert.equal(schedule!.status, 'active') + assert.equal(Number(await connection.zscore('schedules::due', id)), nextRunAt) + } + }) + + test('concurrent upsert and next-run updates leave the due index canonical', async ({ + assert, + cleanup, + }) => { + const secondConnection = new Redis({ + host: process.env.REDIS_HOST || 'localhost', + port: Number.parseInt(process.env.REDIS_PORT || '6379', 10), + keyPrefix: KEY_PREFIX, + db: 15, + }) + cleanup(async () => { + await secondConnection.quit() + }) + + const adapter = new RedisAdapter(connection) + const secondAdapter = new RedisAdapter(secondConnection) + const id = 'concurrent-upsert-schedule' + + await adapter.upsertSchedule({ + id, + name: 'ConcurrentJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + + for (let i = 0; i < 20; i++) { + const nextRunAt = Date.now() + 60_000 + i + await adapter.updateSchedule(id, { status: 'paused' }) + + await Promise.all([ + adapter.updateSchedule(id, { nextRunAt: new Date(nextRunAt) }), + secondAdapter.upsertSchedule({ + id, + name: 'ConcurrentJob', + payload: { iteration: i }, + everyMs: 60_000, + timezone: 'UTC', + }), + ]) + + const schedule = await adapter.getSchedule(id) + assert.equal(schedule!.status, 'active') + assert.equal(schedule!.nextRunAt!.getTime(), nextRunAt) + assert.equal(Number(await connection.zscore('schedules::due', id)), nextRunAt) + } + }) + + test('interval claims update the due index from the canonical hash', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + const id = 'interval-index-schedule' + + await adapter.upsertSchedule({ + id, + name: 'IntervalJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + await adapter.updateSchedule(id, { nextRunAt: new Date(Date.now() - 1_000) }) + + assert.equal((await adapter.claimDueSchedule())?.id, id) + + const schedule = await adapter.getSchedule(id) + const score = await connection.zscore('schedules::due', id) + assert.isNotNull(schedule!.nextRunAt) + assert.equal(Number(score), schedule!.nextRunAt!.getTime()) + }) + + test('cron claims update the due index from the canonical hash', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + const id = 'cron-index-schedule' + + await adapter.upsertSchedule({ + id, + name: 'CronJob', + payload: {}, + cronExpression: '* * * * *', + timezone: 'UTC', + }) + await adapter.updateSchedule(id, { nextRunAt: new Date(Date.now() - 1_000) }) + + assert.equal((await adapter.claimDueSchedule())?.id, id) + + const schedule = await adapter.getSchedule(id) + const score = await connection.zscore('schedules::due', id) + assert.isNotNull(schedule!.nextRunAt) + assert.equal(Number(score), schedule!.nextRunAt!.getTime()) + }) + + test('cron finalization does not undo a concurrent pause or delete', async ({ + assert, + cleanup, + }) => { + const secondConnection = new Redis({ + host: process.env.REDIS_HOST || 'localhost', + port: Number.parseInt(process.env.REDIS_PORT || '6379', 10), + keyPrefix: KEY_PREFIX, + db: 15, + }) + cleanup(async () => { + await secondConnection.quit() + }) + + const adapter = new RedisAdapter(connection) + const secondAdapter = new RedisAdapter(secondConnection) + + for (const mutation of ['pause', 'delete'] as const) { + const id = `cron-finalize-${mutation}` + await adapter.upsertSchedule({ + id, + name: 'CronFinalizeJob', + payload: {}, + cronExpression: '* * * * *', + timezone: 'UTC', + }) + await adapter.updateSchedule(id, { nextRunAt: new Date(Date.now() - 1_000) }) + + const originalEval = connection.eval.bind(connection) + let releaseClaim!: () => void + let claimReturned!: () => void + const claimReleased = new Promise((resolve) => { + releaseClaim = resolve + }) + const claimHasReturned = new Promise((resolve) => { + claimReturned = resolve + }) + let gateNextEval = true + + connection.eval = (async (...args: Parameters) => { + const result = await originalEval(...args) + if (gateNextEval) { + gateNextEval = false + claimReturned() + await claimReleased + } + return result + }) as typeof connection.eval + + const claim = adapter.claimDueSchedule() + await claimHasReturned + + if (mutation === 'pause') { + await secondAdapter.updateSchedule(id, { status: 'paused' }) + } else { + await secondAdapter.deleteSchedule(id) + } + + releaseClaim() + await claim + connection.eval = originalEval + + if (mutation === 'pause') { + assert.equal((await adapter.getSchedule(id))!.status, 'paused') + } else { + assert.isNull(await adapter.getSchedule(id)) + } + assert.isNull(await connection.zscore('schedules::due', id)) + } + }) + + test('cron finalization does not apply a calculation from superseded configuration', async ({ + assert, + cleanup, + }) => { + const secondConnection = new Redis({ + host: process.env.REDIS_HOST || 'localhost', + port: Number.parseInt(process.env.REDIS_PORT || '6379', 10), + keyPrefix: KEY_PREFIX, + db: 15, + }) + cleanup(async () => { + await secondConnection.quit() + }) + + const adapter = new RedisAdapter(connection) + const secondAdapter = new RedisAdapter(secondConnection) + const id = 'cron-finalize-superseded-config' + + await adapter.upsertSchedule({ + id, + name: 'OriginalCronJob', + payload: {}, + cronExpression: '0 9 * * *', + timezone: 'UTC', + }) + await adapter.updateSchedule(id, { nextRunAt: new Date(Date.now() - 1_000) }) + + const originalEval = connection.eval.bind(connection) + let releaseClaim!: () => void + let claimReturned!: () => void + const claimReleased = new Promise((resolve) => { + releaseClaim = resolve + }) + const claimHasReturned = new Promise((resolve) => { + claimReturned = resolve + }) + let gateNextEval = true + + connection.eval = (async (...args: Parameters) => { + const result = await originalEval(...args) + if (gateNextEval) { + gateNextEval = false + claimReturned() + await claimReleased + } + return result + }) as typeof connection.eval + + const claim = adapter.claimDueSchedule() + await claimHasReturned + await secondAdapter.upsertSchedule({ + id, + name: 'UpdatedCronJob', + payload: {}, + cronExpression: '0 9 * * *', + timezone: 'America/New_York', + }) + + releaseClaim() + await claim + connection.eval = originalEval + + const schedule = await adapter.getSchedule(id) + assert.equal(schedule!.name, 'UpdatedCronJob') + assert.equal(schedule!.timezone, 'America/New_York') + assert.isNull(schedule!.nextRunAt) + assert.isNull(await connection.zscore('schedules::due', id)) + }) + + test('cron finalization survives a concurrent manual trigger metadata update', async ({ + assert, + cleanup, + }) => { + const secondConnection = new Redis({ + host: process.env.REDIS_HOST || 'localhost', + port: Number.parseInt(process.env.REDIS_PORT || '6379', 10), + keyPrefix: KEY_PREFIX, + db: 15, + }) + cleanup(async () => { + await secondConnection.quit() + }) + + const adapter = new RedisAdapter(connection) + const secondAdapter = new RedisAdapter(secondConnection) + const id = 'cron-finalize-concurrent-trigger' + + await adapter.upsertSchedule({ + id, + name: 'CronTriggerJob', + payload: {}, + cronExpression: '* * * * *', + timezone: 'UTC', + }) + await adapter.updateSchedule(id, { nextRunAt: new Date(Date.now() - 1_000) }) + + const originalEval = connection.eval.bind(connection) + let releaseClaim!: () => void + let claimReturned!: () => void + const claimReleased = new Promise((resolve) => { + releaseClaim = resolve + }) + const claimHasReturned = new Promise((resolve) => { + claimReturned = resolve + }) + let gateNextEval = true + + connection.eval = (async (...args: Parameters) => { + const result = await originalEval(...args) + if (gateNextEval) { + gateNextEval = false + claimReturned() + await claimReleased + } + return result + }) as typeof connection.eval + + const claim = adapter.claimDueSchedule() + await claimHasReturned + await secondAdapter.updateSchedule(id, { + runCount: 2, + lastRunAt: new Date(), + }) + + releaseClaim() + await claim + connection.eval = originalEval + + const schedule = await adapter.getSchedule(id) + assert.equal(schedule!.runCount, 2) + assert.isNotNull(schedule!.nextRunAt) + assert.equal( + Number(await connection.zscore('schedules::due', id)), + schedule!.nextRunAt!.getTime() + ) + }) + + test('updating a deleted schedule does not recreate or index it', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + const id = 'update-after-delete' + + await adapter.upsertSchedule({ + id, + name: 'DeletedScheduleJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + await adapter.deleteSchedule(id) + + await adapter.updateSchedule(id, { + status: 'active', + nextRunAt: new Date(Date.now() - 1_000), + runCount: 0, + }) + + assert.isNull(await adapter.getSchedule(id)) + assert.isNull(await connection.zscore('schedules::due', id)) + assert.isNull(await adapter.claimDueSchedule()) + }) + + test('claim removes a malformed due score and continues to a valid schedule', async ({ + assert, + }) => { + const adapter = new RedisAdapter(connection) + const malformedId = 'malformed-next-run-at' + const validId = 'valid-after-malformed' + + await connection + .multi() + .hset(`schedules::${malformedId}`, { + id: malformedId, + status: 'active', + next_run_at: 'not-a-number', + }) + .sadd('schedules::index', malformedId) + .zadd('schedules::due', Date.now() - 2_000, malformedId) + .exec() + + await adapter.upsertSchedule({ + id: validId, + name: 'ValidJob', + payload: {}, + everyMs: 60_000, + timezone: 'UTC', + }) + await adapter.updateSchedule(validId, { nextRunAt: new Date(Date.now() - 1_000) }) + + assert.equal((await adapter.claimDueSchedule())!.id, validId) + assert.isNull(await connection.zscore('schedules::due', malformedId)) + }) + + test('exhausted schedules are removed from the due index', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + + for (const config of [ + { id: 'limited-interval', everyMs: 60_000, limit: 1 }, + { id: 'limited-cron', cronExpression: '* * * * *', limit: 1 }, + { id: 'ended-interval', everyMs: 60_000, to: new Date(Date.now() + 1_000) }, + ]) { + await adapter.upsertSchedule({ + ...config, + name: 'ExhaustedJob', + payload: {}, + timezone: 'UTC', + }) + await adapter.updateSchedule(config.id, { nextRunAt: new Date(Date.now() - 1_000) }) + + assert.equal((await adapter.claimDueSchedule())?.id, config.id) + assert.isNull((await adapter.getSchedule(config.id))!.nextRunAt) + assert.isNull(await connection.zscore('schedules::due', config.id)) + } + }) + + test('claim removes a due index member whose canonical hash is missing', async ({ assert }) => { + const adapter = new RedisAdapter(connection) + await connection.zadd('schedules::due', Date.now() - 1_000, 'missing-schedule') + + assert.isNull(await adapter.claimDueSchedule()) + assert.isNull(await connection.zscore('schedules::due', 'missing-schedule')) + }) }) test.group('Adapter | Knex (SQLite)', (group) => { diff --git a/tests/queue_manager.spec.ts b/tests/queue_manager.spec.ts index 46dde6d..df6b1f0 100644 --- a/tests/queue_manager.spec.ts +++ b/tests/queue_manager.spec.ts @@ -263,6 +263,7 @@ test.group('QueueManager', () => { destroy: async () => { destroyedCount++ }, + migrate: async () => {}, upsertSchedule: async () => 'schedule-id', createSchedule: async () => 'schedule-id', getSchedule: async () => null, @@ -330,6 +331,7 @@ test.group('QueueManager', () => { size: async () => 0, sizeOf: async () => 0, destroy: async () => {}, + migrate: async () => {}, upsertSchedule: async () => 'schedule-id', createSchedule: async () => 'schedule-id', getSchedule: async () => null,