perf(redis): use sorted set index for O(log N) schedule claiming

[isimisi]

explanation: #13 (comment)

[RomainLanz]

Hi!

Thanks for the PR.

We need a migration path for existing Redis users before merging this.

With this change, claimDueSchedule() only reads from the new schedules::due ZSET, but users upgrading from the current version will only have the existing schedule hashes plus schedules::index. Those schedules will never be claimed until something manually rewrites them or calls backfillDueIndex().

Can we make the migration automatic/idempotent, or expose and document a required migration step? For example, on startup or first claim, backfill schedules::due from schedules::index for active schedules with next_run_at.

[isimisi]

Hey @RomainLanz, great point. I completely missed the migration path for existing users.

I'll add an automatic backfill that runs on startup: on first claimDueSchedule() call (or worker boot), scan schedules::index and populate schedules::due from any schedule that has a next_run_at. I'll make it idempotent so it's safe to run multiple times across worker restarts or multi-instance deployments.

[isimisi]

Hey @RomainLanz, I pushed a follow-up commit that handles the migration automatically.

On the first claimDueSchedule() call per worker process, it runs backfillDueIndex() to populate the ZSET from existing schedules. An instance flag skips it on subsequent calls, so the cost is a one-time.

The approach is idempotent (ZADD with the same score is a no-op), safe under concurrent workers, and self-healing if the ZSET is ever cleared. backfillDueIndex() is public for anyone who wants manual control.

Does this work for u? Otherwise the we remove the automatic backfill and let users handle it themselves one-time, then we don't need to have a check on claimDueSchedule().

[RomainLanz]

Oops, I have pushed some changes.

Can you resolve the conflicts? 😅

[isimisi]

Should be good now! We have some formatting differences in our editors it seem. Don't know if you care about that haha

[RomainLanz]

Thanks for resolving the conflicts and adding the automatic backfill.

I'm still not fully comfortable merging this as-is.

The main concern is that schedules::due becomes a second source of truth for next_run_at. claimDueSchedule() now trusts the ZSET score and does not re-check the schedule hash next_run_at before claiming. If the ZSET ever contains a stale due score while the hash says the schedule is not due anymore, we could claim it too early.

Since the hash is still the canonical schedule data everywhere else, can we keep the ZSET as a derived index only? For example, the Lua script could validate next_run_at <= now from the hash before claiming, and repair/remove stale ZSET entries when it finds them.

I'm also a bit concerned about the automatic backfill running on the first claimDueSchedule() call per adapter instance. That hides an O(N) scan in the scheduler hot path, and multiple workers can all discover/run it at startup. I think I’d prefer either an explicit migration/bootstrap step, or a Redis-level idempotent backfill marker/lock.

The direction is good, but before merging I'd like us to tighten the invariant: the schedule hash stays canonical, schedules::due stays derived, and claiming repairs stale index entries instead of trusting them.

PS: You can run yarn format:fix to fix the formating

[isimisi]

I'll update the Lua script to re-check next_run_at <= now from the hash before actually claiming. If the ZSET has a stale score it'll repair it (update to match the hash or remove if next_run_at is gone) and move on to the next candidate. That way the ZSET is purely a fast lookup index, never the authority.

On the migration side: I've been going back and forth on this. The thing with a fully explicit migration step is that claimDueSchedule() would still need some kind of per-call check to detect that the user hasn't run it yet. Otherwise the ZSET is just empty and schedules silently stop firing, returning null forever with no error. That check ends up being O(1) either way (an EXISTS or ZCARD), so it's not expensive, but it's still there on the hot path.

So I'm leaning towards the Redis-level marker approach as a middle ground. Something like a SETNX on a marker key, first worker globally runs the backfill, all subsequent instances just hit an O(1) EXISTS and skip straight to claiming. That avoids both the per-instance redundancy and the "user forgot to migrate" silent failure. But if you'd rather keep it fully explicit I can do that too and just add a guard that throws when schedules exist but the ZSET hasn't been populated.

