mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-21 00:02:23 +00:00
5549bdc67a
* fix(debounce): never supersede a running debounce survivor
Companion to the windmill-ee-private change in upsert_debounce_key.
With debounce_args_to_accumulate + a concurrent_limit, a message arriving
while its debounce survivor is already running was marked completed/skipped
("Debounced Running by ...") and the running survivor deleted from the
queue, silently dropping accumulated elements. A slow step + concurrent
limit keeps the survivor running for a long window, so any arrival during
it was lost. The fix leaves a running survivor untouched and starts a fresh
debounce window for the late arrival.
Adds regression coverage in windmill-queue/tests/debounce_test.rs (push,
flow post-preprocessing, no-accumulation, committed-running, and
max-count-window cases) and refreshes the SQLx cache for the changed
upsert_debounce_key queries.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(debounce): add missing SQLx cache for test-only running-flag query
The cargo_test CI job compiles the test target with SQLX_OFFLINE=true; the
new regression tests use `UPDATE v2_job_queue SET running = true ...` which
was not in the offline cache (the library-only `cargo sqlx prepare` skipped
test targets). check_oss/check_ee passed because they don't build tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(debounce): harden running-survivor guard against concurrent arrivals
Companion to windmill-ee-private: switch the running-state check to a
correlated EXISTS on the post-conflict-lock holder so two late arrivals
racing after a survivor started running can't both spawn independent
windows (the row lock serializes them; the second debounces into the
first's fresh window).
Adds a concurrent regression test
(test_debounce_concurrent_arrivals_after_running_survivor) asserting
exactly one late arrival survives and the other is debounced, and refreshes
the SQLx cache for the updated upsert_debounce_key queries.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(debounce): serialize upsert per key (simpler, race-free)
Companion to windmill-ee-private: the running-survivor guard and batch
chaining are now protected by a per-key advisory lock instead of
snapshot-sensitive single-statement SQL. This closes a concurrent-arrival
data-loss race where a debounced late arrival's args could be dropped
because the batch lookup couldn't see the predecessor's just-committed
batch row.
Extends test_debounce_concurrent_arrivals_after_running_survivor to pull the
survivor and assert its accumulation includes BOTH racing late arrivals
(shared batch), and refreshes the SQLx cache for the rewritten queries.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(debounce): atomic upsert robust to concurrent pull-time key deletion
Companion to windmill-ee-private: keep upsert_debounce_key a single atomic
INSERT ... ON CONFLICT DO UPDATE so a chaining push cannot fail when the
worker pull path concurrently deletes the holder's debounce_key (the prior
read+UPDATE split could hit "no row updated"). Adds
test_debounce_push_races_key_deletion_by_pull (races a chaining push against
the key deletion 50x, asserts the push never errors) and refreshes the SQLx
cache.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(debounce): claim-based exactly-once batch consumption
Eliminates the rare duplicate/loss when two survivors land on one debounce
batch (a narrow push/pull race), without locking the worker pull hot path.
- migration: v2_job_debounce_batch gains consumed_at + consumed_by.
- pull side (maybe_apply_debouncing): instead of deleting the batch on consume,
a survivor atomically claims its own row + any unclaimed siblings (stamping
consumed_by = itself) and accumulates exactly the rows it claimed. A second
survivor of the same batch finds its row already consumed by another job and
runs empty (no duplicate); a re-pulled survivor recognizes its own prior claim
and keeps its accumulated args; a never-batched job (CE/legacy) keeps its own
args. Non-accumulate debounce paths still hard-delete their batch rows.
- complete_debounced_job (EE companion) never completes a running predecessor,
so its in-flight run is not killed (no loss); the claim then prevents the
duplicate the guard would otherwise allow.
- monitor: GC sweep deletes consumed batch rows past a 1h grace.
Together with the running-survivor guard this makes debounce accumulation
exactly-once. Adds tests: batch_consumed_exactly_once, repull_keeps_accumulated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(debounce): exhaustive edge cases + tighten consumed-batch GC grace
Tighten the consumed debounce-batch GC grace 1h -> 10min: per-op cost of the
claim is unchanged (an indexed mark is as cheap as the old delete), so the only
cost of retaining consumed rows is table growth, which a shorter grace bounds
under high-throughput debounce (a survivor that could still reference a row is
pulled long before 10min; GC is not correctness-critical since a re-pull whose
row was swept falls back to its persisted args).
Adds edge-case tests: never-batched keeps own args (CE fallback), concurrent
claim partitions a batch disjointly (exactly-once under real concurrency),
three survivors -> first takes all / rest run empty, non-accumulate debounce
hard-deletes its batch rows (no leak), and the GC sweep deletes only
past-grace consumed rows.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(debounce): port the #9781 regression case, flow-node guard, full-path bench
- Port the regression from #9781
(test_post_preprocessing_debounce_into_running_survivor_loses_message):
post-preprocessing survivor accumulates + runs, a later same-key message must
start a new batch (survive) not be folded into the running survivor. Exercises
the full EE path via jobs_ee::maybe_debounce_post_preprocessing.
- Add the third EE entry point's guard:
test_flow_node_debounce_running_survivor_not_superseded (maybe_debounce_flow_node).
- Add an #[ignore] full-source throughput bench (bench_debounce_full_path) driving
the real maybe_debounce + maybe_apply_debouncing end-to-end.
All debounce tests exercise the real jobs_ee implementation (run with
--features private,enterprise); none stub it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(debounce): scalar-arg accumulation + GC-then-repull no-loss
Close two accumulation edge gaps (both run on --features private,enterprise,
exercising the real jobs_ee path):
- accumulate bare-scalar values (the T | T[] union fallback): each scalar is
wrapped and accumulated into the survivor's list.
- GC reclaiming a survivor's consumed batch row before a re-pull must not lose
data: the re-pull finds no row and keeps its already-persisted accumulated
args (had_row=false fallback), rather than running empty.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(debounce): real-worker end-to-end accumulation test
Drives the full real path on --features enterprise,deno_core,private: push 3
same-key debounced flow jobs (real push() -> maybe_debounce collapses the
batch), a real worker pulls the survivor (real pull() -> maybe_apply_debouncing
claim+accumulate) and executes the deno flow, then asserts the executed result
is the full accumulated set [1,2,3] and the two superseded messages are skipped.
Complements the in-process unit tests with a genuine worker-execution run.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(debounce): atomic claim+persist, GC only non-queued rows; reword comment
Address review findings:
- [P1] Claim and accumulated-args persist are now in one transaction. Before,
a crash between stamping batch rows consumed_by=self and the `UPDATE v2_job
SET args` could let a zombie re-pull see its own prior claim and keep only its
own args (dropping the siblings it had claimed). Wrapping claim + accumulate +
persist in a tx makes them commit together or roll back together (re-pull then
re-claims cleanly).
- [P1] GC of consumed batch rows now also requires the job to no longer be in
v2_job_queue. A consumed sibling can stay queued well past any time grace under
a concurrency limit / backlog; reclaiming its marker by age alone let its
eventual pull treat it as never-batched and re-run its item (a duplicate).
Keeping the row until the job leaves the queue preserves the "already consumed"
signal. Test extended with a still-queued consumed row that must survive GC.
- [P2] Drop "Customer" attribution from a test doc comment (AGENTS.md).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(debounce): emit accumulation log after committing the claim transaction
append_logs opened a second pool connection while the claim transaction (and its
batch row locks) were still held; under concurrent debounced pulls that risks
pool-exhaustion stalls/timeouts. Defer the log line until after tx.commit().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: update ee-repo-ref to 6aabd7c5ce53b9153be05c3e7bc9a76eadb1a48a
This commit updates the EE repository reference after PR #631 was merged in windmill-ee-private.
Previous ee-repo-ref: 30d740e619fad219108ec4b4c6a9d67c1ab42d46
New ee-repo-ref: 6aabd7c5ce53b9153be05c3e7bc9a76eadb1a48a
Automated by sync-ee-ref workflow.
* fix(debounce): claim whole batch in one UPDATE (no deadlock); assert test setup
Both Codex (P1) and Claude (P2) flagged a deadlock: the claim used two writable
CTEs (claim_self then claim_rest), locking the self row before siblings, so two
survivors of the same batch pulled concurrently acquired row locks in opposite
order and PostgreSQL aborted one with deadlock_detected (a transient pull error
on exactly the two-survivors race this path handles).
Replace with a single `UPDATE ... WHERE debounce_batch = (...) AND consumed_at IS
NULL RETURNING id` that claims the whole batch: both transactions lock rows in
the same scan order, so one simply waits and re-evaluates under EvalPlanQual.
A `claimed_self` flag (EXISTS id = self in the claimed set) plus the `mine`
snapshot still distinguishes fresh-claim / consumed-by-other / own-re-pull.
Also assert add_survivor_to_batch_of actually inserts a row (rows_affected == 1)
so a mis-set-up test can't pass vacuously.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
113 lines
4.3 KiB
Rust
113 lines
4.3 KiB
Rust
// End-to-end debounce test: drives the FULL real path — real `push()` (EE
|
|
// `maybe_debounce` collapsing the batch), real `pull()`, real
|
|
// `maybe_apply_debouncing` (claim + accumulate), and a real worker executing the
|
|
// surviving flow — then asserts the executed result contains every accumulated item.
|
|
// Runs on --features deno_core,enterprise,private (debounce is EE/compile-gated).
|
|
#[cfg(all(feature = "deno_core", feature = "enterprise", feature = "private"))]
|
|
mod debounce_e2e {
|
|
use serde_json::json;
|
|
use sqlx::{Pool, Postgres};
|
|
use uuid::Uuid;
|
|
use windmill_common::flows::FlowValue;
|
|
use windmill_common::jobs::JobPayload;
|
|
use windmill_common::worker::Connection;
|
|
use windmill_test_utils::*;
|
|
|
|
async fn initialize_tracing() {
|
|
use std::sync::Once;
|
|
static ONCE: Once = Once::new();
|
|
ONCE.call_once(|| {
|
|
let _ = windmill_common::tracing_init::initialize_tracing(
|
|
"test",
|
|
&windmill_common::utils::Mode::Standalone,
|
|
"test",
|
|
);
|
|
});
|
|
}
|
|
|
|
/// A one-step flow that debounces on `items` and returns `flow_input.items`,
|
|
/// so the flow's result is exactly the accumulated batch.
|
|
fn debounce_flow() -> FlowValue {
|
|
serde_json::from_value(json!({
|
|
"modules": [{
|
|
"id": "a",
|
|
"value": {
|
|
"type": "rawscript",
|
|
"language": "deno",
|
|
"content": "export async function main(items: any[]) { return items }",
|
|
"input_transforms": {
|
|
"items": { "type": "javascript", "expr": "flow_input.items" }
|
|
}
|
|
}
|
|
}],
|
|
"debounce_delay_s": 1,
|
|
"debounce_key": "e2e_debounce_key",
|
|
"debounce_args_to_accumulate": ["items"]
|
|
}))
|
|
.expect("valid flow value")
|
|
}
|
|
|
|
/// Fire three same-key messages; only the survivor runs, and it must execute once
|
|
/// with ALL accumulated items (none dropped, none duplicated).
|
|
#[sqlx::test(fixtures("base"))]
|
|
async fn debounce_accumulation_runs_once_with_all_items(
|
|
db: Pool<Postgres>,
|
|
) -> anyhow::Result<()> {
|
|
initialize_tracing().await;
|
|
let server = ApiServer::start(db.clone()).await?;
|
|
let port = server.addr.port();
|
|
|
|
// Push 3 debounced flow jobs (same key) — each collapses the previous; the last
|
|
// is the survivor. scheduled_for is ~1s out, so all 3 land before any worker pull.
|
|
let mut survivor = Uuid::nil();
|
|
let mut superseded = Vec::new();
|
|
for n in [1i64, 2, 3] {
|
|
if survivor != Uuid::nil() {
|
|
superseded.push(survivor);
|
|
}
|
|
survivor = RunJob::from(JobPayload::RawFlow {
|
|
value: debounce_flow(),
|
|
path: None,
|
|
restarted_from: None,
|
|
})
|
|
.arg("items", json!([n]))
|
|
.push(&db)
|
|
.await;
|
|
}
|
|
|
|
// Run a real worker until the survivor completes.
|
|
let listener = listen_for_completed_jobs(&db).await;
|
|
in_test_worker(Connection::Sql(db.clone()), listener.find(&survivor), port).await;
|
|
|
|
let cj = completed_job(survivor, &db).await;
|
|
assert!(cj.success, "survivor flow must succeed");
|
|
let mut result: Vec<i64> =
|
|
serde_json::from_value(cj.json_result().expect("result present"))
|
|
.expect("result is an array of numbers");
|
|
result.sort();
|
|
assert_eq!(
|
|
result,
|
|
vec![1, 2, 3],
|
|
"survivor must execute exactly once with ALL accumulated items"
|
|
);
|
|
|
|
// The two superseded messages must have been debounced (skipped), not run.
|
|
// (use a lightweight status query — skipped jobs have NULL started_at, which the
|
|
// full CompletedJob row decoder rejects)
|
|
for s in superseded {
|
|
let skipped: Option<bool> =
|
|
sqlx::query_scalar("SELECT status = 'skipped' FROM v2_job_completed WHERE id = $1")
|
|
.bind(s)
|
|
.fetch_optional(&db)
|
|
.await?;
|
|
assert_eq!(
|
|
skipped,
|
|
Some(true),
|
|
"superseded message {s} must be debounced (skipped), not executed"
|
|
);
|
|
}
|
|
|
|
Ok(())
|
|
}
|
|
}
|