[jmandel]

Small fixes: requests no longer wait behind batches, one hung reply, and a few speed-ups

Seventeen independent fixes found while load-testing FHIRsmith with real IG Publisher traffic and comparing its answers with tx.fhir.org's. Each is its own commit with its own test, and they can be taken separately. The branch is merged with main after #335.

Still growing. We'll keep adding small fixes here as we find them, each as its own commit with a test, so they can be taken one at a time. Larger features each get their own PR: SNOMED CT memory-mapping (#330) and VSAC value sets read on demand.

Decisions for you

Each fix is independent, so you can take any subset. Most have one obvious answer; these involve a choice:

Row	What we chose	Alternative	Why
K11	A value set with no url is named by its id, else '??'	Plainer words, such as "the value set in the request"	tx-ecosystem snomed-expand-inactive2 expects '??' exactly; plainer words need that test changed first
K3	A SNOMED CT filter on a concept the edition doesn't have selects nothing (200, empty expansion)	A 4xx	The FHIR CodeSystem and LOINC providers already do this. Open: $validate-code against concept in <unknown> still answers 400 "filter not understood", as main does; it could answer result false instead
K2	A code system with no content gives 422, as does a missing content element or a supplement used as the system	List the codes unverified, with a warning	The same status main gives for an unknown code system
F4	keepAliveTimeout 65 s and headersTimeout 66 s, fixed	A config setting	Longer than nginx's 60 s upstream keepalive; a setting is easy to add if a deployment needs another value
P3	Long operations yield every 10 ms instead of 25	Keep 25 ms	Halves the wait for a request arriving during a long operation, with no measurable cost to the long one
L7	An externally defined answer list adds no blank Answer code	Keep the code	It changes the LOINC 2.83 total to 257,266, so loinc-expand-all-limited and -total need their expected total updated when the test database is rebuilt
X7	Supplement designations must contain every word of a text filter, as LOINC's own names must	Any word	Matches the code system's rule. Whether a supplement nobody asked for takes part at all (X5) is unchanged

Speed

Supplements are looked up in an index, not by scanning every CodeSystem

What was wrong. Provider.loadSupplements() walked every loaded CodeSystem (thousands, from the npm packages) and every resource in the request, on every code-system lookup, several times per $validate-code. In a CPU profile of IG-build traffic this scan was 18–24% of all CPU time.

Effect. Replaying IG-build traffic against one process: at 100 requests/s the process was busy 51% of the time instead of 94%, and it kept up with about 160 requests/s instead of 80. (Measured on our development branch, which carries the same change; the scan is in code it shares with main.)

Change. The provider's CodeSystem map keeps an index of supplements by the URL they supplement, updated on every set and delete. The worker picks out the request's own supplement resources once per request.

$batch-validate-code gives other requests a turn between entries

What was wrong. Validating one entry never yields the event loop, and IG builds send batches of up to 1,000 codes. A 1,000-code batch held the event loop for about 1 s, and every request that arrived meanwhile waited for it.

Effect. On main, with a real 1,000-code SNOMED CT batch from an IG build running, a single-code $validate-code that takes 4–6 ms on its own was sent every 10 ms. Those lookups typically waited 0.6–0.9 s behind the batch, the worst 1.35 s, and in three of five runs 36–48 of them took over 1 s. With this change they typically took about 50 ms (worst 73–238 ms) and none took over 1 s. The batch was no slower (5 runs each).

Change. checkAndYield before each entry. It only yields after YIELD_INTERVAL ms of compute, so small batches are unaffected.

Long operations yield every 10 ms instead of every 25 ms

Effect. In the same experiment (with the batch yield above), a lookup that arrives during the batch waits about two slices: typically 22 ms instead of 51 (worst 42 instead of 86). The batch took 842 ms instead of 879, so there is no measurable cost. This was measured only in that experiment, on our development branch with the same yield code, not under full load.

