mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
refactor: show the warning in the schedule editor, fed by the caller's runs
This commit is contained in:
@@ -1,48 +1,48 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { scheduleOutlastsItsInterval } from './scheduleDrift'
|
||||
import { runsOutlastingInterval } from './scheduleDrift'
|
||||
|
||||
const runs = (duration_ms: number) => Array.from({ length: 5 }, () => ({ duration_ms }))
|
||||
|
||||
// Each exemption below is a schedule that is genuinely running less often than
|
||||
// its cron reads, and is still not something to report. Losing one of them turns
|
||||
// the badge into noise on a correctly configured schedule.
|
||||
describe('scheduleOutlastsItsInterval', () => {
|
||||
it('flags runs that outlast the gap between slots', () => {
|
||||
expect(scheduleOutlastsItsInterval({ enabled: true, interval_s: 20, jobs: runs(50_000) })).toBe(
|
||||
true
|
||||
describe('runsOutlastingInterval', () => {
|
||||
it('reports how long runs that outlast the gap between slots are taking', () => {
|
||||
expect(runsOutlastingInterval({ enabled: true, interval_s: 20, jobs: runs(50_000) })).toBe(
|
||||
50_000
|
||||
)
|
||||
})
|
||||
|
||||
it('says nothing while the runs still fit', () => {
|
||||
expect(scheduleOutlastsItsInterval({ enabled: true, interval_s: 20, jobs: runs(5_000) })).toBe(
|
||||
false
|
||||
)
|
||||
expect(
|
||||
runsOutlastingInterval({ enabled: true, interval_s: 20, jobs: runs(5_000) })
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('exempts a schedule that queues its next run as the previous one starts', () => {
|
||||
expect(
|
||||
scheduleOutlastsItsInterval({
|
||||
runsOutlastingInterval({
|
||||
enabled: true,
|
||||
queues_next_run_at_start: true,
|
||||
interval_s: 20,
|
||||
jobs: runs(50_000)
|
||||
})
|
||||
).toBe(false)
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('exempts a disabled schedule, which is not running at all', () => {
|
||||
expect(
|
||||
scheduleOutlastsItsInterval({ enabled: false, interval_s: 20, jobs: runs(50_000) })
|
||||
).toBe(false)
|
||||
runsOutlastingInterval({ enabled: false, interval_s: 20, jobs: runs(50_000) })
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('waits for more than one run before calling it a pattern', () => {
|
||||
expect(
|
||||
scheduleOutlastsItsInterval({
|
||||
runsOutlastingInterval({
|
||||
enabled: true,
|
||||
interval_s: 20,
|
||||
jobs: [{ duration_ms: 50_000 }]
|
||||
})
|
||||
).toBe(false)
|
||||
).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,26 +1,29 @@
|
||||
const MIN_RUNS = 3
|
||||
|
||||
/**
|
||||
* Whether a schedule's runs are taking longer than the gap between its slots.
|
||||
*
|
||||
* A plain script schedule queues its next run only once the previous one has
|
||||
* completed, so a run that outlasts the interval necessarily pushes the next
|
||||
* one to a later slot: the schedule quietly runs less often than its cron says.
|
||||
* Schedules that queue the next run as the previous one starts are exempt, and
|
||||
* the server says which those are.
|
||||
*
|
||||
* Reads the runs the schedules page has already loaded, and asks for a few of
|
||||
* them so that one slow run is not read as a change of cadence.
|
||||
*/
|
||||
export function scheduleOutlastsItsInterval(schedule: {
|
||||
/** What the schedules page has already loaded about how a schedule is running. */
|
||||
export type ScheduleRunsSample = {
|
||||
queues_next_run_at_start?: boolean
|
||||
enabled?: boolean
|
||||
interval_s?: number
|
||||
jobs?: Array<{ duration_ms: number }>
|
||||
}): boolean {
|
||||
const { queues_next_run_at_start, enabled, interval_s, jobs } = schedule
|
||||
if (queues_next_run_at_start || !enabled || !interval_s || (jobs?.length ?? 0) < MIN_RUNS)
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* How long a schedule's runs have been taking, when that is longer than the gap
|
||||
* between its slots, and `undefined` otherwise.
|
||||
*
|
||||
* A plain script schedule queues its next run only once the previous one has
|
||||
* completed, so a run that outlasts the interval necessarily pushes the next one
|
||||
* to a later slot: the schedule quietly runs less often than its cron says.
|
||||
* Schedules that queue the next run as the previous one starts are exempt, and
|
||||
* the server says which those are.
|
||||
*/
|
||||
export function runsOutlastingInterval(sample: ScheduleRunsSample): number | undefined {
|
||||
const { queues_next_run_at_start, enabled, interval_s, jobs } = sample
|
||||
if (queues_next_run_at_start || !enabled || !interval_s || (jobs?.length ?? 0) < MIN_RUNS) {
|
||||
return undefined
|
||||
}
|
||||
const durations = jobs!.map((j) => j.duration_ms).sort((a, b) => a - b)
|
||||
return durations[durations.length >> 1] > interval_s * 1000
|
||||
const median = durations[durations.length >> 1]
|
||||
return median > interval_s * 1000 ? median : undefined
|
||||
}
|
||||
|
||||
@@ -1,8 +1,15 @@
|
||||
<script lang="ts">
|
||||
import { tickPainted } from '$lib/utils/paint'
|
||||
import ScheduleEditorInner from './ScheduleEditorInner.svelte'
|
||||
import type { ScheduleRunsSample } from '$lib/components/schedules/scheduleDrift'
|
||||
|
||||
let { onUpdate }: { onUpdate?: (path?: string) => void } = $props()
|
||||
let {
|
||||
onUpdate,
|
||||
getRunsSample
|
||||
}: {
|
||||
onUpdate?: (path?: string) => void
|
||||
getRunsSample?: (path: string) => ScheduleRunsSample | undefined
|
||||
} = $props()
|
||||
|
||||
let open = $state(false)
|
||||
export async function openEdit(ePath: string, isFlow: boolean, fixedScriptPath?: string) {
|
||||
@@ -26,5 +33,5 @@
|
||||
</script>
|
||||
|
||||
{#if open}
|
||||
<ScheduleEditorInner {onUpdate} bind:this={drawer} />
|
||||
<ScheduleEditorInner {onUpdate} {getRunsSample} bind:this={drawer} />
|
||||
{/if}
|
||||
|
||||
@@ -30,7 +30,15 @@
|
||||
type ErrorHandler
|
||||
} from '$lib/gen'
|
||||
import { enterpriseLicense, userStore, workspaceStore } from '$lib/stores'
|
||||
import { canWrite, emptyString, formatCron, sendUserToast, cronV1toV2 } from '$lib/utils'
|
||||
import {
|
||||
canWrite,
|
||||
emptyString,
|
||||
formatCron,
|
||||
msToReadableTime,
|
||||
msToReadableTimeShort,
|
||||
sendUserToast,
|
||||
cronV1toV2
|
||||
} from '$lib/utils'
|
||||
import { base } from '$lib/base'
|
||||
import Section from '$lib/components/Section.svelte'
|
||||
import { List, Loader2, Save, AlertTriangle } from 'lucide-svelte'
|
||||
@@ -50,6 +58,10 @@
|
||||
import { twMerge } from 'tailwind-merge'
|
||||
import PermissionedAsLine from '../PermissionedAsLine.svelte'
|
||||
import { getTriggerWorkspace } from '$lib/components/triggers/triggerWorkspace'
|
||||
import {
|
||||
runsOutlastingInterval,
|
||||
type ScheduleRunsSample
|
||||
} from '$lib/components/schedules/scheduleDrift'
|
||||
|
||||
let {
|
||||
useDrawer = true,
|
||||
@@ -63,7 +75,14 @@
|
||||
onConfigChange = undefined,
|
||||
onDelete = undefined,
|
||||
onReset = undefined,
|
||||
trigger = undefined
|
||||
trigger = undefined,
|
||||
getRunsSample = undefined
|
||||
}: {
|
||||
[key: string]: any
|
||||
/// Supplied by callers that already hold the schedule's recent runs, so the
|
||||
/// warning below costs no fetch of its own. Absent everywhere else, and the
|
||||
/// warning simply does not appear.
|
||||
getRunsSample?: (path: string) => ScheduleRunsSample | undefined
|
||||
} = $props()
|
||||
|
||||
let optionTabSelected:
|
||||
@@ -123,6 +142,10 @@
|
||||
let labels: string[] | undefined = $state(undefined)
|
||||
let description = $state('')
|
||||
let no_flow_overlap = $state(false)
|
||||
// Measured, not configured: it describes the runs the deployed schedule has
|
||||
// already had, so it is read from the caller's sample rather than the form.
|
||||
let runsSample = $derived(initialPath ? getRunsSample?.(initialPath) : undefined)
|
||||
let outlastingMs = $derived(runsSample ? runsOutlastingInterval(runsSample) : undefined)
|
||||
let tag: string | undefined = $state(undefined)
|
||||
let validCRON = $state(true)
|
||||
let isValid = $state(true)
|
||||
@@ -919,6 +942,16 @@
|
||||
bind:validCRON
|
||||
bind:cronVersion
|
||||
/>
|
||||
{#if outlastingMs && runsSample?.interval_s}
|
||||
<Alert type="warning" size="xs" title="Runs are outlasting the interval">
|
||||
Recent runs have been taking about {msToReadableTimeShort(outlastingMs, 0)}, against {msToReadableTime(
|
||||
runsSample.interval_s * 1000
|
||||
)} between slots. Script runs never overlap, so the next run is only queued once the
|
||||
previous one has completed: this schedule is running less often than its cron asks
|
||||
for. To keep the cadence, schedule a flow instead, which queues its next run when the
|
||||
previous one starts.
|
||||
</Alert>
|
||||
{/if}
|
||||
<div class="flex flex-col gap-1">
|
||||
<Toggle
|
||||
options={{
|
||||
|
||||
@@ -6,14 +6,7 @@
|
||||
type WorkspaceDeployUISettings,
|
||||
WorkspaceService
|
||||
} from '$lib/gen'
|
||||
import {
|
||||
canWrite,
|
||||
displayDate,
|
||||
getLocalSetting,
|
||||
msToReadableTime,
|
||||
storeLocalSetting
|
||||
} from '$lib/utils'
|
||||
import { scheduleOutlastsItsInterval } from '$lib/components/schedules/scheduleDrift'
|
||||
import { canWrite, displayDate, getLocalSetting, storeLocalSetting } from '$lib/utils'
|
||||
import { withForkConflictRetry } from '$lib/utils/forkConflict'
|
||||
import { base } from '$app/paths'
|
||||
import CenteredPage from '$lib/components/CenteredPage.svelte'
|
||||
@@ -212,6 +205,13 @@
|
||||
})
|
||||
let scheduleEditor: ScheduleEditor | undefined = $state()
|
||||
|
||||
// The drawer shows how a schedule is actually running; the rows here already
|
||||
// carry its recent runs, so it is handed those rather than fetching its own.
|
||||
function getRunsSample(path: string) {
|
||||
const row = schedules.find((s) => s.path === path)
|
||||
return row && { ...row, jobs: row.jobs }
|
||||
}
|
||||
|
||||
// Deep link: #<path> opens that schedule's edit drawer. Tracks the last
|
||||
// handled hash (not a one-shot flag) so a hash change on the already-mounted
|
||||
// page (e.g. the AI session preview re-pointing its tab) opens the drawer
|
||||
@@ -342,7 +342,7 @@
|
||||
</script>
|
||||
|
||||
<DeployWorkspaceDrawer bind:this={deploymentDrawer} />
|
||||
<ScheduleEditor onUpdate={loadSchedules} bind:this={scheduleEditor} />
|
||||
<ScheduleEditor onUpdate={loadSchedules} {getRunsSample} bind:this={scheduleEditor} />
|
||||
|
||||
{#if $userStore?.operator && $workspaceStore && !$userWorkspaces.find((_) => _.id === $workspaceStore)?.operator_settings?.schedules}
|
||||
<div class="bg-red-100 border-l-4 border-red-600 text-orange-700 p-4 m-4 mt-12" role="alert">
|
||||
@@ -410,19 +410,13 @@
|
||||
{/if}
|
||||
{:else if items?.length}
|
||||
<div class="border rounded-md divide-y">
|
||||
{#each items.slice(0, nbDisplayed) as { path, error, summary, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, extra_perms, canWrite, jobs, interval_s, queues_next_run_at_start, paused_until, labels, inherited_labels, draft_only, is_draft } (path)}
|
||||
{#each items.slice(0, nbDisplayed) as { path, error, summary, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, extra_perms, canWrite, jobs, paused_until, labels, inherited_labels, draft_only, is_draft } (path)}
|
||||
{@const hasDraft =
|
||||
getLocalDraftHint($workspaceStore, 'trigger_schedule', path) ?? is_draft}
|
||||
{@const href = `${is_flow ? '/flows/get' : '/scripts/get'}/${script_path}`}
|
||||
{@const avg_s = jobs
|
||||
? jobs.reduce((acc, x) => acc + x.duration_ms, 0) / jobs.length
|
||||
: undefined}
|
||||
{@const outlastsInterval = scheduleOutlastsItsInterval({
|
||||
queues_next_run_at_start,
|
||||
enabled,
|
||||
interval_s,
|
||||
jobs
|
||||
})}
|
||||
|
||||
<div
|
||||
class="bg-surface-tertiary hover:bg-surface-hover w-full items-center px-4 py-2 gap-4 first-of-type:!border-t-0
|
||||
@@ -482,20 +476,6 @@
|
||||
<div class="gap-2 items-center hidden md:flex">
|
||||
<Badge large color="blue">{schedule}</Badge>
|
||||
<Badge small color="gray">{timezone}</Badge>
|
||||
{#if outlastsInterval}
|
||||
<Popover notClickable>
|
||||
<Badge small color="yellow">runs outlast the interval</Badge>
|
||||
{#snippet text()}
|
||||
<div>
|
||||
Recent runs take longer than the {msToReadableTime(interval_s! * 1000)} between
|
||||
slots. Script runs never overlap, so the next run is only queued once the
|
||||
previous one has completed, and this schedule is running less often than its
|
||||
cron asks for. To keep the cadence, schedule a flow instead: a flow queues its
|
||||
next run when the previous one starts.
|
||||
</div>
|
||||
{/snippet}
|
||||
</Popover>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
<div class="hidden lg:flex flex-row gap-1 items-center">
|
||||
|
||||
Reference in New Issue
Block a user