Repository navigation
Keep a checkpoint record per task instead of one for the whole schedule - #17
Conversation
Reviewer's GuideReplaces the growing, shared checkpoint payload with independently persisted last-run timestamps per task plus a schedule-level drained position, allowing crash-safe resumption, bounded value sizes, one write per run, and backward-compatible upgrades. Sequence diagram for per-task checkpoint persistencesequenceDiagram
participant Generator as TaskGenerator
participant Checkpoint
participant Cache
participant Task
Generator->>Checkpoint: lastRun(taskId)
Checkpoint->>Cache: getMultiple(scheduleKey, taskKeys)
Cache-->>Checkpoint: schedule position and task timestamps
Checkpoint-->>Generator: task last-run time
Generator->>Task: getNextRunDate(seedTime)
Generator-->>Task: yield TaskContext
Generator->>Checkpoint: save(taskId, runTime)
Checkpoint->>Cache: set(taskKey(taskId), runTime)
Generator->>Checkpoint: markTick(now)
Checkpoint->>Cache: set(cacheKey(), schedule position)
Entity relationship diagram for checkpoint recordserDiagram
SCHEDULE_CHECKPOINT {
string schedule_key PK
timestamp last_evaluated
}
TASK_CHECKPOINT {
string task_key PK
string task_id
timestamp last_run
}
SCHEDULE_CHECKPOINT ||--o{ TASK_CHECKPOINT : owns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. If a per-task checkpoint is wrong or fails to advance as intended, the scheduler can run that task again after a restart; any side effects the task already performed would remain after reverting this change. The checkpoint records can be cleared or rebuilt, but effects from a duplicate run may not be reversible.
0d01fb4 to
b069fc7
Compare
b069fc7 to
96c35d8
Compare
One record held every run emitted at a due time, so its size grew with the tasks sharing that time and every run rewrote all of them: a thousand tasks due at the same minute wrote 14.3 MB over that minute, and a backend with a limit on the size of a stored value would eventually refuse the write. Each task now keeps the time of its own last emitted run, and the schedule keeps the time it was last evaluated to the end. A stored value is 42 bytes whatever the schedule, and the same thousand tasks write 25 kB. The position of the schedule moves only once a tick has drained, so a process that died partway leaves the runs it never started still due; a task picked up after its own record no longer has to be told from one that was already run.
96c35d8 to
37687a7
Compare
samdark
left a comment
There was a problem hiding this comment.
Two regressions need correction before merge:
-
In TaskGenerator.php:149, a new stateful scheduler can lose unstarted runs after its first due tick is interrupted. Query
nextRunDate()at 00:00, start the due tick at 00:01, and interrupt it after the first task completes. No drained schedule position exists yet, so a fresh scheduler seeds the remaining task from 00:01 and skips its pending run. The base resumes that task. Persist the initial seed before processing can be interrupted and cover recovery without a prior idle tick. -
In Checkpoint.php:244, appending the task hash breaks previously valid names on portable cache backends.
daily_subscription_invoicesused a 51-character key but now requests 68-character task keys, which a 64-character-limited cache rejects during scheduler construction. PSR-16 guarantees support only through 64 characters. Derive a bounded task key from the schedule/task identity and add a 64-character-limited cache regression.
The position of the schedule moves only once a tick has drained, which is what leaves the runs of a process that died partway still due. Until the first tick drains there is no position at all, and a task with no record of its own was then seeded from the now of whichever process read the schedule next. A daemon that asks for the next run date at 00:00, wakes for the tick at 00:01 and is killed after its first task therefore lost the rest of that tick: the replacement process found no position, seeded the untouched tasks from 00:01 and stepped over the runs they never started. The point the runs are computed from is now written down as they are computed, before any of them is emitted, so an interrupted first tick leaves its unstarted runs behind a position rather than behind nothing.
A task key spelled the schedule name out and appended the hash of the task id, so it ran 41 characters longer than that name. A schedule called daily_subscription_invoices kept its own position under a key of 51 characters, which every backend took, and asked for task keys of 68, which a backend limited to 64 refuses — while reading the records back, so the schedule could not even be constructed. Hashing both halves holds the key at 57 characters whatever the schedule and the tasks are called. The tasks of one schedule still share a prefix, so they can be told apart from another schedule's when the cache is inspected.
#17 landed while this was open. The checkpoint it adds and the identifiers this changes do not touch the same code; the changelog is the only file both write to.
Stacked on #16 — review that one first. The base moves to
masteronce it lands.#16 made the checkpoint record which tasks had already run, by id. That left one record holding every run emitted at a due time, so its size grew with the tasks sharing that time, and
persist()rewrote the whole list after every single run.Measured over one due minute, with every task due at it:
Each task now keeps the time of its own last emitted run under a key of its own, and the schedule keeps the time it was last evaluated to the end. Writing is one value per run instead of a growing list, and a stored value is the same size whatever the schedule, so no backend limit on the size of a value is in reach.
The position of the schedule moves only once a tick has drained. A process that died partway leaves it where it was, so the runs it never started stay due — which is what lets a task be seeded strictly after its own record, and drops the pass that had to tell a re-discovered run from a fresh one.
A checkpoint written by either earlier format is read for its time alone, so an upgrade resumes after it without replaying.
Summary by Sourcery
Keep checkpoint state per task so schedule persistence remains bounded and interrupted runs resume correctly.
Enhancements:
Tests: