Files
windmill/backend/tests/debounce_e2e.rs
Ruben Fiszel 5549bdc67a fix(debounce): never supersede a running debounce survivor (#9780)
* 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>
2026-06-25 16:36:16 +00:00

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(())
}
}