Record the machine that produced each library's results - #320
Merged
Conversation
The report's "System info" line was CPU model, RAM and distro, which is identical on ryzen-5950x-1 and ryzen-5950x-2, and nothing anywhere said which node a run came from. job_claim.host is the only place the hostname was written, and that is a live claim table - the next run overwrites it, so the question "which machine produced this result" was unanswerable a day later. Put the hostname in front of the report's system info, and store the hostname and the whole system info string in [libversion], which is already keyed (date, branch, libname) - one row per library per run, which is the right granularity now that libraries of the same branch can be claimed by different machines. Both schema changes are ADD COLUMN only: the rows already in the databases keep their results and read back NULL for a run whose machine was never recorded. sqlite migrates on user_version 3 -> 4 (and from 1 and 2, which also had to gain the columns); PostgreSQL uses ADD COLUMN IF NOT EXISTS, since the shared database is created once and never migrated. Verified on a copy of both: the existing rows survive, the new insert shape works, re-running createTables is a no-op, and the NATURAL JOIN in test.py is unaffected - the added names collide with nothing in omcversion or in a branch table. sqlite2postgres.py learns the two columns as well, and now fills a text column the source lacks with NULL instead of the string "0". What made this worth doing: ExternalMedia and Buildings' Utilities.IO.Python_3_8 models have been flipping in exact anti-phase on master since 2026-08-11, ten run pairs out of ten. It turned out that ExternalMedia does not load on the Ubuntu 22.04 node and the Buildings Python library does not load on the Ubuntu 24.04 nodes, so master alternating between two machines is the whole of it - but establishing that took cross-referencing a transient claim table against report timestamps, which is exactly the work this commit makes unnecessary. See OpenModelica/OpenModelica#16376.
This was referenced Aug 21, 2026
adrpo
added a commit
that referenced
this pull request
Aug 24, 2026
#320 stores socket.gethostname() as the machine that produced a library's results. That is right until the run is inside a container, where it is the container id: job_claim currently says the wasm-jit libraries were tested by "e9725d308091", which identifies no machine and is gone with the container. #295 wants every job in a Docker image, which would make that the answer everywhere and leave the new libversion.host column as useless as no column. Ask Jenkins instead. It names its agents and propagates NODE_NAME into a container it starts, so that is the machine name when there is a container in the way. LIBTEST_HOST overrides it, for the same reason LIBTEST_DB exists. NODE_NAME is only consulted inside a container - detected by /.dockerenv, or /run/.containerenv under podman. Outside one the kernel's answer is the right one and a NODE_NAME left in the environment must not override it, which is also what keeps this a no-op for the machines running the testing today. One helper in resultsdb, so the claim in job_claim and the host recorded with the results cannot disagree about which machine this is. Checked all four paths: plain host, NODE_NAME set but no container (unchanged, the real hostname wins), LIBTEST_HOST set, and inside a container with and without NODE_NAME. The container paths were exercised by faking the marker file rather than in a real container - Docker is not available where this was written. Follow-up to #320, part of #301, and needed before #295.
adrpo
added a commit
that referenced
this pull request
Aug 24, 2026
The library testing runs against a branch and says what broke after a change was merged. This tests a pull request before that, and compares it against master. The compiler is built from refs/pull/<N>/merge - the pull request as it would land, not the branch on its own - fetched from GitHub itself, since refs/pull/* is not on the read-only mirror the job clones. It is checked out detached, so nothing is left in the shared workspace for the next run to trip over. The run fills a pr-<N> table like any other branch, which the shared database of #295 makes cheap: the claim is (branch, libname), so it cannot collide with a master run, and the results carry their own omcversion, so nothing else can reuse them by mistake. What did not exist is the comparison. all-reports.py reports a branch against its own previous run, which is exactly what a pull request must not do, and its query reads one table. pr-report.py takes the newest run of pr-<N> and the newest run of the baseline branch and compares them with the same rule and the same thresholds: the phase each model reached, and what each phase cost. Per library it compares the newest run each side has of it, because a run does not necessarily hold every library - one whose version, compiler and configuration were tested before keeps the results of the run that produced them. It writes the report next to the nightly ones, in history/pr-<N>/<baseline run>..<pull request run>.html, and a summary to comment on the pull request with beside it. Two things would make a difference mean something other than "the pull request did this", and the report says so when they apply: - the machine, since two runs produced on different hardware compare the hardware as much as the change. The parameter defaults to the node that produces the master runs, and the report names both machines, which #320 records per library; - the libraries, since two runs that tested different library versions, or verified against different reference files, differ for reasons of their own. The models one run has and the other does not are counted and listed rather than quietly left out of the comparison, since a library that failed to load looks like nothing at all otherwise. In Jenkins it is the pull_request parameter, with pull_request_baseline, pull_request_config and pull_request_node beside it. A full run takes days, so this is on demand and takes a configuration file: testing every pull request is not the idea. The tables accumulate, roughly 19500 rows each, and unlike a branch a pull request is tested once and never again. drop-pr-tables.py drops the tables of pull requests that have been merged or closed, and of those tested more than --older-than days ago, together with the rows their runs left in the other tables; it lists them and does nothing unless it is given --yes. The reports published for them are not touched. It is the drop_stale_pull_request_tables parameter in Jenkins, and the same stage keeps the per-pull-request omc builds on the test node from piling up. --- Generated by Claude Code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nothing recorded which machine produced a result. The report's
System infoline iscpu_name(), RAM, lsb_release:which is the same string on
ryzen-5950x-1andryzen-5950x-2. The hostname was writtenin exactly one place,
job_claim.host, and that is a live claim table — the next runoverwrites the row — so "which machine produced this result" became unanswerable a day
later.
This puts the hostname in front of the report's system info:
and stores the hostname and the whole system info string in
libversion, which isalready keyed
(date, branch, libname)— one row per library per run. Per library ratherthan per run is the right granularity now that libraries of the same branch can be
claimed by different machines.
Not losing anything
Both schema changes are
ADD COLUMNonly. Rows already in the databases keep theirresults and read back
NULLfor a run whose machine was never recorded.user_version3 → 4. The 1 and 2 paths gain the columns too, sincethe migration applies one step per invocation.
addLibversionHostchecksPRAGMA table_infofirst, so it is a no-op on a database that already has them.ADD COLUMN IF NOT EXISTS: the shared database is created once andnever migrated, so
createTableshas to be able to bring an existing table forward.sqlite2postgres.pylearns the two columns, and now fills a text column the sourcelacks with
NULLinstead of the string"0".Verified against a copy of each backend — an old-shape table with rows in it, migrated:
the existing rows survive with
NULLin the new columns, the new insert shape works,running
createTablesa second time changes nothing, and theNATURAL JOINintest.py:755still resolves (host/sysinfocollide with no column inomcversionorin a branch table, so
single-model.py'sNATURAL JOIN libversionis unaffected too).Every other reader names its columns explicitly.
What made this worth doing
ExternalMedia(24 models) and Buildings'Utilities.IO.Python_3_8.*(8 models acrossBuildings_11andBuildings_12) have been flipping in exact anti-phase onmastersince 2026-08-11 — in every run pair where either group moves, one recovers and the other
breaks, never both, never neither:
Both groups depend on the node's environment. The answer is that ExternalMedia does not
load on the Ubuntu 22.04 node, and the Buildings Python 3.8 models do not compile on the
Ubuntu 24.04 nodes, so
masteralternating between two machines is the whole of it.Different mechanisms: ExternalMedia fails in a compile-time
dlopenduring constantevaluation, while the Buildings models reach the C compiler and hit 24.04's clang
rejecting an implicit function declaration in Buildings'
pythonWrapper.c— the latter isbeing worked around in #321.
Establishing that took cross-referencing
job_claim.hostagainst theTest startedofeach branch's published report —
ryzen-5950x-2andryzen-9950xcome out as Ubuntu24.04.4, and the 5950X on Ubuntu 22.04.5 in the
masterreport isryzen-5950x-1byelimination — and then checking all 11 branches with an August run, which split on the
distro with no exceptions. That is exactly the work this commit makes unnecessary, and it
only worked because the distro happens to differ; it would not have worked once the two
nodes are aligned.
Relation to the open tickets
Part of #301, which asks for exactly this and proposes a
machine (branch, date)table. I put the columns on
libversioninstead, keyed(date, branch, libname): a tablekeyed per run cannot express "ExternalMedia ran on A and Buildings on B in the same run",
which is precisely the situation #301 exists because of, now that jobs are claimed per
library rather than pinned per machine. This supplies the data only — making the
performance comparison machine-aware, the second half of #301, is still to do.
#295's remaining task, running the testing from one Docker image, is the actual fix
for the divergence this uncovered — the three nodes differ from each other in ways nobody
intended, down to
ryzen-5950x-2having libpython3.8 whileryzen-9950xdoes not. Andonce the nodes are aligned the
System infoline stops telling them apart at all, sincethe distro is the only thing that currently differs, which is the other reason the
hostname needs to be in there.
Related: OpenModelica/OpenModelica#16376, which is about
omcthrowing away thedlopen()error that would have named the missing dependency in the first place.generated by Claude Code