fix: atomic AND-join gate + preserve resolved partition; drop scratch artifacts

Addresses local-review findings before PR:
- P1: record_and_check_join_slot was a non-atomic check-then-act on a
  pooled connection; concurrent completion of a subscriber's last two
  partition-bearing inputs on different workers could double-dispatch.
  Now one transaction guarded by a tx-scoped advisory lock keyed on
  (workspace, subscriber, partition) so the gate fires exactly once.
- P2: the preprocessed-args overwrite in result_processor replaced args
  wholesale, dropping a partition resolved by resolve_partition_for_job;
  the UPDATE now preserves an existing persisted partition key.
- P2: gate resolve_partition_for_job on a cheap code.contains check so
  non-pipeline script jobs skip the annotation scan on the hot path.
- P2: remove 40 scratch screenshot PNGs, a flicker-debug script and a
  local scheduler lock accidentally committed; gitignore the lock.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-05-17 08:13:29 +00:00
parent 99e81c5b50
commit 19a386ce55
49 changed files with 104 additions and 165 deletions
-1
View File
@@ -1 +0,0 @@
{"sessionId":"2cfbff54-4fc8-4390-ab4d-83b737f1fb6b","pid":59389,"procStart":"864605","acquiredAt":1778411865080}
+2 -1
View File
@@ -33,4 +33,5 @@ backend/chrome_profiler.json
.fast-check/
__pycache__/
.playwright-mcp/
.codex
.codex
.claude/scheduled_tasks.lock
BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 194 KiB