Change. YIELD_INTERVAL in operation-context.js is 10 instead of 25.

Responses are serialised once

What was wrong. The res.json wrapper in tx.js stringified the resource to log its size, then called the original res.json(data), which stringified it again. That is about 3 ms of extra work per MB of response.

Change. Send the string already built. The headers are the same, since Content-Type is set just above.

A code that is not in an enumerated include is looked up once, not once per listed code

What was wrong. When the code being validated is not in an include's concept list, the checker tries each listed concept as the same concept written in another form (ICD-11's short code and entity id). For every listed concept, sameConceptDifferentForm first looked up the code being validated, to ask whether it has another form at all. That answer is the same for every listed concept.

Effect. On tx-dev.fhir.org, an RxNorm code not in a 1,185-code VSAC value set took 99 s (about 80 ms per listed code), and one not in a 33,341-code value set got no answer in 600 s. In the new test, a miss against a 300-code list looked the code up 301 times; it now looks it up twice.

Change. Ask once per include whether the code has another form, and only try the listed concepts when it does. The answers are the same.

The ConceptUsageTracker startup scan yields

What was wrong. Just after startup the tracker reads every value set (about 4,000 with tx.fhir.org's sources), about 1 s of work without a break. Requests arriving then waited for all of it.

Change. Give the event loop a turn every 10 ms of the scan. The counts are the same.

Correctness

An unknown cache-id on CodeSystem/$validate-code is answered

What was wrong. setupAdditionalResources reports an unknown cache-id as a 404 cache-id-unknown Issue, and a duplicated x-cache-id header as a 400 cache-id-duplicate Issue. In handleCodeSystem (type-level CodeSystem/$validate-code), this is thrown before the inner try, so it reached the outer catch. There, an Issue not marked handleAsOO fell into an empty branch ("handled in the inner method"), so no response was ever sent. The client waited until its own timeout. This was confirmed on tx.fhir.org. The other operations already answer these Issues.

Change. The outer catch answers every Issue that reaches it with its OperationOutcome and status code, as the ValueSet handler does. Issues raised inside the validation are still turned into a result by the inner method, as before.

An edition-only SNOMED CT version resolves to the latest loaded version of that edition

What was wrong. registerProvider also registers each code system under a partial version: for SNOMED CT, the edition URI without its date, and for semver, major.minor. It overwrote that key on every load, so with several versions of an edition loaded, a request that names only the edition got whichever version the library listed last. #331 picks the latest version for implicit value-set URLs, but not for this key.

Change. Keep the latest version under the partial-version key, whatever the load order.

The LOINC importer no longer adds a blank Answer code

What was wrong. AnswerList.csv has one row with no AnswerStringId for each externally defined answer list (ExtDefinedYN = Y: ICD-10, RxNorm and so on; 65 lists in 2.83). The importer turned each such row into an answer with the code ''. All 65 lists shared that one blank Answer, with an Answer/AnswerList link to each list. So Loinc_2.83b.db has 257,267 codes, one more than LOINC 2.83 has (257,266). That is the total loinc-expand-all-limited and loinc-expand-all-limited-total expect.

Change. Such a row adds the list and nothing else. A database built with this change has 257,266 codes, so those two tests' expected total moves to 257,266 once the test database is rebuilt. Until then, the existing Loinc_2.83b.db still has the blank code.

$lookup works for LOINC codes of class type 3 (register L6)

What was wrong. The importer stores the CLASSTYPE of class type 3 as "Claims attachment", but the LOINC provider's map knew only "Claims attachments". For those codes the CLASSTYPE property's value was undefined, so the $lookup response check failed and the client got a 500, every time. That is 1,161 codes in LOINC 2.83, among them 18719-5, 19005-8, 18834-2, 18725-2, 26436-6, 18783-1, 18782-3 and 18726-0, all seen in IG Publisher traffic. The CLASSTYPE = 3 filter searched for the plural too, and found none of them.

Effect. On a local server with Loinc_2.83b.db, $lookup of each of those eight codes went from 500 to 200, with CLASSTYPE = 3 and the description "Claims attachment". An expansion with the filter CLASSTYPE = 3 went from 0 codes to 1,161.

Change. The map uses the importer's spelling, and still accepts the plural.

A supplement's designations match a text filter only when they have every word (register X7)

What was wrong. #335 adds the codes whose supplement designations match a text filter to the code system's own matches. It tests each designation with SearchFilterText.passes, which is true when any one word of the filter matches. LOINC's and SNOMED CT's own text search needs every word. So with the Uzbek LOINC supplement loaded (its language is en), $expand?url=http://loinc.org/vs&filter=serum%20glucose listed codes whose supplement text has either word. It hit the 1,000-code limit, and only 47 of those 1,000 had "glucose" in their display. Before #335 it listed none.

Effect. The same request now lists 38 codes. 34 have "glucose" in their display; the other 4 have a word that starts with it ("Glucosamine", "glucosidase"), which is how words match in text filters. Whether a supplement that wasn't asked for takes part at all is a separate question (register X5), and is unchanged.

Change. A new SearchFilterText.passesAll matches each word as passes does, but needs all of them. The supplement search uses it.

A SNOMED CT filter on a concept the edition doesn't have selects nothing, rather than failing (register K3)

What was wrong. concept is-a, =, descendent-of, child-of, generalizes and descendent-leaf on a concept that isn't in the edition threw "The SNOMED CT Concept … is not known", and $expand answered 500. concept in did the same for any unknown id in its list. This happens in real traffic: US Core's VSAC-derived value sets ask for descendants of 584021000052104, a concept from a national extension that the US Edition doesn't have. tx.fhir.org gave the same 500 during a US Core IG build on 30 Oct. The FHIR CodeSystem and LOINC providers answer an unknown code in such a filter with an empty set.

Effect. On a local server, $expand with concept in 41000147108 (International Edition) and with concept is-a 584021000052104 (US Edition) went from 500 to 200 with no codes. $validate-code of 22298006 against the second value set went from 500 to result false. $validate-code against the in value set still answers 400 "filter … not understood", as on main, because the provider's doesFilter still says no to an unknown concept.

Change. In the provider's filter, a concept the edition doesn't have selects no concepts (every concept, for is-not-a), and filterIn skips unknown ids. ECL is unchanged: an unknown concept there is still INVALID_ECL.

The too-costly message never names a value set 'undefined' (register K11)

What was wrong. A value set given in the request needn't have a url. The too-costly message then named it 'undefined' or '??', depending on where the limit was hit.

Effect. For the register's request (all of urn:iso:std:iso:3166:-2, inline, no url), the 422 now says "The value set '??' expansion has too many codes to produce (>3000)" instead of naming it 'undefined'. A value set with an id is named by its id.

Change. A small helper gives the url, else the value set's id, else '??'. All five places that raise the message use it. We would have liked "the value set in the request", but tx-ecosystem's snomed-expand-inactive2 expects the exact text '??', so changing it means changing that test first.

Regex filters with alternation match whole codes: fixed upstream in #335

#335 anchors every regex filter as ^(?:pattern)$, which is what this branch's commit did for the FHIR CodeSystem and country providers. Main's version is kept and this fix is dropped. The branch still adds a country-code test (US|CA must not match USA, LCA or TCA), since #335 tests alternation only for the FHIR CodeSystem provider.

Robustness

An unknown modifier extension is refused with 422, not 500 (register K10)

What was wrong. Extensions.checkNoModifiers refuses an element that carries a modifier extension the server doesn't know. That is right, since the extension changes what the element means, but the Issue had no status, so the client got 500 as if the server had failed. VSAC's value set 2.16.840.1.113883.11.20.9.46 (Ability) has one (vsacOpModifier) on an exclude filter.

Effect. The register's request (a SNOMED CT is-a filter with an unknown modifier extension) answers 422 business-rule instead of 500, with the same message.

Change. The Issue carries 422.

$expand from a code system with no content answers 422, not 500 (register K2)

What was wrong. A value set that lists codes from a code system whose content is not-present can't be expanded, since there is nothing to check the codes against. The refusal had no status, so the client got 500. A code system the server doesn't have at all already answers 422.

Effect. The register's request (THO's time-period-ranges, code P10Y--P15Y) answers 422 instead of 500, with the same message.

