Repository navigation
[feature](external) Support Elasticsearch HTTP scans in FileScannerV2 on 4.2 - #69644
Merged
Merged
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)
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
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); |
Contributor
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); |
Contributor
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.
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Oct 27, 2026
4 tasks done
Gabriel39
added a commit
to Gabriel39/incubator-doris
that referenced
this pull request
Oct 29, 2026
Apply the outstanding changes in master merge order: apache#69545 7d16729 apache#69551 7224a2d apache#69612 61d1aef apache#69615 8d2bd1e apache#69650 3dc7341 apache#69651 c00ecb1 apache#69646 e493c61 apache#69668 7865646 apache#69614 already exists on branch-4.2 through apache#69644. Adapt datasource APIs and tests to the release branch, retaining its existing scanner dispatch and statement-bound metadata contracts.
yiguolei
pushed a commit
that referenced
this pull request
Oct 30, 2026
### What problem does this PR solve? Backport the requested external-data and FileScannerV2 changes to `branch-4.2` in their actual master merge order: | Order | Master PR | Change | Backport | | --- | --- | --- | --- | | 1 | #69545 | Preserve Paimon schema options after field renames | Included | | 2 | #69551 | Expand Iceberg capability, transaction and snapshot tests | Included with 4.2 API adaptations | | 3 | #69614 | Elasticsearch HTTP scans in FileScannerV2 | Already present through #69644 (`2104b7509cda`); not applied twice | | 4 | #69612 | Check catalog visibility before SHOW DATABASES accesses metadata | Included | | 5 | #69615 | Publish Iceberg Parquet split offsets | Included | | 6 | #69650 | Preserve ES compound residuals and expand scanner coverage | Included | | 7 | #69651 | Accept Elasticsearch 9.x during node discovery | Included | | 8 | #69646 | Prioritize Parquet V2 row groups using TopN bounds | Included | | 9 | #69668 | Arrow loads and schema inference in FileScannerV2 | Included | The eight outstanding changes are combined in one commit. This branch retains the 4.2 FE datasource APIs instead of importing master's connector-module refactor. ### Backport adaptations - Port Paimon schema restoration, catalog-option provenance, fallback handling and tests to the existing table-copy/serialization lifecycle. Keep statement-bound fields when reader options are applied after a schema-only rename. - Exercise Iceberg's existing metadata and transaction entry points with real in-memory tables and retained-generation fixtures. Preserve the nine disabled known-bug reproductions; omit two connector-only lifecycle tests. Expiration checks verify live data-file sets and exact delete-file counters; 4.2's separate manifest-name/data-counter issue remains unchanged. The delete/history SQL suite checks visible rows and eager/late-materialization parity because 4.2 does not expose master's `_file`/`_pos` columns. The inventory labels upstream-only evidence explicitly. - Port ES residual tracking to the legacy planner and add compound OR/NOT, docvalue eligibility and Thrift-context tests there. Use Java 8-compatible HTTP APIs for opt-in ES 9 tests and the branch's supported node-address fixtures. - Keep the existing full-schema Iceberg metadata-table scan, which already supplies the file-size dependency for split offsets. Adapt the external write suite to verify offsets without requiring master-only distribution plans or golden output. - Use the existing Parquet statistics interface and physical row-location representation. Keep the TopN scheduling and pruning invariants. - Include the shared Arrow array normalizer required by the new reader, taken from the same master revision. Preserve 4.2 Lance/Avro dispatch; omit the schema-inference test for the unavailable Hive OpenCSV parser. ### Validation - FE production and test sources compile with the branch's Java 8 target. Across the targeted runs and focused reruns, 20 test classes report 181 discovered, 171 passed, zero failures/errors, and 10 skipped: nine intentionally disabled known-bug reproductions plus the opt-in live ES 9 class. - FE Checkstyle: passed with zero violations. - Clang compiler syntax checks: all 23 affected C++ translation units passed against the current sources and freshly generated Thrift, using cached third-party/generated build dependencies. Focused BE runtime validation is listed below. - clang-format 16: all 30 affected C++ source/header files passed. - Groovy 4.0.19 syntax and AST conversion: all seven changed suites passed. All 105 local inventory links resolve. - Complete staged-diff self-review, whitespace/conflict checks, and sensitive-information checks passed. A full BE rebuild and SQL/external-service regressions were not run locally. Live ES 9 tests require their opt-in endpoint. CI is requested for the combined backport. ### Release note Fix Paimon reads after field renames, external-catalog SHOW DATABASES authorization, and ES compound residual filtering. Support ES 9.x node discovery, publish Iceberg Parquet split offsets, improve Parquet V2 TopN row-group scheduling, and use FileScannerV2 for Arrow loads and text/JSON schema inference. ### Check List (For Author) - Test - [x] Unit Test - [x] Regression test additions and syntax checks; SQL execution pending CI - Behavior changed: - [x] Yes. Backport the changes listed above. - Does this need documentation? - [x] No. Existing interfaces and settings are retained; the test coverage inventory is included. ### Follow-up fixes - Set the Thrift `enable_file_scanner_v2` default to `true`, matching the session default. Preserve explicit `false` for legacy query scans and extend the existing load-selection test. - Keep the selected Paimon snapshot and schema together during relation projection. Historical sequence/aggregation options must not be combined with the latest field names. Read-only principals can resolve historical selectors without INSERT permission. - Read legacy append schemas with missing or empty bucket keys without bucket pruning, preserving every historical file. The regression covers both unaware and multiple fixed buckets. Follow-up validation: 169 targeted Paimon FE tests passed with zero failures/errors/skips. FE production compilation and Checkstyle passed. Both changed C++ test translation units passed compiler syntax checks and clang-format 16. Regenerated Java/C++ defaults are enabled; a Java 8-targeted protocol check verifies constructor/clear defaults, an omitted wire field, and an explicit-false round trip. Complete BE CI and live external SQL suites remain pending. The two new reproductions first failed with the same missing-bucket-key and renamed-sequence-field errors as CI. A separate read-only-principal reproduction also failed before the permission correction. Complete external regression execution remains pending CI. ### BE unit-test compatibility - Complete the native Arrow Variant input dependency for `branch-4.2`: accept binary storage and the Parquet Variant extension, validate metadata and payloads before appending, and retain owned data after Arrow buffers are released. Allow that ingestion path through the load reader while preserving query-format restrictions and source-default validation. - Preserve Variant V2 markers in the Arrow test fixtures, including nested types. Verify timestamp input against this branch's SerDe for positive, negative, and second-boundary values. Match the existing DATE range diagnostic in the TopN pruning test. - Add tests for sliced and empty reads, invalid ranges and carriers, malformed metadata/payloads, NULL children, atomic failure, buffer ownership, and the query capability boundary. Validation: all 13 previously failing BE tests passed. The expanded run passed all 309 tests across nine suites, including Arrow/JSON loads, load validation, Iceberg, table mapping, and Variant input/output. The focused executable rebuilds the affected readers and SerDe against cached 4.2 build dependencies; this is not a complete BE rebuild. All seven changed C++ translation units passed compiler checks with current headers/generated Thrift, and all 33 affected C/C++ files passed clang-format 16. The complete staged diff, whitespace, and sensitive-information checks passed. Full CI is requested with `run buildall` and `/review`.
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 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)