Repository navigation
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #89, #122, #78
get_schedules()calls_get_previous_time_schedules()on every call, which runsSCAN 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_rununused and the docstrings saying "first run only" (#89).last_runfor them and a past schedule that stays in Redis (failedpost_send) would fire every minute. Deleted ones are already filtered out by the existingmget.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_itercalls in the deprecatedRedisScheduleSourcealone.Tests:
test_past_schedules_lookup_once_per_minutechecks for one scan per minute (fails on main withassert 60 == 1).test_schedule_from_pastnow 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:count=1000The max column is the per-minute scan.