[RomainLanz]

Otherwise the ZSET is just empty and schedules silently stop firing

I believe it is fine if we document it properly. This will be released as a breaking change with a manual migration path.

[isimisi]

So drop the auto-backfill entirely and no check. Let users run the migration themselves.
What's the best DX for that though? Keep backfillDueIndex() public on the Redis adapter (users would need to cast from the Adapter interface), add a migrate() method to the Adapter interface itself, or maybe a standalone migration function?

[isimisi]

Hey! Went ahead and pushed the changes. Went with migrate() on the Adapter interface - other adapters no-op. Also added the hash validation in the Lua script. Should be ready for review.

[isimisi]

@RomainLanz did u decide against this?

[RomainLanz]

Hey! The direction is still good, sorry for the delay.

Since main has evolved, the PR now needs a refresh, notably adding migrate() to KyselyAdapter and resolving any new tests/type errors.

I'm happy to push those compatibility fixes directly to your branch, keeping the scope limited to the Redis ZSET index and migration path. Is that okay with you?

[isimisi]

Hey, yeah go for it! Push whatever u need to the branch.

Sorry for the delay, been deep in thesis writing haha. Let me know if u want me to take anything.

[RomainLanz]

Thanks again for authorizing the maintainer updates. GitHub still reports maintainerCanModify: false, so the branch cannot technically be updated yet. Could you please enable Allow edits from maintainers on this PR? The fixes will be prepared and verified locally in the meantime.

[isimisi]

ah, yes - the fork is org-owned. I've sent you a collaborator invite on Juristic/queue instead, which gives you push access to the branch

[RomainLanz]

Damned, my agent asked for it directly lol. Thanks!

[RomainLanz]

I have made a request so my agent can push to your repository. Let me know once that's approved!

[isimisi]

You should be good now

[Copilot]

Pull request overview

This PR improves Redis schedule-claim performance by introducing a schedules::due sorted-set index (scored by next_run_at) so due schedule lookup no longer requires scanning every schedule ID client-side.

Changes:

• Add and maintain a Redis ZSET due index (schedules::due) and switch schedule claiming to ZRANGEBYSCORE ... LIMIT 1-based selection.
• Introduce an idempotent Adapter#migrate() contract and implement Redis migration via an atomic backfill script.
• Expand Redis adapter test coverage to validate index correctness, idempotency, and concurrent mutation/claim scenarios.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/queue_manager.spec.ts	Update adapter mocks to include the new migrate() method.
tests/adapter.spec.ts	Add comprehensive Redis schedule index + migration + concurrency behavior tests.
tests/_utils/register_driver_test_suite.ts	Add a cross-adapter test ensuring migrate() is safe to call repeatedly.
tests/_mocks/memory_adapter.ts	Implement no-op migrate() for the mock adapter.
src/drivers/sync_adapter.ts	Implement no-op migrate() for the sync adapter.
src/drivers/redis_scripts.ts	Add Lua scripts for upsert/update/backfill/claim using the due ZSET index and safe cron finalization.
src/drivers/redis_adapter.ts	Wire new Lua scripts, maintain schedules::due, and add migrate()/backfillDueIndex().
src/drivers/kysely_adapter.ts	Implement no-op migrate() for the Kysely adapter.
src/drivers/knex_adapter.ts	Implement no-op migrate() for the Knex adapter.
src/drivers/fake_adapter.ts	Implement no-op migrate() for the fake adapter.
src/contracts/adapter.ts	Extend the Adapter contract with an idempotent migrate() method.
README.md	Document Redis schedule migration requirement and recommended deployment usage.
.changelog/redis-schedule-due-index.md	Add changelog entry describing the indexed claims and upgrade notes.
.changelog/hot-reloading-jobs.md	Remove an older changelog entry (cleanup).
.changelog/consistent-job-runtime.md	Remove an older changelog entry (cleanup).
.changelog/adapter-aware-workers-and-schedules.md	Remove an older changelog entry (cleanup).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.