[christabone]

The October 4 production investigation found a long search tail that bot rate limits alone cannot address. Four captured ten-minute CDN windows contained 55,146 API requests; 778 took at least five seconds, and 640 of those used /api/search. Eleven model-search requests ended with CDN 504 OriginError after approximately 55 seconds. These counts describe the captured windows, rather than the entire weekend, and CDN duration is not identical to backend query time.

A gene search filtered by models=CX11321 (Cel) with limit=50 returned 200 after 50.826 seconds, almost entirely before the first byte. Inspection of the source retrieved from the running production API confirmed that SearchService then enriches results sequentially: each gene without stored relatedData triggers five synchronous Elasticsearch count searches, and each model triggers three. A bounded metadata-only lookup for that gene-search filter found 502 matching genes and returned 50; all 50 lacked relatedData. Independent gene/model samples also lacked it. The fan-out therefore occurs on production data, rather than being only a theoretical code path.

This PR removes that confirmed multiplier: a page of 50 genes goes from 250 additional count searches to five, and a page of 50 models goes from 150 to three. It also adds phase timings so subsequent observations can separate main-search latency from enrichment latency. The metadata lookup's 72 ms excluded API enrichment, facets, scoring and highlighting; it was not a replay or benchmark of the original request. Historical phase timings are unavailable, so the investigation does not establish how much of the 50.826 seconds came from counts, the main query or connection waiting.

Related-data predicates are deduplicated within each page and executed in sequential batches of at most 50 using named filters aggregations. The existing query builder, link order, labels, zero-count omission and precomputed metadata are preserved. Counts remain exact for overlapping multi-valued fields and totals above 10,000. The count layer rejects failed, timed-out or incomplete responses; the API preserves successful main-search results and facets, logs the enrichment failure and returns empty calculated-link lists while retaining stored metadata. It never presents partial counts as exact.

Slow searches (at least one second) and debug searches log total, main Elasticsearch and related-data durations, plus count-filter/request totals, to distinguish the remaining sources of latency.

Validation:

• All 25 API/core unit tests pass, including regressions for main-search availability and preservation of stored metadata when enrichment fails.
• Two integration tests passed against a disposable Elasticsearch 7.17.28 instance with three primary shards. They compare counts with the original count-query behavior and verify five searches for a 50-gene page.
• Checkstyle passes with test sources included; Git secret-scanning hooks pass.
• PR validation now runs 15 focused unit tests and both integration tests with an Elasticsearch service before the existing full package build; the existing Docker build check is retained.
• An independent Astra medium review approved the code before this PR was opened and approved the availability follow-up.

Production latency improvement still needs measurement. Main-query facet cost and the shared client's long snapshot-oriented timeouts remain separate follow-ups.

Requested review: @oblodgett.

[claude]

Review summary

The batching itself looks right. Keys are deduplicated, counts come back in input order through keyed filters buckets, batches are capped at 50, an empty list makes no ES request, and the filters bucket counts should equal the old per-query totalHits. Switching to an injectable SearchDAO instance is also fine for the @RequestScoped bean, because ESDAO is empty.

One behavior change that could break production: a related-data count failure now fails the whole search

• Before: performCountQuery caught the IOException and returned 0L, and it ignored partial shard failures. If ES had a hiccup, the related-data badges silently dropped off and /search still returned results.
• Now: performCountQueries (SearchDAO.java) throws IllegalStateException on an IOException, a timeout, getFailedShards() > 0, terminatedEarly, or a missing aggregation, and it sets allowPartialSearchResults(false). populateRelatedDataLinks is called directly from query() with nothing catching the exception. So a transient problem in a secondary, decorative count request now becomes a 500 for the main search endpoint, even though the main search already succeeded.

Unless failing the whole search is intended, consider catching the exception in populateRelatedDataLinks (or around the call in query()), logging it, and falling back to the old behavior: leave relatedData empty or omit it. That keeps the strictness for the tests without making search availability depend on the count batch.

Nothing else blocking.

[christabone]

@claude Please review my changes. Commit b7faed6 addresses your availability finding: the DAO still rejects incomplete counts, while SearchService catches count-layer failures, logs them and returns the successful main results, totals and facets with empty calculated-link lists. Stored relatedData is preserved. Both service regressions and the full 25-test API/core suite pass; Astra medium approved this follow-up. Please inspect the updated diff and confirm whether any material issues remain.

[claude]

Claude finished @christabone's task in 37s —— View job

---

Re-review after b7faed6

• Check commits since the previous review (eeb218788 was reviewed earlier; b7faed6ad is new)
• Inspect the updated SearchService / SearchDAO diff against origin/stage
• Check the new regression tests
• Post findings

The availability finding from my earlier review is fixed in b7faed6. No material issues remain.

What I checked in SearchService.populateRelatedDataLinks (SearchService.java:381-424):

• Error coverage: the try covers both performCountQueries and the size-mismatch check. catch (RuntimeException) handles the DAO's IllegalStateException, which wraps IOException, timeouts, shard failures, early termination and a missing aggregation or bucket. It also handles ElasticsearchStatusException, which the high-level client throws directly on a 4xx/5xx response.
• Partial counts: no partial counts can leak. Counts are only assigned after the whole batch loop succeeds. On failure, every pending result gets relatedData = [], which is the same shape as the old "all counts zero" case, so the frontend contract doesn't change.
• Stored metadata: results that already have relatedData are skipped before the try, so the fallback never overwrites them. failedCountsOmitCalculatedLinksAndPreserveStoredMetadata asserts this with assertSame.
• Main search: query() now always sets total, results and aggregations. searchStillReturnsMainResultsTotalsAndFacetsWhenCountEnrichmentFails runs the real query() path with a stubbed performQuery, so it is a genuine end-to-end regression test rather than just a unit test of the helper.
• Logger switch: the change from io.quarkus.logging.Log to a static JBoss Logger is fine. It's needed because Log is only rewritten inside Quarkus-built classes, and the plain JUnit service test would otherwise hit it uninstrumented.

Optional, non-blocking

1. The fallback is ambiguous in the timing log. On failure the method returns -1, so the slow-search line shows related_count_filters=-1 related_count_requests=-1 completed=true. The -1 sentinel also means "enrichment never ran". You can tell the two apart today only through completed=true combined with the separate WARN line. An explicit related_data_failed=true field would make the slow-phase logs easier to grep when you measure production latency.
2. Slow ES is still not covered. The fallback only helps when the count request fails. If ES is merely slow, the count request still waits on the shared client's long timeouts, which the PR description already lists as a follow-up. A cheap middle step would be .timeout(...) on the count SearchSourceBuilder in buildCountSearchSource. A timed-out response already trips the isTimedOut() check and drops into this new fallback. Note that the ES timeout is best-effort per shard and doesn't bound connection waiting.
3. Transient failures can be cached. If the CDN caches /api/search 200s, a response without badges from a brief failure gets cached like any other. The old code had the same exposure for each individual count that fell back to 0, so this isn't a regression.
4. Dead code. SearchDAO.performCountQuery (line 90) no longer has any callers, and neither do the public getRelatedDataLink overloads. You could remove them now or in a later cleanup.

Not verified

I did not run the test suite or the Elasticsearch integration tests in this job. This review is based on reading the diff and the tests.

Verdict: the earlier blocking finding is resolved, and nothing material remains. I have no objection to merging once the CI run (including the new Elasticsearch-backed test step) is green.
· Branch: fix/search-related-data-fanout

[oblodgett]

We are going to achieve this a different way.