perf: stop Tracker preheat from fully materializing OptionSet.options [41] - #24813
Merged
Conversation
Adds a new tracker preheat supplier that computes the (option set, code) pairs actually referenced by an import payload and confirms them against the optionvalue table, instead of relying on OptionSetMapper to fully materialize every OptionSet.getOptions() collection. Wired into TrackerPreheatConfig.preheatOrder immediately after ClassBasedSupplier, since it depends on DataElement/TrackedEntityAttribute already being preheated. Not yet consumed by validation - ValidationUtils still reads .getOptions() until a follow-up task switches it over to TrackerPreheat.isValidOptionCode(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Switch ValidationUtils.validateOptionSet to read option-code validity from TrackerPreheat.isValidOptionCode(optionSetId, code) instead of iterating optionSet.getOptions(), avoiding the N+1 tracker import query. OptionSetMapper's options mapping is left in place for now (removed in a follow-up task). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
OptionSetMapper's `options` mapping forced Hibernate to fully load the OptionSet.options collection (including JSONB attribute values) on every preheat, regardless of how many option codes the import actually referenced. Now that ValidationUtils.validateOptionSet reads from TrackerPreheat's targeted (option set, code) cache populated by OptionValueSupplier instead of OptionSet.getOptions(), this mapping is dead weight and safe to remove. Verified via: - dhis-service-tracker suite (135 tests, mock-based/unit) green - dhis-test-integration real end-to-end tests (54 tests: Event/ TrackedEntity/EnrollmentAttr ImportValidationTest) green against a real DB and the real preheat pipeline with the mapping removed - Two mutation tests confirming coverage is non-vacuous: disabling OptionValueSupplier.addCandidates and switching validateOptionSet's allMatch to anyMatch both broke the expected tests for the expected reason
The tracker import preheat only resolves the (option set, code) pairs the import payload actually references, so validation can check option codes without materializing whole OptionSet.options collections. Program rule ASSIGN actions break that assumption: they can add or overwrite data values and attributes that were not in the payload, so the codes they introduce were never resolved and valid data was rejected with E1125. Fix it where the assigned values first become known: the values an ASSIGN action will apply are the rule engine's already evaluated output, captured in the Assign*Executor objects that calculateRuleEffects builds from RuleEffects#getData. Nothing evaluated later can change them, so right after calculateRuleEffects returns we can collect every (target, value) pair from those executors and resolve it through OptionValueSupplier's existing entry point, all within the same "Running Rule Engine" stage. No validator chain changes are needed, and the whole bundle costs one batched query, or none at all when no ASSIGN action is present. Note the injection point matters. Doing this over the bundle's entities instead of its executors would be a no-op: calculateRuleEffects only computes RuleEffects, it mutates nothing. The mutations happen later, per entity, inside the rule engine validators, interleaved with the very validators that read the resolved codes, so there is no whole-bundle boundary in between where the mutated entities could be re-read. Identifiers are converted with TrackerPreheat#getIdSchemes, the same way the executors convert them when applying a value, because preheat metadata is keyed by the payload's id scheme, not by UID. Also adds a diagnostic warning distinguishing "this code was checked and does not exist" from "this option set was never resolved", the latter being an internal bug rather than user error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lueSupplier The two loops in queryChunk cannot be combined: the second depends on confirmed, which is only populated by the JDBC query that runs between them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 8, 2026
jason-p-pickering
changed the title
jason-p-pickering
changed the title
netroms
pushed a commit
that referenced
this pull request
Aug 11, 2026
…(master port of #24813) (#24815) * feat: add valid-option-code storage to TrackerPreheat Adds per-option-set valid-code and resolved-set tracking so validators and the ASSIGN-action pipeline can check option codes without materializing the full OptionSet.options collection. * feat: add OptionValueSupplier, preheat only referenced option codes Queries optionvalue directly for the (option set, code) pairs actually referenced by the import payload, in chunks, instead of relying on OptionSetMapper to hydrate entire option collections. Cost is bounded by distinct codes referenced, not option set size. Includes the NOSONAR suppression for the two-phase collect/query loop (false positive: confirmed is populated by the query immediately before it's read). * feat: validate option codes against preheated OptionValueSupplier data Switches validateOptionSet() to check TrackerPreheat's valid-code cache instead of scanning OptionSet.getOptions(), with a diagnostic log.warn when an option set never got resolved during preheat. Updates all three call sites (tracked entity, enrollment, and event attribute/data value validators) and their tests accordingly. * perf: stop preheat from fully materializing OptionSet.options OptionSetMapper no longer maps the options collection, which was forcing a full lazy-collection load (and JSONB deserialization of every option) for every OptionSet touched during tracker preheat. Callers now go through OptionValueSupplier/TrackerPreheat instead. * fix: resolve option codes introduced by program rule ASSIGN actions ASSIGN actions can introduce data element/attribute values that were never part of the original import payload, so OptionValueSupplier's single preheat pass can't have seen their option codes. Runs the assigned values back through OptionValueSupplier as a synthetic TrackerObjects before validation, so option-set validation still sees them as preheated. Adds getter access to the executors' resolved data element/attribute UID and value, and asserts no errors in the previously-unchecked option-value ASSIGN warning test. * test: remove redundant public visibility modifier in AttributeValidatorTest SonarQube java:S5786 -- JUnit5 lifecycle methods don't need public visibility. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
muilpp
approved these changes
Aug 12, 2026
Preheat only the (option set, code) pairs actually referenced by an import payload, via a new OptionValueSupplier that queries optionvalue directly (unnest parallel-array join), instead of relying on OptionSetMapper to hydrate entire OptionSet.options collections (including per-row JSONB attributevalues deserialization) regardless of how many option codes the import actually references. ValidationUtils.validateOptionSet reads from TrackerPreheat's targeted (option set, code) cache instead of OptionSet.getOptions(). Program rule ASSIGN actions are resolved through the same supplier right after calculateRuleEffects, since they can introduce option-set values that were never in the original payload. Verified functionally equivalent to master's #24850, file by file, with 2.41's own type conventions kept where master has since diverged (String ids predating UID, javax.persistence.EntityManager, etc.). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
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
2 participants
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.
Tracker import preheat maps every referenced
OptionSetthroughOptionSetMapper. The generated MapStruct code called.size()on the lazy Hibernate collection backingOptionSet.options, forcing a full materialization of everyOptionrow for that set — including JSONBattributevaluesdeserialization per row — regardless of how many option codes the import payload actually references. See attached profile trace for details:This is a direct continuation of #23485/#23495, which replaced per-value
OptionService.existsAllOptions()DB queries with an in-memory check against the preheatedOptionSet.getOptions()collection. That PR's own numbers show the residual cost plainly: "The remaining 998 queries are Hibernate loading the options collection during preheat." That residual is exactly what this PR removes.This PR shifts the cost of validation from "How many options exist in an option set?" to "How many valid options exist in the payload?". Under most circumstances (data entry/android upload), the number of options in any given payload is a small fraction of the possible options (particularly for large code lists like ICD-10, equipment lists, or commodities).
Fix
TrackerPreheatgainsaddValidOptionCode/isValidOptionCode, storage for confirmed-valid(option set, code)pairs.OptionValueSupplierpreheat supplier walks the payload's attribute/data values, collects only the option codes actually referenced (splitting multi-text values), and resolves them with one batched, chunked JDBC query againstoptionvalue— cost bounded by payload width, not option set size.ValidationUtils.validateOptionSetreads from that preheated data instead ofOptionSet.getOptions().OptionSetMapperstops mappingoptionsentirely — the expensive collection load never happens during preheat.ASSIGNactions can introduce option-set values that were not in the original payload. Since those values are the rule engine's already-evaluated output (captured in theAssign*ExecutorobjectscalculateRuleEffectsbuilds), they are resolved the same way, right after the rule engine runs and before any validation reads them — no validator-chain changes needed, one batched query for the whole bundle, or none when noASSIGNaction is present.Prior Art
Same approach developed by @teleivo in #24773. Here, the problem is large option sets as opposed to large lists of data elements. Small payloads pay the full price of materializing a large option set, even if they only contain a single option to be validated (which is exactly the case with the Inpatient morbidity dataset in the SL database and the production database where this was initially noticed).
Performance
ICD-10 diagnosis option set (~14,000 options) on a copy of a production system. Similar to Inpatient morbidity and mortality on the SL database with a large ICD-10 option set. Glowroot trace, single tracker event import.
Before:
JobProgress.runStage(preheat + validate)OptionSetMapperImpl.mapOptionValues→PersistentBag.sizeAfter:
OptionValueSupplierbatched JDBC resolution (6 option sets, 2 codes, 6 rows matched)POST /api/41/trackerrequestOptionSetMapperImpl.mapOptionValues/PersistentBag.sizeno longer appear anywhere in the trace. The equivalent resolution step went from 804 ms to 0.95 ms — cost is now flat regardless of how many options the set actually has.Testing
TrackerPreheat,OptionValueSupplier,ValidationUtils's three validator call sites, including multi-text and program-rule-assign coverage.EventImportValidationTest,TrackedEntityImportValidationTest,EnrollmentAttrValidationTest,ProgramRuleAssignActionTest) re-run after every behavioral change in this PR, confirming no regression toE1125semantics.A Gatling load test targeting this same Inpatient Morbidity/ICD-10 scenario has been prepared separately and will follow in a subsequent PR.