Change. The refusal carries 422. So do its two neighbours in checkSource: a code system with no content, and a supplement named as the system.

Idle connections stay open longer than nginx keeps them

What was wrong. Node closes an idle keep-alive connection after 5 s, but nginx keeps idle upstream connections for 60 s. When nginx reuses a connection just as Node closes it, the request is reset. nginx does not retry a POST, so the client gets a 502. In load tests, up to 2% of requests failed this way at some load levels.

Change. keepAliveTimeout is 65 s, and headersTimeout is 66 s (Node requires it to be longer). They are set in a small helper, library/keep-alive.js, called from server.js.

A VSAC refresh interval that is too long no longer turns into 1 ms

What was wrong. Node runs a timer whose delay is above 2^31−1 ms (about 596 hours) after 1 ms instead. With refreshIntervalHours above that, the VSAC refresh ran back to back. 100,000 hours gave thousands of unhandled rejections within seconds.

Change. The interval is clamped to the longest delay a timer accepts. A value that is not a positive number falls back to the 24-hour default. Both log a warning.

Tests

New or extended tests. The enumerated-include, keep-alive and usage-tracker tests were also run against the code without their fix, and fail there.

• tests/tx/loinc-import-answer-lists.test.js: an externally defined list adds no blank answer; ordinary lists are unchanged.

