Document the built-in scheduler component plugin (5.2)#592
Conversation
New reference page for the scheduler (config surface, cron syntax, intervals, timezone/DST semantics, handler contract and idempotency, cluster leader election and catch-up behavior), a row in the built-in components table, sidebar entry, and a 5.2 release-notes entry. Companion to HarperFast/harper#1828. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-592 This preview will update automatically when you push new commits. |
There was a problem hiding this comment.
Code Review
This pull request introduces the new scheduler component, which enables recurring jobs to be defined via cron or interval schedules within a component's configuration. The changes include adding a new documentation page, updating the components overview, adding release notes, and registering the new page in the documentation sidebar. A review comment suggests improving the readability of the provided JavaScript example by extracting the time calculation into a named constant.
Record cleanup is built into Harper (table expiration/eviction), so it was a misleading flagship example; point readers there instead and demonstrate the scheduler with jobs only it can do: a daily dataset snapshot (which also demonstrates idempotent handler design) and a scheduled external-API pull. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-592 This preview will update automatically when you push new commits. |
kriszyp
left a comment
There was a problem hiding this comment.
The scheduler reference has three behavior-contract mismatches with the companion core implementation. Please address these before merge.
|
|
||
| <VersionBadge version="v5.2.0" /> | ||
|
|
||
| The scheduler is a built-in plugin that runs recurring jobs declared in a component's configuration. Harper invokes a designated export from your component on a cron or interval schedule, and in a cluster each job fires exactly once per occurrence - on a single, automatically elected node - rather than once per node or worker. |
There was a problem hiding this comment.
The companion implementation has no distributed lock/CAS and explicitly allows duplicate delivery during failover, DST fall-back, and split-brain recovery. Calling this "exactly once per occurrence" contradicts the idempotency warning below and may lead users to omit deduplication. Describe this as leader-coordinated single execution under normal operation with possible duplicates, and update the same absolute claim in the 5.2 release notes.
There was a problem hiding this comment.
Fixed in af1a837: the intro now describes leader-coordinated single execution under normal operation, with explicit duplicate-delivery cases (failover, DST fall-back, split-brain recovery) tied to the idempotency requirement. The 5.2 release-notes entry carried the same overstatement and is aligned too. (AI-generated response)
|
|
||
| **Handlers should be idempotent.** Harper's clustering has no distributed lock, so leadership failover and daylight-saving fall-back can occasionally deliver the same logical occurrence twice. Design handlers so that running twice for one occurrence is harmless. | ||
|
|
||
| Runs of the same job never overlap: if a run is still going when its next occurrence arrives, that occurrence is skipped (with a log entry) rather than stacked. |
There was a problem hiding this comment.
The implementation does not consistently skip an occurrence when a handler outlasts its cadence. It arms the next timer only after the handler settles; an overdue interval then runs immediately, while the cron catch-up sweep can run the latest missed occurrence afterward. This matters because a slow interval handler can execute back-to-back. Either align the implementation with this statement or document the actual late/catch-up behavior.
There was a problem hiding this comment.
Fixed in af1a837, documenting the implementation as-is: runs never overlap; missed time is not queued; an overdue interval job runs back-to-back after a slow handler; cron makes up at most its single most recent missed occurrence (context.catchUp = true). (AI-generated response)
|
|
||
| ### `interval` | ||
|
|
||
| Type: `string` (duration) |
There was a problem hiding this comment.
The schema accepts interval as either a duration string or a number of seconds, so Type: string is incomplete. Runtime validation also rejects intervals longer than 365 days, but only the one-second minimum is documented. List both accepted types and both bounds so configurations do not fail unexpectedly.
There was a problem hiding this comment.
Fixed in af1a837: the field now reads 'Type: string (duration) or number (seconds)' and documents both bounds (1 second to 365 days). (AI-generated response)
Unquoted leading-asterisk crons are YAML aliases and @macros are reserved characters; a space before # in a handler reference silently drops the export name. Surfaced by audit of the feature PR. Also fixes a leftover cleanupOldRecords reference from the example rework. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-592 This preview will update automatically when you push new commits. |
- 'exactly once per occurrence' -> leader-coordinated single execution under normal operation, with possible duplicate delivery during failover/DST fall-back/split-brain recovery (consistent with the idempotency requirement); release-notes entry aligned too - overlap semantics corrected: runs never overlap, but an overdue interval job runs back-to-back after a slow handler, and cron makes up at most its most recent missed occurrence - interval documents both accepted types (duration string or number of seconds) and both bounds (1s to 365 days) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-592 This preview will update automatically when you push new commits. |
heskew
left a comment
There was a problem hiding this comment.
Verified the documented config surface against #1828's implementation: job keys (name/cron/interval/timezone/handler) match scheduler.ts's KNOWN_JOB_KEYS exactly, the "exactly one of cron/interval" rule and the module#export handler format match, and the context props (jobName/scheduledAt/catchUp) match engine.ts's handler call. Accurate as written — just worth landing this alongside/after #1828. LGTM.
🤖 Reviewed via cross-model pipeline; approved by @heskew.
kriszyp
left a comment
There was a problem hiding this comment.
Approving — clean, thorough addition; the clustering/idempotency edge-case coverage is great.
One fix worth making to the example (reference/components/scheduler.md:97): spread the untrusted external payload before the explicit keys, so an id in the API response can't silently override our 'latest' id and redirect the write:
await tables.ExchangeRates.put({ ...rates, id: 'latest', fetchedAt: context.scheduledAt });(As written — ...rates last — a rogue id key from the external API would win.) Always spread untrusted objects first so explicit keys override them.
— Claude
Review nit (kriszyp): as written, a rogue `id` in the external API response would override the explicit 'latest' id and redirect the write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-592 This preview will update automatically when you push new commits. |
Documents the new built-in
schedulercomponent plugin shipping in HarperFast/harper#1828 (issue HarperFast/harper#951):reference/components/scheduler.md: thescheduler:config surface, supported cron syntax,intervaldurations, timezone/DST semantics, the handler contract (JobRunContext, idempotency requirement), and cluster behavior (leader election, failover timing, missed-occurrence catch-up, run state).reference/components/overview.mdand a sidebar entry.Draft until the feature PR itself leaves draft; the two should land together.
npm run buildandnpm run format:checkpass locally (the one broken-anchor warning on the 5.1 release-notes page is pre-existing on main).Generated by an LLM (Claude Fable 5) pairing with @jcohen-hdb.
🤖 Generated with Claude Code