Skip to content

fix: scan past time keys once per minute in ListRedisScheduleSource - #149

Open
volveezz wants to merge 2 commits into
taskiq-python:mainfrom
volveezz:fix/list-source-catch-up-once-per-minute
Open

volveezz wants to merge 2 commits into
taskiq-python:mainfrom
volveezz:fix/list-source-catch-up-once-per-minute

Conversation

@volveezz

Copy link
Copy Markdown

Refs #89, #122, #78

get_schedules() calls _get_previous_time_schedules() on every call, which runs SCAN MATCH {prefix}:time:* over the whole db at the default COUNT 10. With a short --update-interval, every tick pays for the whole keyspace. With 100k unrelated keys one call takes ~1.7 s, so a 1 s interval can't keep up.

#83 dropped the first-run gate on purpose so past schedules aren't lost, but left _is_first_run unused and the docstrings saying "first run only" (#89).

  • The past-keys scan runs on the first call of each wall-clock minute, and every call in that minute returns the ids it found. Otherwise the scheduler forgets last_run for them and a past schedule that stays in Redis (failed post_send) would fire every minute. Deleted ones are already filtered out by the existing mget.
  • The only behaviour change: a schedule added with an already-passed time is picked up at the next minute boundary instead of the next call, so up to ~60 s later. With the default 60 s interval nothing changes.
  • scan_iter(..., count=1000) cuts a pass over 100k keys from ~10k round trips to ~100. Separate commit, easy to drop or make configurable.

#121 removes SCAN altogether; this is a smaller fix for the current layout. I left the three scan_iter calls in the deprecated RedisScheduleSource alone.

Tests: test_past_schedules_lookup_once_per_minute checks for one scan per minute (fails on main with assert 60 == 1). test_schedule_from_past now also checks that a past schedule is returned on repeated calls, and that one added to a passed minute after the first run is picked up the next minute, which fails if the plain first-run gate comes back.

Redis 7 on 1 CPU, 100k unrelated keys, 9 schedules, get_schedules() every 1 s for 60 s:

SCAN/s Redis CPU p50 / max
main 5,498 29.5% 1,753 / 1,887 ms
once per minute 315 1.9% 1.2 / 1,747 ms
+ count=1000 3.3 0.3% 1.3 / 32 ms

The max column is the per-minute scan.

get_schedules() ran _get_previous_time_schedules() on every call. That
method SCANs the whole Redis db for `{prefix}:time:*`, so with a short
scheduler update interval the cost of every tick grows with the total
number of keys in the db, not with the number of schedules.

Past time keys are now looked up on the first call of each minute and
the ids found are returned by every call of that minute, so the
scheduler keeps its last-run bookkeeping for them exactly as before.
Schedules written into an already passed minute are still picked up,
at most one minute later. With the default update_interval of 60s the
behaviour is unchanged.

Refs taskiq-python#89, taskiq-python#122
scan_iter() without count uses the Redis default COUNT of 10, so one
pass over a db with 100k keys takes ~10k round trips. COUNT 1000 takes
~100 round trips for the same pass.

Refs taskiq-python#78, taskiq-python#122
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant