the elo race that silently ate 79% of every vote
a textbook lost-update bug: a read-modify-write vote handler that lost 79% of every rating change under load
it’s the kind of bug that lets you sleep fine for weeks.
i’d built a little nougat-ranking toy — you get shown two flavours, you pick the one you’d rather eat, and an ELO rating quietly updates in the background so a leaderboard falls out of thousands of these pairwise duels. side project, single box, htmx and go and postgres, held together with database/sql and good intentions. it worked. people voted. the leaderboard moved. every vote came back with a fresh pairing and a 200. green across the board.
then, before pushing it anywhere with real traffic, i did a proper hardening pass: stand up a throwaway postgres, seed it with a plausible went-viral amount of data, and hammer every endpoint like someone who wants to break it. the very first thing i measured told me that roughly four out of every five votes had been quietly doing almost nothing the whole time.
not erroring. not failing. the vote was recorded. the rating just didn’t move the way it was supposed to.
the setup: two ratings and one vote
the whole app is one hot path. you pick a winner, and the handler has to: read both flavours’ current ratings, compute the new ELO for each, and write both back. the ELO math itself is boring and correct — standard expected-score, standard update:
internal/http/elo.go
// UpdateRatings calculates new ratings for both players after a match
// Returns (newRating1, newRating2)
func UpdateRatings(rating1, rating2 float64, player1Won bool, kFactor float64) (float64, float64) {
exp1 := CalculateExpectedScore(rating1, rating2)
exp2 := CalculateExpectedScore(rating2, rating1)
var actualScore1, actualScore2 float64
if player1Won {
actualScore1, actualScore2 = 1.0, 0.0
} else {
actualScore1, actualScore2 = 0.0, 1.0
}
newRating1 := CalculateNewRating(rating1, exp1, actualScore1, kFactor)
newRating2 := CalculateNewRating(rating2, exp2, actualScore2, kFactor)
return newRating1, newRating2
}nothing wrong here. the new rating is a pure function of the ratings you read. hold that thought, because it’s the entire bug.
the handler wraps the read and the write in a transaction, which felt responsible at the time. here’s the shape of it — i’ve cut the ninety lines of streaks and campaigns and advent bookkeeping that sit in the middle:
internal/http/handler.go
// Start transaction to prevent race conditions in concurrent votes
tx, err := h.db.Begin()
// ...
defer tx.Rollback() // Rollback if not committed
// Get both torros within transaction (ensures consistent read)
t1, err := h.torroRepo.GetTx(tx, r.Context(), p.Torro1)
// ...
t2, err := h.torroRepo.GetTx(tx, r.Context(), p.Torro2)
// ...
// Calculate new ELO ratings based on match result
new1, new2 := UpdateRatings(t1.Rating, t2.Rating, winnerId == t1.Id, K)
// ... write a Results row, bump the user's counters ...
// Update global ratings within transaction
_, err = h.torroRepo.UpdateTx(tx, r.Context(), t1.Id, new1)
// ...
_, err = h.torroRepo.UpdateTx(tx, r.Context(), t2.Id, new2)read that comment on the GetTx line — “ensures consistent read.” past me really believed that. past me thought “it’s in a transaction” was the same sentence as “it’s safe.” it is not, and the gap between those two things ate most of my data.
the bug: a read-modify-write with the lock left out
here’s GetTx, the read side, as it shipped:
internal/repository/postgres_torro.go
func (r *postgresTorroRepo) GetTx(tx *sql.Tx, ctx context.Context, id string) (*domain.Torro, error) {
row := tx.QueryRowContext(ctx,
`
SELECT "Id", "Name", "Rating", "Image", "Class"
FROM "Torrons"
WHERE "Id" = $1`,
id,
)
// ... scan into torro
}and here’s the write side — an absolute update, “set the rating to this number”:
internal/repository/postgres_torro.go
func (r *postgresTorroRepo) UpdateTx(tx *sql.Tx, ctx context.Context, id string, rating float64) (*domain.Torro, error) {
err := tx.QueryRowContext(ctx,
`
UPDATE "Torrons" SET
"Rating" = $2
WHERE "Id" = $1
RETURNING /* ... */`,
id,
rating,
).Scan( /* ... */ )
// ...
}a plain SELECT, then SET "Rating" = $2 where $2 is a value computed back in go. this is the textbook read-modify-write lost update, and it doesn’t care that it’s inside a transaction. postgres runs at READ COMMITTED ↗
by default, and a bare SELECT under READ COMMITTED takes no row lock. so two votes for the same flavour, arriving together, do this:
- vote A reads rating
1500. - vote B reads rating
1500(A hasn’t committed; even if it had, B’s plain select wouldn’t have waited). - A computes
1521, writes1521, commits. - B computes
1521from the1500it read, writes1521, commits.
two votes happened. the rating went up by twenty-one points, once. the second vote’s entire contribution was overwritten by a value calculated from a stale pre-image. the wrapping transaction changed nothing about this — both writes are perfectly valid, they just clobber. atomicity was never the problem. isolation was.
the part that should scare you
now the genuinely unsettling bit, and the reason this postmortem exists. i fired 50 concurrent votes for one winner, from a clean 1500 / 1500 baseline, and compared the result against the mathematically-correct serialized value:
test/e2e/test_concurrency.sh
# fire N concurrent votes for FX_T1 as winner
for i in $(seq 1 "$N"); do
( curl -s -o /dev/null -w '%{http_code}\n' \
-X POST "${BASE_URL}/pairings/${FX_PAIRING}/vote?id=${FX_T1}" >"$tmp/$i" ) &
done
wait
observed="$(PSQL -t -A -c "SELECT round(\"Rating\",2) FROM \"Torrons\" WHERE \"Id\"='${FX_T1}'")"
results_written="$(PSQL -t -A -c "SELECT count(*) FROM \"Results\" WHERE \"Pairing\"='${FX_PAIRING}'")"
# the mathematically-correct value if the 50 votes had serialized
expected="$(python3 -c "
K=42; r1=r2=1500.0
for _ in range($N):
e1=1/(1+10**((r2-r1)/400)); e2=1/(1+10**((r1-r2)/400))
r1,r2 = r1+K*(1-e1), r2+K*(0-e2)
print(round(r1,2))
")"
# Sanity: every vote must still be recorded (the race corrupts ratings, not row writes)
if [[ "$results_written" == "$N" ]]; then _ok "[RACE0] all $N votes recorded"; fithe winner landed at 1555.78. serialized-correct was 1771.07. the rating was supposed to move 271 points and it moved 56 — ~79% of the signal, silently gone.
and RACE0, the sanity check, passed. all 50 Results rows were written. the per-user vote counts were exactly right. if you’d looked at the database the way a human looks at a database — are the rows there? do the counts add up? — everything was immaculate. the audit trail was pristine. the thing the audit trail was supposed to be protecting was corrupt.
that’s the trap. “the rows are all there” is not “the data is correct.” a lost update leaves no gap, no null, no error log, no failed request. every individual vote looks like it worked because, from its own point of view, it did work — it read a number, did honest arithmetic, wrote an honest result. the corruption only exists in the space between the votes, and nothing in the schema is watching that space. i shipped this. it ran. i had no idea, because there was nothing to have an idea about.
this is also, painfully, a bug that a single integration test would have caught — spin up a real postgres, fire concurrent writes, assert the ratings converge. i even had the harness to do it. i’ve written a whole series about testing repositories against a real postgres , and i still didn’t point one at the one path where correctness actually lived. write the test for the thing that would ruin you.
the fix: FOR UPDATE, in an order you choose
the fix is two words. the read has to take a lock:
internal/repository/postgres_torro.go
SELECT "Id", "Name", "Rating", "Image", "Class"
FROM "Torrons"
WHERE "Id" = $1
FOR UPDATE`,SELECT ... FOR UPDATE ↗
locks the rows it returns “as though for update… other transactions that attempt UPDATE, DELETE, SELECT FOR UPDATE… will be blocked until the current transaction ends.” now vote B’s read waits for vote A to commit, then reads 1521, and computes 1542 from the truth. the read-modify-write serializes. no clobber.
but a lock you take on two rows has a second edge: deadlock. if vote A locks flavour X then Y, and vote B — the reverse duel of the same pair — locks Y then X, they wait on each other forever until postgres kills one. the answer is to always grab the locks in the same order, regardless of which flavour “won”:
internal/http/handler.go
// Read + lock both torró rows FOR UPDATE (GetTx takes a row lock) so
// concurrent votes on the same torró serialize their read-modify-write of
// "Rating" instead of clobbering each other (lost update). The two rows are
// locked in a deterministic order (sorted by id) so two votes touching the
// same pair can never deadlock by acquiring the locks in opposite orders.
var t1, t2 *domain.Torro
if p.Torro1 <= p.Torro2 {
if t1, err = h.torroRepo.GetTx(tx, r.Context(), p.Torro1); err == nil {
t2, err = h.torroRepo.GetTx(tx, r.Context(), p.Torro2)
}
} else {
if t2, err = h.torroRepo.GetTx(tx, r.Context(), p.Torro2); err == nil {
t1, err = h.torroRepo.GetTx(tx, r.Context(), p.Torro1)
}
}sort by id, lock in that order, every time. t1 still means “the first flavour in the pairing” for the rest of the handler — only the lock acquisition order changed. deterministic lock ordering is one of those disciplines that costs three lines and a comment and saves you a 3am incident you’ll never be able to reproduce.
(the personalized per-user ELO, in a separate table, had the exact same read-modify-write and got the exact same FOR UPDATE. lower blast radius — it needs the same user voting concurrently — but the same bug is the same bug.)
after: 50 concurrent votes land on 1771.07 exactly. delta of zero. and it runs in about a second.
the second bug the fix uncovered
adding the locks broke something else, which is how you know you’re doing real work.
with FOR UPDATE in place, transactions now held their database connection for much longer — they sat there waiting on row locks instead of blowing through in a millisecond. and the pool is bounded:
internal/api/api.go
dbConnection.SetMaxOpenConns(25) // Maximum total connections (in-use + idle)
dbConnection.SetMaxIdleConns(5) // Maximum idle connections in pool
SetMaxOpenConns ↗
caps the whole app at 25 connections. now look at what the handler did inside the transaction: it called GetActive, an unrelated lookup, on the pooled *sql.DB — not on the transaction. so each in-flight vote held its transaction’s connection and tried to check out a second connection for that lookup, while holding the first.
do that under load and you get a classic pool self-deadlock: 25 votes each grab a connection for their transaction, all 25 connections are gone, and now all 25 block forever waiting for a 26th connection to run GetActive — a connection that can never free up, because the things holding them are the things waiting. the pool is deadlocked against itself. the fix is to just not do that — pull the lookup out and run it before Begin:
internal/http/handler.go
// Look up the active campaign to tag this vote with, BEFORE opening the
// transaction. GetActive uses the pooled *sql.DB (not tx); running it while
// the transaction below holds its own pooled connection (and, now, FOR
// UPDATE row locks) means each in-flight vote would hold one connection and
// block waiting for a second — under concurrent votes that exhausts the
// pool and deadlocks.
var campaignIdPtr *string
if campaign, err := h.campaignRepo.GetActive(r.Context()); err == nil {
campaignIdPtr = &campaign.Id
}
tx, err := h.db.Begin()the lesson that stuck: never check out a second pooled connection while you’re holding one. either everything in the unit of work goes through the same tx, or it happens outside the transaction entirely. the moment a request needs two connections at once, your effective concurrency limit is half the pool, and the failure mode isn’t “slow” — it’s “hangs until the write timeout.”
the honest part: what this fix is and isn’t
it’s a real fix, not a clever one, and there are warts worth naming.
i chose locks over an atomic update, and it’s a genuine trade. the other honest fix is to skip the go round-trip entirely and write SET "Rating" = "Rating" + $delta, with the delta computed from a locked read — postgres applies the increment atomically against the current row value. for a pure counter that’s cleaner. i kept the explicit FOR UPDATE because the ELO update isn’t a simple increment: the delta for one flavour depends on both current ratings via the expected-score term, so i need a consistent, locked view of both rows in go anyway. FOR UPDATE gives me that; a relative update alone doesn’t. reasonable people would pick differently.
it serializes votes on a hot pair. two votes for the same duel now genuinely queue. at my scale that’s a sub-second, invisible cost. if one flavour ever goes viral enough to be a true write hotspot, this lock becomes the ceiling, and the answer would be a different design — batched deltas, or moving the aggregation out of the write path. i’m not there and i may never be. today, correct-and-serialized beats fast-and-wrong every single time.
the whole thing was invisible for weeks and that’s the actual indictment. not the missing FOR UPDATE — everyone writes that bug once. the failure was that i had no measurement pointed at concurrent correctness, so a silent 79% data loss looked identical to a healthy system. rows present, counts right, requests 200. i had monitoring for the things that fail loudly and nothing for the thing that fails quietly, which is the only kind of failure that actually gets you.
lessons learned
- “it’s in a transaction” is not “it’s safe.” transactions give you atomicity; they do not give you a lock. a read-modify-write under
READ COMMITTEDneedsFOR UPDATE(or a relative update) on top of the transaction, not instead of one. - the rows being all there is not integrity. lost updates leave a perfect audit trail of individually-correct writes that are collectively wrong. if the only thing you check is “did the row get written,” you are blind to an entire class of corruption. measure the invariant, not the paperwork.
- lock in a deterministic order, always. sorting by id before you take two row locks is three lines that make a whole category of deadlock impossible. free insurance.
- never hold a pooled connection while you reach for another. one unit of work, one connection — or do the extra query outside the transaction. otherwise your pool can deadlock against itself long before it’s actually busy.
- write the test for the path that would ruin you. i had the tooling to catch this and pointed it everywhere except here. the scariest bugs live exactly where you assumed correctness instead of asserting it.
next time: the other critical finding from that same audit — a vote rate-limiter that a rotated header walked straight through, and why trusting X-Forwarded-For from the open internet means you never really had a limiter at all.
the fix was two words in the right place; the postgres docs on explicit locking ↗ explain those two words better than i can, and i clearly should’ve read them sooner.