nopayloaddb

nopayloaddb is the HEP Software Foundation’s reference conditions database, maintained at Brookhaven and used in the sPHENIX and Belle II ecosystems. It answers one question at scale: which calibration payload was valid at this point in a run?

Project

HEP Software Foundation reference conditions database

My role

External contributor, 1 merged, 7 open

Merged

Fix comb_iov calculation in PayloadIOVAttachAPIView

Shipped in

v5.0.0 and v5.1.0

Repository

BNLNPPS/nopayloaddb

BNLNPPS/nopayloaddbHEP Software Foundation reference conditions database. Django REST Framework over PostgreSQL.Python

The problem

An interval of validity is a two-level integer pair: a major component, typically the run, and a minor one, typically the sub-run. Asking “which payload applies at (major, minor)?” against two columns is a comparison the database cannot serve from one index.

So the schema denormalises it. comb_iov collapses the pair into a single DECIMAL(38,19): major plus minor divided by ten to the nineteenth, so the minor component occupies the fractional part, and a covering index on it turns the lookup into one range scan ordered descending, limit one.

That is a good trade, and it comes with an obligation: every code path that changes a start interval must recompute the derived key. Miss one, and the index is quietly wrong.

What I found

One was missed. In the branch of the attach endpoint that adjusts a neighbouring interval when a newly attached one overlaps it, the recomputation read the major component twice instead of reading major and then minor.

The consequence is narrow and nasty. It only triggers when an existing interval is adjusted during an attach, and only when that interval’s minor component differs from its major, so most of the time the wrong arithmetic produces the right number. When it does not, nothing raises: the ordering key is simply wrong in its fractional part, and a validity lookup returns a different payload than it should.

I opened an issue first, with a table showing the formula was correct at four other call sites and wrong at one, naming the two views whose queries depend on that ordering. Then the patch: one file, one line added, one removed. It merged ten days later, into v5.0.0 and v5.1.0.

Hard parts

Proving a one-line change is the right one. A single-character difference in an unfamiliar codebase is indistinguishable from a misunderstanding. What made it reviewable was the audit around it – every place the same expression appears, which of them are correct, and why this branch is the exception – so the maintainer could check the claim without reconstructing it.

A silent bug in a project without unit tests. Correctness here is verified by standing the API up in Docker and running an external C++ client’s test suite against it. That catches behaviour a client exercises; it does not catch an arithmetic slip in a rarely-taken branch that produces a plausible number. Reading the code was the only way this surfaces.