Repository navigation
[improvement](fe) Optimize external file COUNT scan scheduling - #68726
Merged
Merged
Conversation
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
Repeated queries against a single remotely accessible file split can repeatedly select the same backend when external file caching and consistent-hash scheduling are disabled. The round-robin scheduler reconstructs its candidate list from a host map, losing the query-local candidate order chosen during initialization.
This change preserves that order for round-robin assignment. It also keeps local scheduling and split redistribution within the active backend candidates when load selection retains a preferred subset. Replacing the order is restricted to round-robin policies.
For unfiltered, ungrouped `COUNT(*)` and `COUNT(1)` over File, HDFS, HTTP, Local, and S3 TVFs, this change enables the existing file COUNT pushdown under the existing session switches. DISTINCT, grouped aggregates, filtered plans, merge stages, and stream/ingest TVFs are excluded. A worker with exactly one assigned COUNT scan range uses one instance instead of creating empty local-shuffle instances. Workers with multiple ranges retain the existing parallelism rules.
Previously saved performance measurements on master baseline `0dcd2a31a7b`, using three BEs on one host, a single-split CSV containing 1,000,000 rows, concurrency 64, a 10-second warmup, and a 30-second measurement window, recorded median QPS of 179.900 before and 561.000 after (3.118x). All six rounds reported zero errors. These measurements predate the final review fixes and are workload-specific; the final PR revision has not been benchmarked again. This result does not establish the same improvement for other queries or customer deployments.
### Release note
Improve round-robin scheduling for external file scans and reduce execution overhead for unfiltered file TVF row-count queries with a single assigned scan range.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
Validation performed:
- 19 focused JUnit tests passed across `FederationBackendPolicyTest`, `FileTvfCountPushDownTest`, and `UnassignedScanSingleRemoteTableJobTest`. They were executed using a standalone JUnit Launcher with an explicit Byte Buddy agent and available FE compiled dependencies. The modified scheduling class and its test were compiled from the final sources. This is not a successful standard `run-fe-ut.sh` reactor run; that run remains pending because the shared FE build artifacts were inconsistent.
- FE Checkstyle passed with `mvn -o -pl fe-core -am -DskipTests validate`.
- `git diff --check` passed.
- The S3 compression regression includes new COUNT cases, but execution and framework-generated `.out` entries are still pending. This PR is a draft until those results and final-version validation are available.
- Behavior changed:
- [ ] No.
- [x] Yes. Round-robin assignment preserves initialized candidate order; preferred load subsets are honored by local scheduling and redistribution; eligible file TVF COUNT scans use pushdown and one instance per worker when that worker has one assigned range.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
OIiveirra
marked this pull request as ready for review
October 4, 2026 17:13
OIiveirra
requested review from
924060929,
CalvinKirs,
englefly,
morrySnow and
starocean999
as code owners
October 4, 2026 17:13
Collaborator
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28623 ms |
Contributor
TPC-DS: Total hot run time: 153174 ms |
Contributor
ClickBench: Total hot run time: 24.47 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
|
Review notes for
I did not identify a concrete blocking issue in preserving the initialized round-robin candidate order. The existing behavior where cache/consistent-hash scheduling can keep a hot split on the same BE is unchanged and is not a regression introduced by this PR. |
5 of 16 tasks
OIiveirra
added a commit
to OIiveirra/doris
that referenced
this pull request
Oct 8, 2026
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68726 Problem Summary: Independently planned queries for one remote split can keep selecting one backend. Enable automatic external scan spreading by default with value 0, using all eligible backends in the current compute group and minimum assigned weight with random ties. Preserve locality and single split execution. Value 1 restores original scheduling; explicit N retains bounded hash candidates. Other backend policy consumers remain unchanged. In a shared-host three-BE cache/hash-off SUM experiment at concurrency 64, three completed alternating pairs increased QPS from 13.867 to 40.506 (2.9211x). The first attempt at pair three had a gRPC keepalive failure in original mode; its one-error 6.10 QPS group is retained separately and the complete pair was repeated after recovery. This is not customer or physical three-host validation. No COUNT, DOP or CPU-routing optimization is added. ### Release note Remote external scans automatically spread across eligible backends by default. Set external_scan_consistent_hash_spread_num to 1 to restore original scheduling. Automatic file-cache scans can increase cache copies and warmup reads. Persisted global settings are retained on upgrade. ### Check List (For Author) - Test: Unit Test / Regression test / Manual test - Standard FE build and Checkstyle; 32 focused FE tests passed. - Two targeted regression suites framework-generated and verified. - 400 correct SUM query profiles verified one split and one target. - Three completed paired C64 SUM measurements; interruption disclosed above. - Behavior changed: Yes, default automatic external scheduling with explicit rollback. - Does this need documentation: Yes, docs/external-scan-consistent-hash-spread.md included.
OIiveirra
added a commit
to OIiveirra/doris
that referenced
this pull request
Oct 8, 2026
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68726 Problem Summary: Spread scheduling disables global split redistribution, but still builds and retains alternative Host lists for every split. The only consumer of these lists is redistribution. Automatic mode considers all eligible backends, so these unused lists retain O(split count * backend count) Host references and allocate unnecessary temporary candidate copies. Build alternative Host lists only for the original scheduling mode. Preserve candidate selection, assigned weights, locality and split execution unchanged. Add tests for multiple automatic hash/random splits without retained lists and an original-mode control that still retains its alternative Host. ### Release note None ### Check List (For Author) - Test: Unit Test / Regression test - 33 focused FE unit tests passed (23 spread policy, 9 original policy, 1 variable). - Standard FE build and Checkstyle passed. - Parameter and external scan regression suites verified on the rebuilt FE. - Behavior changed: No, remove unused allocation without changing scheduling. - Does this need documentation: No, scheduling and parameter semantics are unchanged.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68726 Problem Summary: The single-instance COUNT rule counted assigned range envelopes, which can represent many physical splits in dynamic batch scans, and could apply to existing external-table COUNT paths. Inspect each worker's assigned scan source and restrict the shortcut to file TVF row-count scans with one static physical split. Preserve parallelism for dynamic split sources, multiple physical splits, and external tables, and clarify that upper aggregation still computes COUNT. Add four-format execution coverage and tests for the parallelism boundaries. ### Release note Limit single-instance file TVF COUNT execution to one assigned static physical split and preserve existing external-table and dynamic batch scan parallelism. ### Check List (For Author) - Test: Standard FE build and Checkstyle; 23 focused FE unit tests; CSV, JSON, Parquet and ORC file TVF regression suite; compressed S3 TVF regression with equivalent MinIO fixtures; isolated three-BE baseline/candidate manual tests. The standard Aliyun OSS profile requires valid credentials and was not validated. - Behavior changed: Yes. Dynamic and non-TVF COUNT scans retain their original parallelism; one static file TVF row-count split reduces fragment instances. - Does this need documentation: No
Contributor
TPC-H: Total hot run time: 28931 ms |
Contributor
TPC-DS: Total hot run time: 151509 ms |
Contributor
ClickBench: Total hot run time: 24.67 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: N/A Problem Summary: Stream load reads the base schema version without a table read lock. A concurrent schema change can remove the old base index metadata before replacing the base index ID, causing a null pointer exception. Use the existing timed read lock pattern around the returned table metadata and release it in finally. Add a deterministic test using the real table locks to reproduce the index replacement window. ### Release note Fix stream load failures caused by concurrent base index replacement. ### Check List (For Author) - Test: Unit Test: StreamLoadHandlerTest (4 tests passed). The new concurrency test reproduces the original null pointer exception before the fix. - Behavior changed: Yes. Metadata reads wait for concurrent schema changes, using the existing stream load table lock timeout. - Does this need documentation: No
### What problem does this PR solve? Issue Number: N/A Problem Summary: The cloud rowset property test repeats the same SELECT while checking BE capture counters. CI audit logs show that the query after setting the user property hits SQL cache with zero scanned rows, so the expected BE counter does not increase. Disable SQL cache in the test connection so each query reaches the BE. Keep all existing assertions unchanged. ### Release note None ### Check List (For Author) - Test: Manual test: Correlated the failing assertion with CI query audit records. The standard regression dry run discovered this suite; it exited with errors in unrelated cloud scripts. Cloud execution is pending CI; the personal environment is not a Cloud cluster. - Behavior changed: No - Does this need documentation: No
924060929
previously approved these changes
Oct 10, 2026
Contributor
|
/review |
liaoxin01
reviewed
Oct 10, 2026
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review complete for PR #68726 at 0be73f90a5f79ea36fdfdef63c910b30a57b8624 after two convergence rounds. I found two new P2 issues, attached inline. The existing P2 Project-expression thread (4228580328) is fixed at this head by the direct and multilayer NoneMovableFunction checks and is not repeated. No existing P0/P1 inline finding still applies. There was no additional user-provided focus.
Critical checkpoint conclusions:
- Goal and proof: The changes preserve initialized round-robin candidate order, enable existing file-reader COUNT handling for eligible file TVFs, reduce one-static-split instance overhead, and protect the regular stream-load schema-version read. FE unit and regression cases cover the main accepted and rejected COUNT shapes. The two inline findings limit the quality and completeness of the implementation.
- Scope and clarity: The execution changes are locally understandable. Reconstructing the host map in every assignment adds avoidable work; the schema-version lock should cover the parallel
load_sqlresult path. - Concurrency and locks:
SplitAssignment.addToQueueholdsassignLockwhile invoking backend assignment, so the new per-split map construction runs under that lock. The regular stream-load read lock uses the existing timed pattern and releases infinally; no new lock-order or deadlock was established. The HTTP stream-load result still reads base-index metadata after its planning locks are released and can race schema change. - Lifecycle and ownership: Static/dynamic file split sources, empty-scan fallback, V1 reader ownership, and V2 metadata-count fallback were traced. No new lifetime leak, circular reference, or static-initialization dependency was found.
- Configuration: No new configuration item is introduced. COUNT uses the existing session switches; dynamic split sources retain their prior parallelism.
- Compatibility and FE/BE contract: The TVF scan uses existing thrift COUNT fields and sends an explicit empty argument-slot list for
COUNT(*)/COUNT(1). V1/V2 readers either count range-scoped rows or fall back to record reads. No new storage format, function symbol, or persistence encoding is introduced. - Parallel paths and conditions: The matcher excludes grouped, distinct, filtered, nullable-column, buffer-consuming, and retained nonmovable-expression plans. The one-instance shortcut excludes dynamic and multi-range sources. The parallel HTTP
load_sqlschema-version path remains the exception raised inline. - Tests and expected results: The FE tests and local TVF regression matrix cover scanner V1/V2, pushdown enabled/disabled, empty and multiple files, and assertion behavior. All 132 named TVF snapshots match the suite, and text-fixture counts are consistent. The remote S3 object and binary fixture row contents were not independently executed or decoded. This review ran no builds or tests, as required by the review prompt.
- Observability: Existing user errors and FE logs cover the changed failure paths; no separate metric or logging defect was established.
- Transactions, persistence, and writes: The PR changes no EditLog, transaction protocol, or data-write implementation. The new read lock protects only one metadata snapshot; the distinct HTTP result snapshot remains exposed as described inline.
- Performance and other issues: The intended COUNT and single-instance savings are plausible from the code, but runtime speedups were not measured in this review. The additional host-map traversal and allocations on one-split streaming scans are reported inline. No other substantiated issue remained after the full changed-file sweep.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68726 Problem Summary: Streamed split assignment rebuilt the backend host map for every split. Cache the map and refresh it on initialization and candidate replacement. The load_sql stream load result also read schema version without a table lock during concurrent base index replacement. Read its table properties under the existing timed read lock pattern and release in finally. ### Release note Avoid repeated host map allocation during streamed scans and protect HTTP stream load metadata reads during schema change. ### Check List (For Author) - Test: Standard FE UT: FederationBackendPolicyTest, StreamLoadHandlerTest and HttpStreamLoadTablePropertiesTest, 18 tests passed. Maven Checkstyle and git diff --check passed. - Behavior changed: Yes. HTTP stream load table property reads wait for concurrent schema changes using the request RPC lock timeout. - Does this need documentation: No
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68726 Problem Summary: Merge upstream master e0f2ebb into the published PR branch. Resolve translator conflicts by retaining the COUNT pushdown and retained-expression guard alongside the upstream condition-cache initialization, and remove the obsolete Udf import. ### Release note None ### Check List (For Author) - Test: Standard FE UT: 42 tests passed across COUNT pushdown, condition cache, runtime-filter translation, backend scheduling and stream load suites. Maven Checkstyle passed; PR diff against upstream master passes git diff --check. - Behavior changed: No additional behavior change from the conflict resolution. - Does this need documentation: No
Collaborator
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28967 ms |
Contributor
TPC-DS: Total hot run time: 151728 ms |
Contributor
ClickBench: Total hot run time: 24.41 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
924060929
previously approved these changes
Oct 12, 2026
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Review complete at head 9fd223963b5f6da009282f1704ca046e870f096d. This is a static-only review: the review contract prohibited builds and tests, so inspected tests and their expected outputs are not runtime validation. I found one new P2 issue in the HTTP load_sql transaction cleanup path. No previously reported P0/P1 inline finding still applies; existing_blocking_comment_ids is empty. There was no additional user-provided review focus.
Critical checkpoints:
- Goal and tests: The changes target file-TVF COUNT pushdown, one-range scan scheduling, active backend selection, and schema-version reads during stream load. The planner, scheduling, and metadata reads follow the intended call paths, but the new HTTP lock timeout leaves a transaction open as described inline. FE unit tests and TVF/stream-load regressions cover the intended positive and negative cases in source; none was run here.
- Scope and clarity: The production changes are confined to those paths. The added lock helper keeps the property reads together; the missed transaction cleanup is the one focused correctness issue.
- Concurrency and locks: The FE stream-load RPC races with schema-change work that takes the same
OlapTablewrite lock. The new read locks protect lightweight metadata getters and release infinally; I found no new nested-lock inversion.InsertIntoTableCommand.initPlanreleases its read lock after starting the HTTP transaction, however, so a writer can occupy the interval before the new timed read lock and trigger M1. Streamed split assignment remains serialized by its existing lock. - Lifecycle: Normal HTTP
load_sqlstarts an FE transaction before result metadata assembly. Neither the FE exception path nor the BE failed-plan path aborts it on the new timeout. There is no new static initializer or ownership cycle. Ordinary stream load already gives the BE rollback ownership; group-commit HTTP skips the FE begin. - Configuration: No new configuration item is defined. The COUNT session gates and the request-provided thrift RPC timeout or existing default were checked in their callers; no dynamic-config propagation issue was substantiated.
- Compatibility and definitions: No function symbol, storage format, or
gensrc/thrift/gensrc/protodefinition changes. COUNT translation uses existing FE-to-BE fields, and the inspected BE readers retain normal-scan fallback for unsupported count paths. No distinct rolling-upgrade issue was substantiated statically. - Parallel paths and conditions: Direct/projected COUNT, V1/V2 readers, zero/multiple/static/dynamic splits, backend locality and redistribution, ordinary versus HTTP stream load, and group commit were traced separately. The project
NoneMovableFunctionguard and count eligibility conditions exclude the previously reported synthetic-value case; the existing backend host-map issue is also fixed at this head. No other conditional branch produced a distinct finding. - Coverage and expected results: The FE tests cover translator shapes, one-range scheduling, backend order, and lock ownership/release. The new TVF suite has four scanner/pushdown modes with 33 golden cases each; fixture cardinalities and the changed S3/cloud assertions were checked. The missing negative test is a forced post-begin HTTP lock timeout followed by an explicit-label retry, as noted inline. Golden files were inspected, not generated or executed in this review.
- Observability: Existing FE timeout status/logging and BE plan-error logging identify the failed request, while the transaction remains unclosed; no separate logging or metric finding was substantiated.
- Transactions, persistence, writes, and crashes: No new EditLog or replay format is introduced and no new data-writing operation is added. M1 leaves a PREPARE transaction after the new error path, blocking same-label retry until transaction-manager timeout. I found no separate data-visibility, failover, or crash-specific issue in the changed paths.
- FE-BE variables and performance: No new transmitted variable is introduced. The backend map is cached after candidate replacement, and the one-range COUNT path reduces local-shuffle instances; no independent performance regression was substantiated. The remaining supported COUNT readers still emit bounded batches for the existing upper aggregate.
- Other findings: The earlier inline P2 concerns were treated as duplicate fences and checked against this head. The final sweep of all 29 changed paths and the candidate ledger left only the new P2 finding below.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68726 Problem Summary: Baseline testing reproduces the stream load schema-version race before the COUNT optimization. Remove the independent table read-lock changes and their tests from this PR so they can be fixed separately, including HTTP transaction cleanup on lock failure. Preserve the active backend host map and all COUNT scheduling changes. ### Release note None ### Check List (For Author) - Test: Standard FE UT, 33 tests passed across FederationBackendPolicyTest, FileTvfCountPushDownTest, UnassignedScanSingleRemoteTableJobTest and StreamLoadHandlerTest. Maven Checkstyle and git diff --check passed. - Behavior changed: Yes. Restore upstream stream load metadata-read behavior pending a separate fix. - Does this need documentation: No
4 of 16 tasks
Collaborator
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29356 ms |
Contributor
TPC-DS: Total hot run time: 155054 ms |
Contributor
ClickBench: Total hot run time: 24.62 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
liaoxin01
approved these changes
Oct 13, 2026
924060929
approved these changes
Oct 13, 2026
Contributor
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: Review recovery stopped: PR base/head or open state changed; refusing to resume stale context Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Gabriel39
pushed a commit
that referenced
this pull request
Oct 15, 2026
### What problem does this PR solve? Issue Number: N/A Related PR: #68726 Problem Summary: Stream Load reads the base schema version from table metadata outside the planning lock. Concurrent schema change removes the old base-index metadata before switching the base index ID under the table write lock. An unlocked read in that interval can dereference missing metadata and fail the load. A deterministic unit test reproduces the null dereference on the pre-optimization baseline. For regular Stream Load, populate the response database ID, table ID, and base schema version inside the existing `generatePlan(OlapTable)` read-lock scope. Populate them only for the first planned table when a response is present, preserving multi-table behavior. Planning and response metadata use the same lock scope without an additional lock acquisition. For HTTP `load_sql`, obtain the response table ID and base schema version from the generated `OlapTableSink`. These values were already captured during planning under the existing table lock. Keep the other table-property reads as before. This removes the second metadata-lock acquisition and the transaction-abort branch introduced solely for its timeout. Existing transaction handling and Thrift fields are preserved. This fix is separate from the scheduling and file TVF COUNT optimization in #68726. ### Release note Prevent Stream Load response schema-version reads from racing with schema change by reusing the existing planning lock and planned sink metadata. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason Validation performed: - Standard focused FE UT reactor passed `StreamLoadHandlerTest` (14) and `HttpStreamLoadTablePropertiesTest` (2): 16 tests, zero failures/errors. Maven Checkstyle passed. - Unit coverage includes real concurrent base-index replacement with one read-lock acquisition, metadata read-lock ownership and exceptional release, the existing planning timeout, first-table response semantics, and planning without a response. HTTP tests verify planned metadata is used even when current base-index metadata is absent, for both regular loads and Group Commit, without another read-lock acquisition. - Standard ASAN FE/BE build, deployment, and health checks passed in the personal shared-nothing single-BE environment. P0 `load_p0/http_stream/test_stream_load_schema_metadata` and `load_p0/http_stream/test_http_stream_properties` passed with zero failed suites/fatal scripts. Existing regression snapshots were compared without regeneration or manual edits. - P0 `schema_change_p0/test_schema_change_unique_mow` also passed: one suite, zero failed suites/fatal scripts. All three targeted suites used the binaries built from this revised implementation. Source changes pass `git diff --check`. - The concurrency unit test controls the race window; SQL regression exercises compatibility and does not guarantee it hits that exact window. This local shared-nothing single-BE environment is not complete TeamCity or Cloud coverage. - Behavior changed: - [ ] No. - [x] Yes. Response metadata comes from the existing protected planning scope or the completed HTTP plan, avoiding an unlocked base-index lookup and additional lock waiting. Multi-table first-table behavior and protocol fields are preserved. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
Gabriel39
pushed a commit
that referenced
this pull request
Oct 15, 2026
#68791) ### What problem does this PR solve? Issue Number: N/A Related PR: #68726 Problem Summary: Repeated independently planned queries for a single remotely accessible external file split can select the same backend under consistent hashing. The candidate order is stable, each query starts with zero assigned weight, and the existing minimum-weight tie rule repeatedly selects the same candidate. Increasing the configured candidate count alone does not remove this hotspot. Add the session variable `external_scan_consistent_hash_spread_num` with default **1**, preserving existing scheduling: | Value | Behavior | | --- | --- | | `1` (default) | Preserve the original strategy, configured candidate counts, tie rule and global redistribution. This is a mode, not a one-backend limit. | | `0` | Enable automatic spreading across all eligible BEs in the query's compute group. Use the hash strategy when file cache or consistent hashing is enabled; otherwise use the random strategy. | | `N > 1` | Spread within at most N distinct hash candidates when file cache or consistent hashing is enabled; otherwise retain original round-robin scheduling. | In spread mode, select the candidate with the least query-local assigned split weight and break ties uniformly at random. Assign each split once, retain weight history across batches, and honor compute-group eligibility, local preference and mandatory Host constraints. Candidate counts are capped by the eligible BE count; the implementation is not specific to three BEs. Disable global split redistribution only in spread mode so it cannot move a split outside its candidate or locality constraints. Do not allocate unused alternative-host lists in this mode. With many splits, balancing is limited to each split's candidates and can be less even than the original global redistribution. This PR adds no live CPU monitoring, shared query counter, COUNT pushdown, DOP adjustment or BE cache protocol. It is independent of #68726 and does not include that PR's commits. The existing round-robin path remains available; when #68726 is merged, its round-robin fix also remains in effect for original-mode callers. The variable does not enable file caching. With caching enabled, automatic mode can warm copies on every eligible BE and increase remote warmup reads and cache storage. Persisted global values survive upgrades; restoring the compiled default to 1 does not reset an already saved global 0. `UNSET VARIABLE` restores the compiled session default 1. Manual performance evidence, collected on scheduling revision `7e4fb1c549d` before this final default-value adjustment: the same independent FE binary, three unchanged BEs on one shared physical host with distinct logical Hosts and disjoint nonexclusive four-core affinities; one million-row single-file CSV SUM; file cache and consistent hashing enabled; all candidate caches warmed; concurrency 64; 10-second warmup and 60-second measurement window; three alternating pairs changing only `spread=1` versus `spread=0`. | Mode | Mean successful QPS | | --- | ---: | | Original (`1`) | 13.3944 | | Automatic (`0`) | 39.2778 | The pooled ratio was **2.9324x**, with paired ratios 2.8782x–2.9603x. All six groups had zero query errors. QPS counts correct completions within the fixed window; late completions are excluded, all workers are joined, and latency includes the tail of queries started within the window. The original hot BE used 97.5%–98.7% of its four-core capacity; automatic mode used 87.2%–97.7% across the three BEs. Whole-invocation remote and peer read increments were zero. This evidence applies to the measured warm-cache shared-host workload. It is not a new benchmark of final revision `813826a7feb`, a cold-start result, a strict >=3x guarantee, a three-physical-host acceptance result or customer validation. The final adjustment changes the default and documentation, not the explicit spread-selection algorithm; spreading must now be explicitly enabled to seek this benefit. No throughput from the related COUNT optimization is attributed to this PR. ### Release note Add opt-in spreading for remote external file scan splits. `external_scan_consistent_hash_spread_num` defaults to 1 and preserves original scheduling. Set it to 0 to spread across all eligible BEs, or a value above 1 to bound hash candidates when file cache or consistent hashing is enabled. Spreading can increase cache copies and warmup costs. Persisted global values are retained on upgrade. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [x] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason Validation performed on the final revision: - Standard `run-fe-ut.sh --run 'FederationBackendPolicySpreadTest,VariableMgrTest#testExternalScanConsistentHashSpreadVariable,FederationBackendPolicyTest'`: 33 tests passed, zero failed/error/skipped. Coverage includes 1/2/3/5 BEs, random ties, multiple batches, locality, bounds, forwarding, original-mode compatibility, default 1 and explicit 0 strategy selection. - Standard `build.sh --fe --clean -j16` and integrated Checkstyle passed. An earlier production build failed with incomplete class/type errors; its evidence was retained and the clean build succeeded without unrelated source edits. - Standard `run-regression-test.sh` with an isolated personal configuration: - `-g p0 -d query_p0/ddl/set_and_unset_commands -s test_external_scan_consistent_hash_spread_variable`; output generated by the framework using `-forceGenOut`, then verified normally. - `-g external -d external_table_p0/tvf -s test_external_scan_consistent_hash_spread`; explicitly enables 0 for automatic-path coverage. - Both suites passed with zero failed/fatal/skipped suites. The fixture is generated by the suite in personal MinIO. - Isolated FE/BE health checks passed; BE process IDs and binary hashes were unchanged. No environment configuration or work log is included. - Separate manual warm-cache experiment on revision `7e4fb1c549d`, described above. Each mode was also certified by 100 query-owned profiles: correct result, one source split/range, one positive execution target, one million rows read, local-cache reads and no remote reads. Original mode selected one BE; automatic mode selected all three. - Standards and specification reviews of the implementation and final default adjustment found no remaining actionable issues. - Targeted local validation does not claim full TeamCity success on the updated head. No BE source changed, so no new BE build or BE UT run was required for this adjustment. - Behavior changed: - [ ] No. - [x] Yes, when explicitly enabled. Default 1 preserves the existing strategy. Values 0 and N>1 enable random minimum-weight ties and disable global redistribution in their applicable paths; caching can require more copies. - Does this need documentation? - [ ] No. - [x] Yes. apache/doris-website#4199 ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
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?
Issue Number: N/A
Related PR: #68964
Problem Summary:
Repeated queries against a single remotely accessible file split can repeatedly select the same backend when external file caching and consistent-hash scheduling are disabled. The round-robin scheduler reconstructed its candidate list from a host map, discarding the query-local randomized order selected during initialization. This change preserves that order for round-robin assignment and keeps local scheduling and split redistribution inside the active backend candidates.
For unfiltered, ungrouped
COUNT(*)andCOUNT(1)over File, HDFS, HTTP, Local, and S3 TVFs, the existing file COUNT pushdown is now used under the existing session switches. The instance reduction is limited to a file TVF row-count scan with exactly one assigned static physical split. Dynamicsplit_sourceentries, multiple physical splits in one range envelope, and existing external-table COUNT paths retain their original parallelism. DISTINCT, grouped aggregates, filtered plans, merge stages, and stream/ingest TVFs remain outside this path.The single-split CSV COUNT benchmark improved median QPS from 169.600 to 543.733 (3.206x) under the conditions below. Dynamic batch scans retain baseline fragment-instance parallelism.
Stream Load schema-version locking has been removed from this PR: a differential unit test reproduces the race on the pre-optimization baseline. It is handled separately in #68964, including HTTP transaction cleanup on table-lock timeout. This PR retains the external scan scheduling and file TVF COUNT changes.
Release note
Improve round-robin scheduling for external file scans and reduce execution overhead for unfiltered file TVF row-count queries with a single assigned static physical split.
Check List (For Author)
Test
Validation performed:
FederationBackendPolicyTest(12),FileTvfCountPushDownTest(5),UnassignedScanSingleRemoteTableJobTest(7), and the upstreamStreamLoadHandlerTest(9). Maven Checkstyle passed. This is functional validation of head0afd4837804; the benchmark numbers below are from the earlier measured revision.FederationBackendPolicyTest(12),FileTvfCountPushDownTest(4), andUnassignedScanSingleRemoteTableJobTest(7), 23 tests total, zero failures/errors. Parallelism tests cover dynamic split sources, multiple physical splits in a single range envelope, external-table exclusion, and actual local-shuffle instance assignment.test_file_tvf_count_pushdownpassed both framework snapshot generation and normal comparison (1 suite, zero failures/fatal scripts). It exercises CSV, JSON, Parquet, and ORC with scanner V1/V2 and pushdown enabled/disabled, including COUNT star/one/nullable/DISTINCT, filters, grouping, empty inputs, multiple files, and CSV headers/blank records/no final newline. Zero-byte JSON is included alongside nonempty files because standalone schema inference is unavailable.test_s3_tvf_compressionpassed against the personal MinIO equivalent using the same public compression fixtures (1 suite, zero failures/fatal scripts). The standard Aliyun OSS profile remains unvalidated because valid credentials are unavailable; community CI should rerun it with its configured credentials.COUNT(*), 1,000,000 rows, 64 clients): three baseline QPS values were 165.533 / 170.800 / 169.600; candidate values were 449.133 / 544.967 / 543.733. Median QPS increased from 169.600 to 543.733 (3.206x). This measures the combined scheduling and COUNT changes for this workload; it does not establish a scheduling-only multiplier.COUNT(value), 64 data files, 1,000,000 rows, expected count 500,000, 16 clients): baseline QPS 29.533 / 30.367 / 28.233, candidate 30.167 / 29.767 / 29.267; medians 29.533 / 29.767 (1.008x). Distributed plans assigned 192 scan fragment instances, 64 per BE, for both versions in all rounds, confirming dynamic sources keep their baseline instance parallelism. These are assigned fragment instances, not active scanner-thread counts. This throughput comparison is limited to the tested fixture and its observed variability.parallel_pipeline_task_num=64, file cache/consistent hash/SQL cache/query cache disabled, COUNT pushdown and scanner V2 enabled). Use 64 clients for the single-split CSV and 16 for the dynamic Iceberg batch; keep profiling disabled during measurement. All measurements require zero errors. The three-BE restart prevents stale cached FE RPC connections after version swaps; startup failures are excluded before measurement.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)