Repository navigation
[feature](external) Support Elasticsearch HTTP scans in FileScannerV2 on 4.2 - #69644
Conversation
… on 4.2 ### What problem does this PR solve? Related PR: apache#69614 Problem Summary: Backport the ES v2 table/file reader with file-local projection, residual filters, and scroll cleanup. Branch 4.2 still uses a dedicated ES scan node, so adapt its planner to emit FILE_SCAN requests only when v2 is enabled. Preserve the existing ES plan when disabled and reuse master's optional Thrift field IDs for ES parameters. Keep the branch's existing Lance dispatch and physical split scheduling. ### Release note Support Elasticsearch HTTP queries through FileScannerV2 when enabled. ### Check List (For Author) - Test: 18 FE unit tests, 72 ASAN BE unit tests, clang-format 16, FE Checkstyle, and clang-tidy on changed C++ lines. - Behavior changed: Yes, ES queries honor enable_file_scanner_v2. - Does this need documentation: No, existing scanner and catalog options. (cherry picked from commit cc55086)
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed all 19 changed files and the affected FE planner/coordinator, BE scanner/reader/mapper, shared transport/parser, Remote Doris helper, and test paths at head a17e138. Two P2 performance regressions are reported inline: negative ES range weights concentrate shard scheduling on one backend, and small LIMIT queries lose the one-shot ES request and materialize a full batch. No P0/P1 finding or existing blocking inline comment was confirmed. There was no additional user-provided review focus.
Review converged after 2 rounds within the static-review scope. Candidate duplicates were merged; transport/lifecycle, projection/count/docvalue, and planner/protocol risks were individually resolved. No unresolved source-level candidate or uncovered changed path remains.
Critical checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and correctness | The new path connects ES planning to FileScannerV2, TableReader, local mapping, shared ES parsing, residual filtering, and table materialization. Focused tests support the intended behavior by inspection; the two performance defects remain. No additional row/type/null/count regression was substantiated. |
| Scope and reuse | Changes are focused on the ES adapter/protocol and reuse the existing parser, transport, mapper, and extracted Remote Doris child-schema helper. |
| Concurrency and locks | Scanner state and streams are private; the existing SplitSource locks and scheduler context remain in use. The immutable planner flag keeps plan/range protocols consistent across threads. No new lock-order issue was found. |
| Lifecycle and static initialization | Params/descriptors outlive readers. First-page scroll capture, terminal-page ID updates, cancellation/error cleanup, split transitions, and move-before-close ownership were traced. No new cross-translation-unit static initialization dependency was introduced. |
| Configuration | No new configuration item. The existing query scanner option is captured during planning; disabling it preserves the legacy ES plan and scanner. |
| Interface/storage compatibility | The parser overload and local interfaces are additive; no storage-format change. The new ES V2 path requires supporting BEs; mixed-version execution was not exercised. |
| Thrift/protobuf | Both touched structs were compared on HEAD, master, and release branches 4.2/4.1/4.0/3.1. New field 13 and fields 31-33 match master, are optional, and are unused on those release refs. No protobuf changes. |
| Parallel paths | Legacy flag-off, parallel/nonparallel scroll, both coordinator paths, preserved Lance dispatch/split scheduling, and Remote Doris helper parity were checked. The new multi-BE scheduler interaction has the inline negative-weight defect. |
| Conditional checks | Docvalue/source fallback, _id, explicit local positions, negative shard ID, stop/EOF, and count fallbacks were checked against their actual upstream invariants. |
| Test coverage | Inspected FE serialization/context tests, scanner admission tests, BE projection/filter/count/page/split/error/cleanup tests, and ES5/6/7/8 v1/v2 differential regression coverage. Multi-BE assignment and small-LIMIT request work need coverage alongside the inline fixes. |
| Expected results | No expected-result fixture was changed. The differential regression compares ordered results with the legacy scanner. |
| Observability | ES read/materialization/filter timers, batch/row counters, common IO timing, table name, and transport error context remain available. |
| Persistence and failover | No EditLog, persisted metadata, transaction publication, or master-failover path changes. |
| Writes and atomicity | This is a read-only external-query adapter; data writes, commit visibility, and delete-bitmap protocols are unchanged. |
| FE-BE propagation | Common ES settings remain embedded in each converted range because this branch uses EsScanNode; per-shard overrides, credentials, SSL, query DSL, docvalue aliases, and backend locations reach the BE path. |
| Performance and memory | Two concrete regressions are inline. Mapping/schema work stays at split/open boundaries, full complex projections satisfy parser storage types, and no additional unbounded allocation or COW/nullable defect was found. ES does not participate in adaptive batching or native condition-cache/index paths. |
| Additional recheck | Rechecked empty/all-filtered input, hidden/reordered/sparse projection, zero-column row counting, last nonempty/empty pages, late runtime-filter fallback, parser errors, repeated close/destruction, HTTPS host preservation, CMake inclusion, and branch field IDs. No additional finding. |
Validation limits: static analysis only. No builds, tests, benchmarks, submodule initialization, or live external ES queries were run. TLS/host locality, late runtime filters, mixed-version execution, and malformed external responses were traced or inspected in tests rather than exercised against a live service. Static convergence does not claim runtime validation.
Review converged after 2 round(s).
| TFileRangeDesc range = new TFileRangeDesc(); | ||
| range.setPath("es://" + shard.getIndex() + "/" + shard.getShardId()); | ||
| range.setStartOffset(0); | ||
| range.setSize(-1); |
There was a problem hiding this comment.
[P2] Keep unknown ES lengths from becoming negative scheduling weights
With parallel scrolling and the same eligible BEs for each shard (for example, ES and Doris deployed on separate hosts), this -1 is summed by LoadBalanceScanWorkerSelector.getScanRangeSize and added to WorkerWorkload.scanBytes. The first selected BE becomes -1 while the others remain 0, so every following shard chooses it again (-2, -3, ...), regardless of the shuffled location order. The default Nereids distribute planner therefore concentrates the ES scan on one BE instead of spreading shards across the cluster. Preserve a nonnegative scheduling weight for unknown-length ES ranges, or normalize the sentinel before workload accounting, and cover multi-BE shard assignment.
| for (const auto& [key, value] : _range.table_format_params.es_params) { | ||
| properties[key] = value; | ||
| } | ||
| properties[ESScanReader::KEY_BATCH_SIZE] = std::to_string(_batch_size); |
There was a problem hiding this comment.
[P2] Preserve the small-LIMIT ES request optimization
For a residual-free query such as SELECT payload FROM es_table LIMIT 1, the legacy EsScanLocalState sets KEY_TERMINATE_AFTER, producing a one-shot search with size=1. The V2 path never forwards that limit, so this property makes the request size the whole session batch (8160 by default), and _read_block materializes that batch before Scanner checks its limit. On a populated shard, source-backed wide payloads therefore require fetching and decoding thousands of documents, plus creating/deleting a scroll context, to return one row. Carry the limit into the ES request under the same no-residual-predicate guard, and add a request-size assertion to the LIMIT coverage.
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
What problem does this PR solve?
Backport #69614 to branch-4.2.
ES HTTP queries do not use FileScannerV2 when
enable_file_scanner_v2is enabled. Add the ES TableReader/FileReader, file-local projection and residual filtering, and scroll cleanup from the master implementation.Branch 4.2 still plans ES through
EsScanNodeand a dedicated ES scanner. Adapt that planner to emit FILE_SCAN plans and per-shard ES parameters when v2 is enabled; disabling the option retains the existing ES plan and scanner. Preserve the query DSL, authentication, SSL, docvalue settings, and backend assignments. Capture the scanner choice during planning so range assignment uses the same protocol without relying on a thread-local context. Reuse master's optional Thrift field IDs for the shared and per-shard ES parameters.Retain branch-4.2's existing Lance v2 dispatch and physical split scheduling.
Release note
Support Elasticsearch HTTP queries through FileScannerV2 when
enable_file_scanner_v2is enabled.Check List (For Author)