@@ -0,0 +1,15 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE v2_job\n SET args = CASE\n WHEN args ? 'partition'\n THEN $1 || jsonb_build_object('partition', args -> 'partition')\n ELSE $1\n END,\n preprocessed = TRUE\n WHERE id = $2",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Jsonb",
"Uuid"
]
},
"nullable": []
},
"hash": "651fc12e1b971d4fd57c98a7a7efbd503d8dea799545e9cb96574d5c6020b90b"
}
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "pg_advisory_xact_lock",
"type_info": "Void"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
null
]
},
"hash": "751f836dc8f78c330387456dd68a8803972c7b3e2b6a2b95c27f15068bed2ca5"
}
@@ -1,15 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE v2_job SET args = $1, preprocessed = TRUE WHERE id = $2",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Jsonb",
"Uuid"
]
},
"nullable": []
},
"hash": "e3ee812acd5bb9d5af39ca7dc61481ddd56a641e8e6f6c7a145f2cd5f3dc4602"
}
+47 -42
View File
@@ -351,16 +351,49 @@ fn is_partition_bearing_ref(trigger_ref: &str) -> bool {
trigger_ref.contains(PARTITION_TOKEN)
}
/// Distinct partition-bearing asset inputs an AND subscriber declares
/// (its `{partition}`-token `// on` lines). Reference inputs (no token)
/// and non-asset triggers are presence-only and do not gate the join in
/// v1, so they are excluded from the required set.
async fn count_required_join_inputs(
/// Record an AND input arrival for `(subscriber, partition)` and report
/// whether every partition-bearing input is now present for that
/// partition. The record -> count -> clear sequence runs in one
/// transaction guarded by a transaction-scoped advisory lock keyed on the
/// slot: the same subscriber's last two partition-bearing inputs can
/// complete concurrently on different workers, and a check-then-act on a
/// pooled connection would let both observe "complete" and dispatch
/// twice. Serializing per `(workspace, subscriber, partition)` makes the
/// gate fire exactly once; the lock is released on commit/rollback.
/// Idempotent per input (PK conflict ignored); the slot is cleared on
/// fire so later writes re-accumulate and can re-materialize.
///
/// `required` = the distinct partition-bearing inputs the subscriber
/// declares (its `{partition}`-token `// on` lines). Reference inputs
/// (no token) and non-asset triggers are presence-only and excluded.
async fn record_and_check_join_slot(
db: &DB,
workspace_id: &str,
subscriber_path: &str,
) -> Result<i64> {
let n = sqlx::query_scalar!(
partition: &str,
trigger_ref: &str,
) -> Result<bool> {
let mut tx = db.begin().await?;
let lock_key = format!("{workspace_id}|{subscriber_path}|{partition}");
sqlx::query!(
"SELECT pg_advisory_xact_lock(hashtextextended($1, 0))",
lock_key,
)
.execute(&mut *tx)
.await?;
sqlx::query!(
r#"INSERT INTO join_pending_inputs
(workspace_id, subscriber_path, partition, trigger_ref)
VALUES ($1, $2, $3, $4)
ON CONFLICT DO NOTHING"#,
workspace_id,
subscriber_path,
partition,
trigger_ref,
)
.execute(&mut *tx)
.await?;
let required = sqlx::query_scalar!(
r#"SELECT count(DISTINCT trigger_ref) AS "n!"
FROM script_trigger
WHERE workspace_id = $1
@@ -372,36 +405,8 @@ async fn count_required_join_inputs(
subscriber_path,
PARTITION_TOKEN,
)
.fetch_one(db)
.fetch_one(&mut *tx)
.await?;
Ok(n)
}
/// Record an AND input arrival for `(subscriber, partition)` and report
/// whether every partition-bearing input is now present for that
/// partition. Idempotent per input (PK conflict ignored), so a re-fired
/// upstream doesn't double-count. On completion the slot is cleared so
/// later writes re-accumulate and can re-materialize the partition.
async fn record_and_check_join_slot(
db: &DB,
workspace_id: &str,
subscriber_path: &str,
partition: &str,
trigger_ref: &str,
) -> Result<bool> {
sqlx::query!(
r#"INSERT INTO join_pending_inputs
(workspace_id, subscriber_path, partition, trigger_ref)
VALUES ($1, $2, $3, $4)
ON CONFLICT DO NOTHING"#,
workspace_id,
subscriber_path,
partition,
trigger_ref,
)
.execute(db)
.await?;
let required = count_required_join_inputs(db, workspace_id, subscriber_path).await?;
let received = sqlx::query_scalar!(
r#"SELECT count(DISTINCT trigger_ref) AS "n!"
FROM join_pending_inputs
@@ -410,9 +415,10 @@ async fn record_and_check_join_slot(
subscriber_path,
partition,
)
.fetch_one(db)
.fetch_one(&mut *tx)
.await?;
if required > 0 && received >= required {
let fire = required > 0 && received >= required;
if fire {
sqlx::query!(
r#"DELETE FROM join_pending_inputs
WHERE workspace_id = $1 AND subscriber_path = $2 AND partition = $3"#,
@@ -420,12 +426,11 @@ async fn record_and_check_join_slot(
subscriber_path,
partition,
)
.execute(db)
.execute(&mut *tx)
.await?;
Ok(true)
} else {
Ok(false)
}
tx.commit().await?;
Ok(fire)
}
async fn push_subscriber(
@@ -763,9 +763,20 @@ pub async fn process_completed_job(
))
})?;
} else if let Some(preprocessed_args) = preprocessed_args {
// Update script args to preprocessed args
// Update script args to preprocessed args, but preserve a
// resolved pipeline `partition` (injected before the body ran
// by resolve_partition_for_job). Run identity is immutable —
// the preprocessor must not change or drop it, or the asset
// cascade would read no partition for this producer.
sqlx::query!(
"UPDATE v2_job SET args = $1, preprocessed = TRUE WHERE id = $2",
"UPDATE v2_job
SET args = CASE
WHEN args ? 'partition'
THEN $1 || jsonb_build_object('partition', args -> 'partition')
ELSE $1
END,
preprocessed = TRUE
WHERE id = $2",
Json(preprocessed_args) as Json<HashMap<String, Box<RawValue>>>,
job.id
)
+5 -2
View File
@@ -4193,8 +4193,11 @@ async fn resolve_partition_for_job(
use windmill_common::partition::{resolve_partition, PARTITION_ARG};
use windmill_parser::asset_parser::PartitionKind;
// Only deployed scripts participate in asset pipelines.
if !matches!(job.kind, JobKind::Script) {
// Only deployed scripts participate in asset pipelines. Cheap
// substring guard so the overwhelming majority of script jobs (no
// `// partitioned` line) skip the full annotation scan on the hot
// path; a false positive only costs one extra parse, never wrong.
if !matches!(job.kind, JobKind::Script) || !code.contains("partitioned") {
return Ok(None);
}
let Some(spec) = windmill_parser::asset_parser::parse_pipeline_annotations(code).partition
BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 188 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 181 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 110 KiB

-102
View File
@@ -1,102 +0,0 @@
// One-off repro for the asset-runs panel re-fetch flicker. Logs in, opens
// /pipeline/km, clicks the first s3 asset node, and counts list_jobs
// requests over a 10 s idle window. > 2 == reactivity bug still present.
import { chromium } from 'playwright'
const baseURL = process.env.BASE_URL ?? 'http://localhost:3000'
const folder = process.env.PIPELINE_FOLDER ?? 'km'
const browser = await chromium.launch({ headless: true })
const ctx = await browser.newContext()
const page = await ctx.newPage()
// Counters
const counters = { listJobs: 0, others: new Map() }
page.on('request', (req) => {
const u = req.url()
if (u.includes('/list_jobs') || u.includes('/concurrency_groups/list_jobs')) {
counters.listJobs++
console.log(`[${new Date().toISOString().slice(11, 23)}] LIST_JOBS #${counters.listJobs} ${u}`)
}
})
// Console capture
page.on('console', (m) => {
const t = m.type()
if (t === 'error' || t === 'warning') console.log(`[console.${t}]`, m.text())
})
console.log('→ navigate to', baseURL)
await page.goto(baseURL)
// Login
await page.locator('input#email[type="email"]').fill('admin@windmill.dev')
await page.locator('input#password[type="password"]').fill('changeme')
await page.locator('button:has-text("Sign in")').click()
await page.waitForURL(/\/user\/(first-time|workspaces|$)|\/$/, { timeout: 15000 })
// First-time skip / workspace pick
if (page.url().includes('/user/first-time')) {
await page.locator('button:has-text("Skip")').click()
}
if (page.url().includes('/user/workspaces')) {
const ws = page.locator('text=Admins').first()
await ws.waitFor({ state: 'visible' })
await ws.click()
await page.waitForURL(/\/$/)
}
console.log('→ navigate to /pipeline/' + folder)
await page.goto(`${baseURL}/pipeline/${folder}`)
await page.waitForLoadState('networkidle', { timeout: 10000 }).catch(() => {})
// Try to click the first asset node on the canvas. Asset nodes use
// AssetGenericIcon; a stable signal is the formatted-asset-path text rendered
// inside the node. Fallback: click any element with class containing 'asset'.
console.log('→ wait for canvas')
await page
.locator('.svelte-flow__node-asset')
.first()
.waitFor({ state: 'visible', timeout: 15000 })
const beforeClick = counters.listJobs
console.log(`baseline list_jobs before asset click: ${beforeClick}`)
console.log('→ click first asset node')
await page.locator('.svelte-flow__node-asset').first().click()
// Wait briefly for the runs panel to mount and fire its initial fetch.
await page.waitForTimeout(1500)
console.log(`list_jobs after click + 1.5s: ${counters.listJobs}`)
const initial = counters.listJobs
// Idle window — no clicks, no input. Anything that fires here is the bug.
const idleSeconds = 10
console.log(`→ idle for ${idleSeconds}s`)
const tickStart = counters.listJobs
const start = Date.now()
while (Date.now() - start < idleSeconds * 1000) {
await page.waitForTimeout(500)
}
const tickEnd = counters.listJobs
console.log(`list_jobs during idle: ${tickEnd - tickStart} (total: ${tickEnd})`)
// Try typing in the editor pane (if present) — this is what was triggering
// the flicker via graphWithDraft re-derivation in earlier reports. We only
// want to verify the fix protects against it.
const editor = page.locator('.monaco-editor').first()
if (await editor.isVisible().catch(() => false)) {
console.log('→ focus + type a couple chars to test draft churn')
await editor.click()
const beforeType = counters.listJobs
for (let i = 0; i < 6; i++) {
await page.keyboard.type('x')
await page.waitForTimeout(200)
}
console.log(`list_jobs while typing: ${counters.listJobs - beforeType}`)
}
console.log('=== final tally ===')
console.log({ total_list_jobs: counters.listJobs })
await browser.close()
BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 112 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 108 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 108 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 107 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 107 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 94 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 86 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 99 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 78 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 194 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 66 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 6.0 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 46 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 110 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 110 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 62 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 42 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 194 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 194 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 194 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 87 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 64 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 72 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 68 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 180 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 865 B

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 180 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 180 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 181 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 180 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 519 B

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 21 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 2.2 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 176 KiB

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 976 B

BIN
View File
Binary file not shown.

Before

Width:  |  Height:  |  Size: 27 KiB