fix: avoid eager OptionSet.getOptions() dead work in analytics MetadataItemsHandler - #24876
Draft
jason-p-pickering wants to merge 3 commits into
Draft
fix: avoid eager OptionSet.getOptions() dead work in analytics MetadataItemsHandler#24876jason-p-pickering wants to merge 3 commits into
jason-p-pickering wants to merge 3 commits into
Conversation
…taItemsHandler MetadataItemsHandler.handle() computed itemOptions (a full OptionSet.getOptions() collection reload per option-set-backed query item) unconditionally on every request, but getOptionItems() only ever reads it on the empty-grid branch - the grid-scoped optionsPresentInGrid is what's used whenever the grid has rows (the common case). Make the computation lazy via a Supplier so it only runs when the grid actually has no results, removing a full option-set reload (e.g. ~10k rows for large option sets) from the hot has-results path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jason-p-pickering
marked this pull request as draft
August 17, 2026 07:01
jason-p-pickering
marked this pull request as ready for review
August 18, 2026 07:15
jason-p-pickering
marked this pull request as draft
August 19, 2026 07:24
|
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
1 participant
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.
Summary
MetadataItemsHandler.handle()computeditemOptions(a fullOptionSet.getOptions()collection reload for every option-set-backed query item) unconditionally on every
request, but
getOptionItems()only ever reads it on the empty-grid branch — thegrid-scoped
optionsPresentInGridis what's actually used whenever the grid has rows(the common case). So on every request that returns any rows at all, the full option
set (e.g. ~10,000 rows for a large option set) was reloaded and immediately discarded.
Found while live-validating #24850 via a Glowroot trace on
/analytics/events/aggregatefor a 10k-option ICD-10 data element: only 12 SQL executions (~187ms DB time) but a
~1.13s total transaction — ~945ms of unaccounted Java-side work from this eager-then-
discarded reload plus its in-memory stream/filter work, invisible to JDBC-only
instrumentation.
itemOptionscomputation in aSupplier<Set<Option>>, only invoked insidegetOptionItems()'s empty-grid branch.just lazily).
OptionSetcollection reload from the has-results path entirely.Not in scope (follow-up candidate): even the grid-scoped path resolves the entire
option set via
optionSet.getOptions()and filters down to matching codes in Java —pushing a
code IN (:codes)filter into the query would avoid materializing unusedrows for very large option sets.
Test plan
MetadataItemsHandlerTest#handleDoesNotEagerlyReloadFullOptionSetWhenGridHasResults— spies on the query item's
OptionSet, assertsgetOptions()is never invokedwhen the grid already has rows. Verified red (fails on the eager call) before the
fix, green after.
MetadataItemsHandlerTest#handleStillSurfacesFilterOptionsWhenGridHasNoResults— proves the empty-grid path (used to surface filter-referenced option metadata
when a query returns zero rows) is unchanged by the fix. Mutation-tested by
temporarily breaking the lazy path and confirming this test fails for the right
reason.
dhis-service-analyticsmodule test suite passes locally.