DEV Community

Ismail Iusifli
Ismail Iusifli

Posted on

Every step was correct. The student still got refunded twice.

How a randomized invariant test found a billing bug none of my example tests could, and why the fix needed a lock.

An admin cancels a lesson, so the student's charge is reversed. Later a teacher tidies up the attendance journal and clears the mark, and the charge is reversed again. The student ends up credited for a lesson they never paid for.

Each of those three operations was correct on its own, and every example test I'd written passed. A test that threw random operations at a real database found the bug on its first run. Here's how.

The setup

I'm the sole engineer of the platform behind an online school with 300+ active students a month. Teachers and admins mark attendance, and every billable mark charges the student for the lesson. Lessons get re-marked, cleared and cancelled, often by different people at the same time.

I pulled the money-handling core out into a public repo, edtech-billing-core, rewritten on plain PostgreSQL so every lock and constraint is visible. Two design rules matter for this story.

The ledger is append-only. A database trigger rejects UPDATE and DELETE on ledger rows. You never edit a charge: you append a reversal, then a new charge if needed. For each student and lesson there's a net, the sum of its rows, and it should always be either 0 or minus the lesson's price.

Writes can't form a lock cycle. Marks and clears lock the student row (FOR NO KEY UPDATE), then the lesson row (FOR SHARE). A cancellation locks only the lesson (FOR UPDATE). When it writes reversals, the foreign-key checks take KEY SHARE on each student, and NO KEY UPDATE doesn't block that, so there's no cycle and no deadlock. In practice: two writes for the same student run one after the other, marks on the same lesson coexist, and a cancellation waits for marks already in flight.

With those two rules in place, I had example tests for every operation: mark, re-mark, clear, cancel. They all passed. Then I wrote a different kind of test.

A test that doesn't know the answer

Example tests check outcomes you already thought of. The bug I was worried about was the one I hadn't thought of, so I wrote a test that throws random operations at the ledger and checks rules that must hold no matter what happened.

The rules are invariants, each written as a SQL query that returns the rows breaking it. An empty result means the rule holds:

-- "a cancelled lesson nets to zero for every student"
SELECT t.student_id
FROM ledger_transaction t
JOIN class_session s ON s.id = t.class_session_id
WHERE s.is_cancelled
GROUP BY t.student_id, t.class_session_id
HAVING SUM(t.amount) <> 0;
Enter fullscreen mode Exit fullscreen mode

There are eight of them, for example: a lesson's reversals never exceed its charges, a free lesson never nets to a charge, each provider payment is credited at most once.

The test itself:

  • creates 4 students and 10 lessons with random prices, some of them free, and puts one student on a billing freeze;
  • runs 200 random operations for each of 8 seeds: marks, clears, cancellations, payments, and sometimes a redelivered payment to exercise idempotency;
  • runs them five at a time in parallel, against a real PostgreSQL;
  • checks all eight invariants after every batch, and that the ledger never shrinks.

The randomness is seeded, so a failure replays exactly:

function prng(seed: number) {
  let s = seed >>> 0;
  return () => {
    s = (s * 1664525 + 1013904223) >>> 0;
    return s / 2 ** 32;
  };
}
Enter fullscreen mode Exit fullscreen mode

One more rule keeps the test honest: the only error it accepts is touching a lesson that's already cancelled. A lost race or a deadlock fails the test, and a guard at the end checks that at least 100 operations really succeeded, so the test can't pass by refusing everything.

It failed on the first run

The invariant that broke was "a cancelled lesson nets to zero for every student". Replaying the seed gave this sequence. Here it is as ledger rows for one student and one lesson priced at 500:

Step Row appended Net
Student marked present LESSON_CHARGE −500 −500
Lesson cancelled CHARGE_REVERSAL +500 0
Attendance cleared CHARGE_REVERSAL +500 +500

The net should be 0. Instead the student was credited 500 for a lesson they were never charged for.

Each step on its own was correct. Marking charges. Cancelling reverses outstanding charges. Clearing reverses the lesson's charge. The bug only exists in that one order.

