perf(client): parse each Elasticsearch page once on the scroll hits path (softclient4es-arrow#160) - #227
Merged
Merged
Conversation
…ath (softclient4es-arrow#160) The scroll / PIT+search_after paths parsed every page three times: the typed client parsed the HTTP response, convertToJson re-serialized the whole response to a JSON String, and core parseResponse re-parsed that string — ~19% of Flight sidecar CPU during JOIN leg extraction, scaling with the number and width of selected columns. This made a post-join aggregate's extra keyword column cost +9.8s over the bare join on the arrow#160 benchmark while the DuckDB aggregation itself cost nothing. Paging searches now run with document type ObjectNode so each _source is materialized once as the Jackson tree the row parser consumes; hitsToResponseNode re-parents those trees into a minimal envelope (_id + _source; whole-hit fallback via the new TokenBuffer-based convertToTree when inner_hits/fields are present), and callers use the node-level parseSingleSearchResponse. Aggregation-bearing responses go through convertToTree instead of StringWriter + readTree. One-shot search and msearch paths are unchanged. Measured on the benchmark corpus (overlay image, same machine/day): J0 40.0->28.3s, J2 45.0->31.1s; 10M-leg extraction 256.7k->364.5k docs/s (J0) and 227.1k->332.0k docs/s (J2); J2-J0 +5.0->+2.7s. Row oracles exact. es8+es9 suites 299/299 green on real ES 8.18.3/9.0.3; new JavaClientConversionSpec pins the envelope contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…all clients (#228) The es6/es7 clients still processed every scroll page three times (typed parse, XContent/Gson re-serialization, Jackson re-parse), and every client double-parsed one-shot single and multi searches: the executor contract returned a JSON String that core parseResponse re-parsed. The same per-column extraction CPU that #227 removed from the es8/es9 scroll path (~19% of Flight sidecar CPU, softclient4es-arrow#160) was being paid on the REPL/JDBC 6.x/7.x paths and on every one-shot query. The executor contract now hands core an already-parsed Jackson tree (executeSingle/MultiSearch sync+async return Option[JsonNode]; core gains the node-level parseResponseTree dispatch, and parseInnerHits moved from Gson to Jackson): - es6/es7 REST: one-shot search, msearch and all scroll/search_after/ PIT paging now go through the low-level RestClient and the raw response entity bytes are Jackson-parsed once — one pass total. The typed builders still build request bodies; endpoints are percent-encoded like RequestConverters did, msearch sends UTF-8 bytes as bare application/json (ES 6.8 answers 406 to "application/x-ndjson; charset=UTF-8"), scroll ids / shard failures / search_after cursors are read from the tree, and pages with failed shards fail loudly like es8/es9 instead of silently losing rows. - es6 Jest: scroll pages and searches parse the retained raw body (getJsonString) — the Gson re-serialization pass is gone. - es8/es9: one-shot search/msearch use document type ObjectNode with the new searchResponseToTree/msearchResponseToTree (re-parented _source trees; token-level whole-envelope fallback for aggregation-bearing responses and msearch failure items). Review hardening (adversarial review, all fixes suite-validated): permanent 4xx on paging fails fast instead of burning retries (ResponseException is an IOException; 408/429/5xx stay retriable); a first page with failed shards releases the scroll context it created; a hit page without sort values aborts paging instead of silently restarting from page one; and paging streams now FAIL on error instead of ending quietly — a mid-scroll failure used to surface a silently truncated result set as a SUCCESSFUL result (same defect class as #209/#224), including pre-existing swallows on es8/es9. Suites on real ES, all green on the final code: core 746 unit tests, es6rest 303, es6jest 290, es7rest 305, es8java 316, es9java 316; 2.12+2.13 cross-compile; new parseResponseTree cases in ElasticConversionSpec and searchResponseToTree/msearchResponseToTree contracts in JavaClientConversionSpec (es8+es9). Closes #228 Co-Authored-By: Claude Fable 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
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.
What
Parse each Elasticsearch page once on the scroll / PIT+search_after hits paths (es8 + es9 java clients). Previously every page was parsed three times: the typed client parsed the HTTP response (
SearchResponse[JMap[String,Object]]),convertToJsonre-serialized the whole response to a JSONString, and core'sparseResponsere-parsed that string into a Jackson tree. JFR on the Flight SQL sidecar during a JOIN-leg extraction measured the string round-trip alone at ~19% of total CPU — per-character string writing (WriterBasedJsonGenerator._writeString2, double→ASCII) and re-parsing (ReaderBasedJsonParser._finishString/_parseName, ASCII→double), all scaling with the number and width of selected columns.How
pitSearchAfter,scrollClassicinitial + continuation) now run with document typeObjectNode, so each_sourceis materialized exactly once as the Jackson tree the row parser consumes.hitsToResponseNodebuilds the response-envelope node directly from the typed response:_id+ the re-parented_sourcetree per hit — no serialization, no re-parse. Core'sparseSimpleHitsreads exactly_id,_source,inner_hits,fieldsper hit; hits carryinginner_hitsorfields(UNNEST, script fields) fall back to a whole-hitconvertToTreefor full shape fidelity.convertToTreeserializes via a JacksonTokenBuffer(JacksonJsonpGeneratorover the buffer,readTree(buffer.asParser())) — a token-level copy with no string materialization. Aggregation-bearing responses (at most one per query) go through it instead ofStringWriter+readTree.parseSingleSearchResponseinstead ofparseResponse(String). One-shot search / msearch paths are unchanged.Why (softclient4es-arrow#160)
Post-join aggregation (J2) cost +9.8 s over the bare join (J0) — ~3× Trino's marginal cost. Phase instrumentation showed the DuckDB join+aggregate+stream phase at ~850 ms for both J0 and J2; the entire marginal cost was the 10M-row leg's extraction slowing from 256.7k to 227.1k docs/s because the aggregate needs one extra keyword column, and per-column extraction CPU was dominated by the double parse.
Measured on the arrow#160 benchmark corpus (overlay image = published 0.2.5-SNAPSHOT sidecar + these jars, same machine/day, INFO logging):
10M-leg extraction rate: 256.7k → 364.5k docs/s (J0) and 227.1k → 332.0k docs/s (J2). Row-count oracles exact on J0/J1/J2 (1,000,000 / 125,361 / 100).
Behaviour note
The old string round-trip serialized with
JacksonConfig(Include.NON_NULL), which silently dropped null-valued_sourceentries; the direct tree preserves them as explicit nulls. Rows for requested fields are unchanged (normalizeRowfills nulls either way); onlySELECT *rows can now surface a null-valued column that previously vanished.Tests
es8java/testOnly *JavaClient*— 299 passed, 0 failed (real ES 8.18.3, Docker), covering ScrollCompleteness, SelectCompleteness (incl. script-fields shape), LimitCompleteness, GroupByCompleteness, WindowPartitionCompleteness, HitMetadata, GatewayApi.es9java/testOnly *JavaClient*— real ES 9.0.3, Docker.Second commit — #228: single parse everywhere (es6/es7 + one-shot paths)
Commit
848ff277extends the single-parse contract to the rest of the ecosystem. Closes #228.executeSingleSearch/executeMultiSearch(sync + async) now returnOption[JsonNode]instead ofOption[String]; core gains the node-levelparseResponseTreedispatch andparseInnerHitsmoved from Gson to Jackson. Internalprivate[client]surface only — no downstream references exist.RestClient, Jackson-parsing the raw response entity bytes exactly once (the typed path parsed every page three times). Request bodies still come from the typed builders; endpoints are percent-encoded likeRequestConverters; msearch sends UTF-8 bytes as bareapplication/json(ES 6.8 rejectsapplication/x-ndjson; charset=UTF-8with a 406 — caught by the suite).getJsonString) — the Gson re-serialization pass is gone.ObjectNode+ newsearchResponseToTree/msearchResponseToTree(same re-parenting technique as this PR's scroll fix; msearch failure items keep full fidelity so core still sees theirerror).Hardening from the adversarial review (Blind Hunter + Edge Case Hunter, all fixes suite-validated):
ResponseExceptionis anIOException, whichretryWithBackoffretries; 408/429/5xx stay retriable — a resilience gain).sortvalues aborts paging loudly instead of silently restarting from page one (unbounded duplicates).scrollRows/async recovers translate the failure into a properElasticFailure.Behaviour notes (release-note material): the null-survival note above now applies to es6/es7 and to one-shot paths as well (one-shot and scroll rows are now consistent for
SELECT *); paging pages with partial shard failures fail loudly on es6/es7 (they were silently accepted); mid-stream paging errors surface as query failures instead of silently truncated successful results (all versions).Tests (all on the final code, real ES via Docker): core 746 unit tests; es6rest 303; es6jest 290; es7rest 305; es8java 316; es9java 316; 2.12+2.13 cross-compile; scalafmt + headerCheck clean. New unit contracts:
parseResponseTreecases inElasticConversionSpec,searchResponseToTree/msearchResponseToTreeinJavaClientConversionSpec(es8+es9).Remaining follow-up (not in this PR):
ArrowTypeMapping.fillBatch(~9% CPU) is the next-order term, in softclient4es-arrow.Closes #228
🤖 Generated with Claude Code