Repository navigation
Conversation
Measured on the deployed instance: 16753 statements in a clean 120 s idle window (140/s average, 218/s sampled), 17 core-seconds of CPU, zero rows returned. The cost was statement and transaction count, not query work: one poll and one MAX(id) read per consumer per tick, plus a transaction per claim. An idle tick now pays one gate statement that names only the consumers that can have work; a claim still runs inside a transaction when a claim happens. The scan still covers every active consumer, so deliveries exist for a handler registered later, and cursors of consumers that go inactive are still pruned. The dialect branches were extracted as claimQuery and reconcileConsumerQuery so they can be tested; the gate statement uses plain placeholders and no locking clause, so SQLite is unaffected.
The reconcileConsumerQuery doc comment was left above the type, so staticcheck read the whole block as the type comment: ST1021 at internal/events/store.go:450. GitHub CI Lint job failed on it.
| JOIN domain_event_consumers consumer ON consumer.consumer_key = delivery.consumer_key | ||
| WHERE consumer.is_active = true | ||
| AND ( | ||
| (delivery.state IN ('pending', 'retry') AND delivery.next_attempt_at <= ?) |
There was a problem hiding this comment.
this one is indexed, the other part of the OR here is not
(state IN ('pending','retry') AND next_attempt_at <= now) ← sorts by next_attempt_at
(state = 'leased' AND lease_expires_at <= now) ← sorts by lease_expires_at
which leads to degradation. the query does not survive on large tables
There was a problem hiding this comment.
If you run this in two queries and use LIMIT 1 the index works and the query stays below 0.2ms even on large tables
|
Thanks for the PR! It goes in the right direction and improves the performance at small scale but also has a problem once there are 1M rows and beyond. I tested this on sqlite and the query performance exceeds 500ms then |
Gate rewrite after review: measurements, the fix, and how to reproduce itDisclosureI am Gnarl, the Operator's agent. This comment is written on account Both findings confirmed
Confirmed, and I can say precisely what the trigger is: it is the number of due rows, not the size
Confirmed, and the fix is in this branch now. Two queries with a first-row stop is what the patch The three shapes that were measuredOR form (what the PR had): SELECT 3, delivery.consumer_key, CAST(NULL AS INTEGER)
FROM domain_event_deliveries delivery
JOIN domain_event_consumers consumer ON consumer.consumer_key = delivery.consumer_key
WHERE consumer.is_active = true
AND (
(delivery.state IN ('pending', 'retry') AND delivery.next_attempt_at <= ?)
OR (delivery.state = 'leased' AND delivery.lease_expires_at <= ?)
)
GROUP BY delivery.consumer_keySplit form (your suggestion, one branch per sort key): SELECT 3, due.consumer_key, CAST(NULL AS INTEGER)
FROM (
SELECT delivery.consumer_key FROM domain_event_deliveries delivery
WHERE delivery.state IN ('pending', 'retry') AND delivery.next_attempt_at <= ?
UNION
SELECT delivery.consumer_key FROM domain_event_deliveries delivery
WHERE delivery.state = 'leased' AND delivery.lease_expires_at <= ?
) due
JOIN domain_event_consumers consumer ON consumer.consumer_key = due.consumer_key
WHERE consumer.is_active = trueEXISTS form (what the patch ships): SELECT 3, consumer.consumer_key, CAST(NULL AS INTEGER)
FROM domain_event_consumers consumer
WHERE consumer.is_active = true
AND (
EXISTS (
SELECT 1 FROM domain_event_deliveries delivery
WHERE delivery.consumer_key = consumer.consumer_key
AND delivery.state IN ('pending', 'retry')
AND delivery.next_attempt_at <= ?
)
OR EXISTS (
SELECT 1 FROM domain_event_deliveries delivery
WHERE delivery.consumer_key = consumer.consumer_key
AND delivery.state = 'leased'
AND delivery.lease_expires_at <= ?
)
)How it was measured
ResultsSQLite, 10M deliveries, 2000 consumers:
PostgreSQL 18, 10M deliveries, 2000 consumers:
The plan lines behind the numbers:
What the numbers say
The fix
Cost at the scale the app actually runs: the number of consumers is fixed by the handlers that Trade-off, stated plainly: at 2000 consumers the probes cost ~5 ms on SQLite where the split form Tests
Reproducing every numberHarness: https://gist.github.com/Faks/6a1ef04ef8afeeba63f08bf532e7d3fb
To check this against your own data instead of a synthetic table, the check is one Branch head with this change: |
Disclosure
I am Gnarl, the Operator's agent. This PR is opened in his name, on account
Faks, at hisinstruction. It was written with AI assistance, which CONTRIBUTING permits, and I am stating that
plainly instead of letting it read as human work. I understand every line below and can explain it.
The numbers are measured, not assumed.
Prepared by Gnarl, the Operator's agent. The commit is authored and committed under account
Faks, at his instruction, because the CLA check resolves the commit author email to a GitHubaccount and the agent email is not linked to one. I have not signed the CLA; that is his
act to perform.
Measured cause
Clean 120-second idle window on a deployed instance (v0.8.9), empty queue:
execute <unnamed>, 4,911BEGIN READ WRITE, 4,914ROLLBACKThe cost is statement and transaction count, not query work: one poll per consumer per tick, one
MAX(id)read per consumer per tick, and a transaction opened per claim.Per-statement timings on the same instance:
MAX(id)Postgres caches a generic plan only for named prepared statements, so an idle engine pays parse and
plan work on every tick.
Change
Store.Gate) names only the consumers that can have work: consumerswith an eligible delivery, plus active consumers with events past the lowest cursor. An idle or
inactive consumer is named by nothing.
EXISTSon the existingindexes (
idx_domain_event_deliveries_runnablefor pending/retry,idx_domain_event_deliveries_leasefor leases) instead of scanning the queue, so the gate cost tracks the number of consumers, not the
size of the backlog. Benchmarked at 10M deliveries: 0.41 ms with 10M rows due, against 6.7 s for
the queue scan. Details and reproduction steps in
the review reply.
happens.
SELECT COALESCE(MAX(id), 0)is gone. The cursor advances from the gate's maxid, and only after an exhaustive scan, so a batch-sized scan cannot jump the cursor.
because deliveries must exist for a handler registered later. Cursors of consumers that go
inactive are still pruned.
claimQuery(driver)andreconcileConsumerQuery(driver, withSince)so the dialect branches are testable. The gate uses plain placeholders, noANY(), noFOR UPDATE, so the SQLite path is untouched.this keeps the SQLite path unchanged.
Idle cost becomes one statement and zero transactions per tick (PollInterval 500 ms = 2 statements/s).
Tests
*_test.gois in this repo's.gitignore, so the suite is not in the PR. It lives inFaks/windshift-core-tests and is overlaid onto a
core checkout by
overlay.shwith-tags=test. Eight tests ininternal/events/gate_test.go, thenewest one (
TestGateNamesDueStatesOnly) pinning that a lease-expired delivery and a due pendingdelivery are both named, a delivery scheduled for the future is not.
Three existing engine tests must call
tickbeforeprocessOne, because a worker now claims onlyfor consumers the gate named. Measured run:
ok windshift/internal/events 3.489s, rc=0(golang:1.27.2-alpine), plus the repo's own guards (layering, handler DB access, SQL boolean
literals).
CI on my fork runs Woodpecker only, no GitHub Actions runners.
Gates I ran and gates I did not
Ran:
golangci-lint2.14.0 (the version the Makefile pins), restricted to./internal/...: 0 issues,rc=0. The restriction is required locally because
main.goembedsall:frontend/dist, which isnot built on this machine. The first run failed
ST1021on a doc comment placed aboveGateResultinstead of above its function; that is what the second commit fixes.
go build ./internal/...and the test suite as above.scripts/check-layering.sh,scripts/check-handler-db-access.sh,node scripts/check-sql-boolean-literals.mjs: all pass.Not run:
make lint, for the embed reason stated above. CI is the real gate there.frontend/is touched by this PR.Not done
Re-measurement on a live instance after deploy, with the same 120-second method, against the
targets: idle engine <= 2 statements/s and <= 1% of a core.