Here's the part that surprised me: this isn't a race. The three steps reproduce one call at a time, in order. What found it was the randomness, not the parallelism: random sequences hit orders I'd never write by hand. The parallel part of the test is there for the other class of bug, the real races, and as the next section shows, the fix for this one created exactly such a race.

The fix, and why it needs a lock

The obvious fix is a rule: clearing refuses a cancelled lesson, because the cancellation already reversed the charge.

But the rule alone opens a real race. If the clear checks is_cancelled, then a cancellation commits, then the clear writes its reversal, the double refund is back. So the check has to run under the lesson's share lock, the lock a cancellation must wait for:

export async function clearAttendance(
  db: Db,
  classSessionId: string,
  studentId: string,
) {
  await inTransaction(db, async (tx) => {
    // FOR NO KEY UPDATE on the student
    await lockStudent(tx, studentId);
    // FOR SHARE on the lesson
    const session = await lockSessionShared(tx, classSessionId);
    if (session.is_cancelled) {
      throw new BillingRuleError(
        "Cannot clear attendance for a cancelled lesson",
      );
    }
    // ... reverse the charge
  });
}
Enter fullscreen mode Exit fullscreen mode

While the clear holds FOR SHARE on the lesson, a cancellation, which needs FOR UPDATE, waits. When the clear commits, the cancellation reads the nets after its lock wait, finds nothing outstanding, and writes nothing. If the cancellation gets there first, the clear sees is_cancelled and refuses. Either way, the net stays at zero.

Both cases are now permanent tests: the plain sequence, and the race. The race test fails if the share lock is removed.

Why not just SERIALIZABLE?

It's the first question people ask, so two answers.

It wouldn't have caught this bug. The double refund happens with the steps run one after another, which is a perfectly serializable history. Isolation levels protect you from bad interleavings, not from wrong rules.

I measured it. My production code wraps billing in SERIALIZABLE and also takes FOR UPDATE on the student, and the first version of this extraction copied that. A concurrency probe during code review showed roughly 9 in 10 concurrent writes aborting with serialization failures, and FOR UPDATE deadlocking against a cancellation's foreign-key checks. You can retry serialization failures, but when most writes fail under ordinary contention, retries become the workload. With one explicit lock order under READ COMMITTED, every concurrent call in the burst tests succeeds. That one deserves its own post.

What I took from it

Example tests check what you thought of. I had tests for clearing and tests for cancelling. I didn't have one for clearing after cancelling, because I hadn't thought of it. A randomized test doesn't need you to.

Write invariants, not expected values. "The balance should be 500" only holds for a case you designed. "A cancelled lesson nets to zero" holds after any sequence, which is what makes it useful against sequences you didn't design.

Test on the real database. The guarantees here live in locks, constraints and a trigger. A mocked database would have passed this test forever.

Make failures replayable. A random test that fails once and never again is noise. With a seed, the failure became a three-step story I could read, fix, and turn into two permanent tests.

Check that your tests can fail. The repo has a mutation script that removes each defence in turn (either lock, the cancelled-lesson guard, the idempotency key and more) and reruns the suite. All 12 mutants are killed, so I know the tests aren't passing by accident.

The full code, the invariant test and the mutation script are in edtech-billing-core. If you build billing, or anything else where a number has to be right, I'd like to hear how you test it.

I'm Ismail, a backend engineer focused on billing, payments and integrations. iusifli.dev · LinkedIn

Top comments (1)

Collapse
 
_firelinks profile image
Mike Dabydeen •

I'd move the rule from clearAttendance into the ledger itself. If every CHARGE_REVERSAL row carried the id of the charge it reverses, a UNIQUE constraint on that column would make a second reversal of the same charge fail at insert, whichever code path tries it. Charges leave the column NULL, and PostgreSQL treats NULLs as distinct in a unique constraint by default, so they never collide. A re-mark appends a new charge with a new id, which can then be reversed once in its turn.

That changes what the next feature can break. Today the guarantee depends on every operation that reverses a charge remembering to check is_cancelled under the share lock, so a future path, say a refund when the teacher doesn't show up, has to know that rule too. With the constraint in place your lock order still decides who wins the race, and a path that forgets the check gets an error at insert instead of a wrong net that the invariant query finds later.