• tests/tx/codesystem-map.test.js: the supplement index across set, replace, delete and clear.

• tests/tx/batch-validate-yield.test.js: one yield check per batch entry, and the results are unchanged.

• tests/tx/cache-id.test.js: CodeSystem/$validate-code with an unknown cache-id answers 404 with the coded error.

• tests/tx/library-partial-version.test.js: the partial version resolves to the latest loaded version, in either load order.

• tests/cs/cs-country.test.js: alternation matches whole codes only (the fix itself is big bug hunt #2 - various issues per Josh's report #335's).

• tests/tx/validate-enumerated-miss.test.js: a miss against a 300-code list looks the code up a handful of times, not 301; a listed code still validates.

• tests/vs/vs-vsac-refresh-interval.test.js: clamping, the fallback, and the delay passed to setInterval.

• tests/library/keep-alive.test.js: the timeouts, and a connection idle for 6 s can still be reused.

• tests/tx/usage-tracker-yield.test.js: other work runs during the scan, with the same counts.

• tests/cs/cs-loinc.test.js: a code with CLASSTYPE "Claims attachment" has the property in $lookup and is found by CLASSTYPE = 3; a supplement's designations match "serum glucose" only when they have both words.

• tests/tx/designations.test.js: passesAll needs every word, passes only one.

• tests/cs/cs-snomed.test.js: each concept filter on a concept the edition doesn't have selects nothing (is-not-a selects everything), and concept in skips it.

• tests/tx/expand.test.js: an unknown modifier extension on a filter gives 422; a code system that is not-present, or a supplement, gives 422; the too-costly message names a value set by url, id, or as '??'.

Each of these six tests fails without its fix and passes with it.

Results, on this branch merged with main after #335 (9059fb9):

• The new and touched suites pass: 15 suites, 322 / 322 tests. They include cs-cs, cs-country, validate, batch-validate, expand and big bug hunt #2 - various issues per Josh's report #335's supplement-filters, and the new LOINC importer test.
• The new LOINC importer test fails without its fix and passes with it.
• With the six new fixes (L6, K10, K11, X7, K2, K3): tests/tx (except the test-cases suite), tests/cs, tests/vs and tests/library give 3,732 passed, 2 failed, 79 skipped. Both failures are the LOINC import in tests/cs/cs-loinc.test.js running past its timeout (the import test, and the table check that waits on it) while the HL7 suite ran alongside; the LOINC provider tests in that file pass. (This run was before the too-costly fallback became '??'; the five expand suites were run again on the tip, f6ba83d, and pass.)
• HL7 tx-ecosystem test cases, validator 7.0.3: main 9059fb9 and this branch both pass 5,048 of 5,061, and fail the same 13: cs-batch-validate, -bad and -codings (R4 and R5, plus the R5 cached runs) and translate-reverse-r5+ and -a (R5, plain and cached).

Before #335 (on main e4a3b1f):

	main	this branch
jest: tests/tx (except the test-cases suite), tests/cs, tests/vs, tests/library	3,583 passed, 1 failed, 79 skipped	3,604 passed, 1 failed, 79 skipped
HL7 tx-ecosystem test cases (tests/tx/test-cases.test.js)	4,565 / 4,565	4,565 / 4,565

The one jest failure was the same on both: the LOINC import test in tests/cs/cs-loinc.test.js ran past its 120 s timeout on this machine.

🤖 Generated with Claude Code

[grahamegrieve]

this is a bad idea. there's evidently correct changes in here, but there's also wrong statements that lead to wrong code. For instance:

That answer is the same for every listed concept.

I don't believe that's true, though it's possible that it's so terse I didn't understand it correctly.

then there's statements like this:

The provider's CodeSystem map keeps an index of supplements by the URL they supplement

how can that be safe?

so with several versions of an edition loaded, a request that names only the edition got whichever version the library listed last

that's a feature.

[jmandel]

Thanks, Grahame. Fair point: too much in one PR, and some of the wording was too terse to check. I'm closing this in favour of four smaller PRs. Each issue in them has the request, the code path on main, the change and its test. On your three points:

"That answer is the same for every listed concept." It was too terse. For each listed concept, sameConceptDifferentForm asks two things. First: does the code being validated have a second spelling at all? It locates that code and asks for its classificationCode. Second: is this listed concept that same concept? Only the first part is the same for every listed concept, since it never looks at the listed concept, and only that part is now asked once per include. The per-concept comparison is unchanged. For every code system except ICD-11 the first answer is always no, so main does a full locate per listed code only to return false each time (13.6 s for a 148-code RxNorm list on tx-dev). The PR quotes the code and has a test that compares the old loop with the new one: #339.

"How can that be safe?" The index is part of the Map, not a cache beside it. CodeSystemMap overrides set, delete and clear, the only ways a Map changes. addCodeSystem and deleteCodeSystem are the only code that writes to provider.codeSystems; the PR lists every call site. For a given url, the index gives every CodeSystem the scan would accept, in the map's own order, and isSupplementFor still decides. Request supplements (tx-resource, cache-id) never enter that map. Your question was a good one: comparing #332's version with the scan over random add/delete sequences showed it missed a url with |version and could return supplements in another order. Both are fixed, and that comparison is now the test: #340.

Edition-only pins. Understood that it's a feature; that change is withdrawn.

The rest:

• Small fixes: 422s instead of 500s, a request that never got an answer, LOINC data, and two robustness fixes #337: twelve independent fixes, one commit each, such as 422 instead of 500, CodeSystem/$validate-code with an unknown cache-id never answering, and LOINC data. One open question in it for you (K3: $validate-code with concept in <unknown>).
• Give other requests a turn: yield between batch entries, every 10 ms, and during the startup scan #338: yield between batch entries, every 10 ms instead of 25, and during the startup scan. These are tuning, kept apart so you can take some and not others.