branch-4.2: [improvement](fe) Default external scans to bounded split batches (#69683) by 924060929 · Pull Request #69991 · apache/doris

924060929 · 2026-11-02T10:15:58Z

cherry pick from #69683

### What problem does this PR solve? Issue Number: None Related PR: apache#69683 Problem Summary: Adapt bounded external split delivery to branch-4.2 without importing master's connector modules or replacing FileScannerV2's existing physical-child/source-claim queue. Default scan batching thresholds to one, bound each backend queue by individual split descriptors, split producer and Hive partition work into configurable small batches, and publish the first sample as soon as a range is enqueued so a producer batch larger than its queue cannot block scan initialization. Distinguish temporary empty split responses from EOF with the existing INCOMPLETE status; retry them on BE with query cancellation and local scanner/connector stop checks. Reuse the branch's session variable validators and preserve the external batch-mode switch. ### Release note Use bounded split batches by default for external scans and avoid treating temporary split-listing delays as end of input. ### Check List (For Author) - Test: Unit Test: 213 selected FE tests and full build.sh --fe package build with Checkstyle passed; 57 relevant BE ASAN tests passed via run-be-ut.sh using local focused test-source selection. Native FileScannerV2 child/source queue tests were retained. Exact clang-format 16 check passed. Full BE test harness was blocked by unrelated ANN test compilation; full BE packaging and external-service regressions were not completed locally. clang-tidy was blocked by the unchanged core/types.h unmatched NOLINTEND annotation. Local compiler-warning/CMake adaptations were used for macOS validation and excluded from this commit. - Behavior changed: Yes, external scan thresholds and split/partition batch defaults change; live empty split fetches are retried until completion or cancellation. - Does this need documentation: No

hello-stephen · 2026-11-02T10:16:04Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

924060929 · 2026-11-02T10:16:27Z

run buildall

924060929 · 2026-11-02T10:16:28Z

/review

github-actions · 2026-11-02T11:42:55Z

Codex automated review failed and did not complete.

Error: Selected model is at capacity. Please try a different model.
Workflow run: https://github.com/apache/doris/actions/runs/41427482768

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

### What problem does this PR solve? Issue Number: None Related PR: apache#69683 Problem Summary: Enabling batch planning for small external scans leaves Iceberg partition counts at zero and split counts approximate when SQL block rules run before execution. It also removes partition and delete-file details from EXPLAIN. Require the existing synchronous planning path for EXPLAIN and enabled applicable partition_num/tablet_num rules, with one shared eligibility gate for Hive and Iceberg. Keep ordinary unrestricted queries on bounded asynchronous split delivery. Rule applicability and the internal-query bypass remain owned by SqlBlockRuleMgr. ### Release note Preserve external-table partition and split SQL limits and complete EXPLAIN scan details when batch split planning is enabled. ### Check List (For Author) - Test: Unit Test - Reproduced the two eligibility regressions before the fix; all 196 related FE unit tests pass after the fix. - Full FE packaging and Checkstyle pass through build.sh --fe -j4 (UI build disabled). - External-service regression remains for remote CI; no BE code changed or BE validation rerun. - Behavior changed: Yes (use exact synchronous scan planning for EXPLAIN and applicable partition/split limits) - Does this need documentation: No

924060929 · 2026-11-02T12:48:10Z

run buildall

924060929 · 2026-11-02T12:48:12Z

/review

github-actions

Reviewed PR #69991 at the required head 0cae86d. The review converged after two bounded rounds: all assigned FE/BE scopes were covered, the latest interaction recheck returned NO_NEW_VALUABLE_FINDINGS, and all candidates were verified, deduplicated, or dismissed. Five P1 correctness issues are included inline: the new exact-planning guard is bypassed by Hudi and MaxCompute; the batch-mode threshold also forces one MaxCompute session/task per partition; the new INCOMPLETE response is incompatible with mixed-version FE/BE upgrades; non-positive fetch_splits_max_wait_time_ms creates an unbounded retry loop; and a legacy scanner stopping at a local LIMIT can leave a full FE queue that starves other backends. No existing blocking inline findings remain applicable. No additional user focus was provided. This was a static review; tests and builds were inspected but not executed.

Review converged after 2 round(s).

github-actions · 2026-11-02T14:03:56Z

            admissionResult = fileCacheAdmissionCheck();
        }

-        if (isBatchMode()) {


[P1] Apply the new exact-planning guard to every batch-capable file scan. createScanRangeLocations() dispatches to HudiScanNode.isBatchMode() and MaxComputeScanNode.isBatchMode(), but those overrides never call isBatchModeEnabled(). With the changed defaults of one partition/file, a Hudi or MaxCompute query enters batch mode during EXPLAIN or when a partition_num/tablet_num SQL block rule is active; FileQueryScanNode then reports numApproximateSplits() (Hudi can still be zero while async listing has not populated numSplitsPerPartition, and MaxCompute uses the partition count) to StmtExecutor.checkBlockRulesByScan. This can show approximate EXPLAIN details and admit or reject a query using the wrong split count. Make the common guard apply to these overrides as well and add equivalent coverage for each batch-capable subclass.

Addressed in abcbb5c. Hudi and MaxCompute now call the common batch-mode eligibility gate before their connector-specific planning. Applicable scan-count admission rules and EXPLAIN VERBOSE use complete split metadata. Ordinary EXPLAIN still describes the actual lazy execution plan, preserving the Arrow Flight batch-path check. The related FE suites cover the connector gates, exact admission/verbose metadata, and ordinary EXPLAIN batching.

github-actions · 2026-11-02T14:03:58Z

                    "If the number of partitions exceeds the threshold, scan ranges will be got through batch mode."},
            needForward = true)
-    public int numPartitionsInBatchMode = 1024;
+    public int numPartitionsInBatchMode = 1;


[P1] Keep MaxCompute's partition batching independent of the batch-mode threshold. MaxComputeScanNode.startSplit() uses getNumPartitionsInBatchMode() as batchNumPartitions, so this new default of 1 submits one TableBatchReadSession task per selected partition. Large tables can exceed the fixed scheduleExecutor queue; rejected submissions leave numFinishedPartitions short of the total, and the split source fails instead of completing. Use a dedicated bounded batch size (or preserve a larger MaxCompute default) and handle rejected tasks with terminal accounting.

Addressed in abcbb5c. MaxCompute now groups partitions using the dedicated split_partition_batch_size (default 16), independently of the threshold that enables batch mode. Each scan submits only one continuation at a time, instead of submitting every group together. Initial/continuation rejection and session failure publish the error and finish the schedule. Tests cover 33 partitions grouped as 16/16/1, the single queued continuation, both rejection paths, and cancellation.

github-actions · 2026-11-02T14:04:00Z

+            result.status = new TStatus(locations.isEmpty() && !splitSource.isLastBatch()
+                    ? TStatusCode.INCOMPLETE : TStatusCode.OK);
            return result;
        } catch (Exception e) {


[P1] Preserve FE/BE rolling-upgrade compatibility for the new INCOMPLETE response. This FE now returns TStatusCode.INCOMPLETE whenever a live SplitSource is temporarily empty, but the previous BE connector treats every non-OK status as an error and aborts the query. A new BE talking to an old FE has the reverse problem because the old FE turns the same temporary empty condition into INTERNAL_ERROR, which the new connector also returns. A mixed-version FE/BE pair will therefore fail lazy external scans during the normal wait window. Add a compatibility path or gate the status change until both sides understand the retry contract.

I checked both directions against the supported BE-first upgrade sequence and the release-branch implementations. All BEs are upgraded before FEs, so an old BE does not receive the new FE retry status in that sequence. With a new BE and old FE, the old SplitSource still returns its existing INTERNAL_ERROR on an empty-wait timeout; the old connector already treated any non-OK status as a failure. That failure is therefore not introduced by INCOMPLETE. Under the supported upgrade contract, no extra retry compatibility path is required for an intermediate unreleased 4.2 build. The source-close extension in abcbb5c is separately capability-gated by an optional TSplitSource.supports_close field: a new BE sends no close request to an old FE, even if the scan stops before its first fetch.

github-actions · 2026-11-02T14:04:02Z

-        while (scanRanges.size() < maxBatchSize && System.currentTimeMillis() - startTime < maxWaitTime) {
-            BlockingQueue<Collection<TScanRangeLocations>> splits = splitAssignment.getAssignedSplits(backend);
+        long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(maxWaitTime);
+        while (scanRanges.size() < maxBatchSize) {


[P1] Reject or handle non-positive fetch_splits_max_wait_time_ms before enabling the retry protocol. With a value of 0 (or negative), getNextBatch()'s deadline is already expired, so every live call returns an empty INCOMPLETE response immediately. The BE's new unbounded INCOMPLETE loop then floods the FE with RPCs and burns CPU until split production finishes; the previous implementation surfaced one timeout error instead. Add validation or a bounded backoff/stop condition for this setting.

Addressed in abcbb5c. The session-variable checker rejects fetch_splits_max_wait_time_ms <= 0. Planning also checks the value before starting split generation, and SplitSource enforces the same constructor precondition for programmatic/persisted values. The wait uses elapsed-time arithmetic rather than adding a deadline that could overflow. Tests cover zero/negative values and positive waits; a live empty batch still waits before returning INCOMPLETE.

github-actions · 2026-11-02T14:04:04Z

+                    backend.getId(), be -> new LinkedBlockingQueue<>(queueCapacity));
+            for (TScanRangeLocations location : locations) {
+                while (needMoreSplit()) {
+                    try {


[P1] Release or mark a backend's split source when its scanner stops at a local LIMIT. appendBatch() now blocks on each finite per-backend queue while needMoreSplit() is globally true, but the legacy BE path only passes a local scanner_stop and never tells FE that this backend will not consume again. After that queue fills, the producer can remain in the offer loop and stop publishing to other backends, whose fetches stay INCOMPLETE indefinitely. Add per-source cancellation/queue closure or make publication skip stopped consumers.

Addressed in abcbb5c. The scan operator owner now releases its shared remote source after Base::close requests scanner stop and releases ScannerContext's transfer lock; no RPC runs inside scanner try_stop. FE marks only that backend consumer closed, clears its bounded queue, skips subsequent publication to it, and clears an offer that races closure. Other backends keep consuming; closing the last one stops the assignment. Optional descriptor capability gates the new close request for old FEs. The two-backend FE test forces the full-queue/close/offer race and verifies the other backend still receives its split. BE tests cover retry, idempotent owner close, old-FE capability absence, and an in-flight fetch returning after closure, alongside the existing native V2 queue/cancellation suites.

I rechecked the carried-forward rolling-upgrade concern against the base release. It already has a 10,000-batch producer queue and stops scanners at a local LIMIT. Its live empty wait throws Timeout, which FrontendService maps to INTERNAL_ERROR. Thus the old FE's per-backend termination gap predates this PR. The new BE still propagates that error after one fetch; the added OldFrontendTimeoutRemainsAnErrorWithoutRetry test verifies one fetch and no unsupported close RPC with capability absent. I corrected the close-path comment that had incorrectly claimed older FEs had no bounded-queue dependency. With the supported BE-first upgrade, the new FE retry/close protocol is used only after BEs support it; an old FE retains its existing protocol and behavior. The new-FE close/queue-release fix and capability gate remain unchanged. The clarification and regression test are in b76e398.

hello-stephen · 2026-11-02T14:44:41Z

BE UT Coverage Report

Increment line coverage 27.59% (8/29) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	64.75% (30325/46835)
Line Coverage	49.61% (320836/646668)
Region Coverage	45.41% (258589/569505)
Branch Coverage	47.07% (122215/259660)

### What problem does this PR solve? Issue Number: None Related PR: apache#69683 Problem Summary: Hudi and MaxCompute bypassed common exact-planning eligibility, MaxCompute used the batch-mode threshold as its task size and queued all partition groups, and non-positive split-fetch waits caused immediate retries. A backend stopping at a local LIMIT could also leave its bounded FE queue blocking publication to the remaining backends. Apply the common eligibility gate, use a dedicated bounded MaxCompute continuation, validate the wait before starting producers, and release each stopped consumer from the scan operator owner through a capability-gated optional request. Preserve ordinary EXPLAIN batch plans, require full metadata for VERBOSE/admission checks, and retain native Hive listing root causes. ### Release note Fix hangs and excessive scheduling during lazy external-table scans, preserve complete scan-count admission and verbose plan metadata, and retain Hive missing-partition diagnostics. ### Check List (For Author) - Test: 205 related FE unit tests passed through run-fe-ut.sh; 60 related BE ASAN unit tests passed through run-be-ut.sh. Full FE package/Checkstyle and full BE ASAN package passed through build.sh. BE validation uses a local-only source-specific CMake warning adjustment and a focused native test target; neither is committed. External-service regression cases are updated and await CI. Changed-line clang-tidy is blocked by the existing unmatched NOLINTEND in be/src/core/types.h:578. - Unit Test / Regression test updates - Behavior changed: Yes; bounded split delivery now releases stopped backend consumers, validates wait intervals, and preserves exact planning where required. - Does this need documentation: No

924060929 · 2026-11-02T16:00:30Z

run buildall

924060929 · 2026-11-02T16:00:32Z

/review

github-actions

Reviewed PR #69991 at head abcbb5c. The change improves bounded external split delivery, exact planning eligibility, and partition batching. Static review covered every changed path, BE scanner shutdown and retry/close races, FE queue ownership and producer termination, all four batch-capable connectors, thrift compatibility on branch-3.1/4.0/4.1/4.2, and the affected tests/regression expectations. One new P2 is included inline: Hive now estimates a whole partition batch as one partition, so ordinary EXPLAIN and total-file-size estimates are inflated (six ten-file partitions become 360 instead of 60). Existing P1 inline finding 4443306717 remains applicable in the capability-absent new-BE/old-FE rolling-upgrade path: the new BE stops fetching at a local LIMIT but cannot send close_source, while the old FE can still fill its finite producer queue and starve other backends. The other existing P1 findings were independently revalidated as addressed and are not reposted. No additional user review focus was provided. This was a static review; tests and builds were inspected but not executed. The review converged after two rounds: the final independent sweep returned no new valuable findings and every candidate was verified, deduplicated, or dismissed.

Review converged after 2 round(s).

Existing P0/P1 findings confirmed for this head: #69991 (comment)

github-actions · 2026-11-02T17:02:18Z

                        try {
                            List<Split> allFiles = Lists.newArrayList();
                            getFileSplitByPartitions(
-                                    cache, Collections.singletonList(partition), allFiles, bindBrokerName,


[P2] Preserve Hive's per-partition approximate split estimate when batching partitions. getFileSplitByPartitions() now receives the whole partitions batch, so allFiles.size() at this line is the total for (up to) 16 partitions. The unchanged numApproximateSplits() multiplies that value by prunedPartitions.size() again; six ten-file partitions therefore report 360 instead of the regression's expected 60 and inflate totalFileSize/split estimates. Track the per-partition maximum (or normalize the batch total) before multiplying.

Fixed in b76e398. Hive now counts the produced splits by their actual partition values within each scheduling batch, then atomically keeps the largest per-partition count. The existing minimum of ten per partition remains intact. This also covers TABLESAMPLE and unpartitioned scans, without repeating metadata reads or file splitting. The real asynchronous split-generation tests reproduced 360 instead of 60 (and 116 instead of 100) before the repair; all four cases now pass across batch sizes 1/2/16, including unequal/empty partitions. All 209 related FE tests and full FE packaging/Checkstyle pass locally.

### What problem does this PR solve? Issue Number: None Related PR: apache#69683 Problem Summary: Hive split generation now schedules several partitions per task, but recorded the whole task output as the split count of one partition. Extrapolating this across all pruned partitions inflated approximate split/file-size estimates: six partitions with ten splits each reported 360 instead of 60. Count actual output by partition values within each batch and atomically retain the largest per-partition count. Preserve the existing minimum and cover uneven/empty partitions, sampling, unpartitioned scans, and different scheduling batch sizes. Correct the legacy-FE closure comment and verify that its existing timeout error is propagated without retries or an unsupported close request. ### Release note Correct approximate Hive split counts and derived file-size estimates during batched external scans. ### Check List (For Author) - Test: 209 related FE unit tests passed through run-fe-ut.sh. The unchanged-production reproduction showed 360 instead of 60 and 116 instead of 100; all four new cases now pass. Full FE packaging and Checkstyle passed through build.sh --fe. 61 related BE ASAN tests passed through run-be-ut.sh, including the new old-FE timeout/no-retry case. BE test validation uses the existing local-only focused CMake target and source-specific compiler-warning adjustment, restored afterwards and excluded from this commit. - Unit Test - Behavior changed: Yes; approximate Hive estimates are independent of scheduling batch size and preserve the per-partition maximum. - Does this need documentation: No Changed-line clang-tidy remains blocked by the existing core/types.h:578 unmatched NOLINTEND and the pre-included gtest/internal/gtest-internal.h:56 float.h diagnostic.

924060929 · 2026-11-02T17:44:23Z

run buildall

924060929 · 2026-11-02T17:44:25Z

/review

github-actions

Reviewed the complete PR diff and related FE/BE control flow at head b76e398. No new blocking or non-blocking finding is verified. The earlier exact-planning, MaxCompute partition batching, INCOMPLETE retry/rolling-upgrade, bounded queue close, and Hive split-estimate findings were rechecked and are fixed in this head. All changed paths, connector overrides, queue/producer terminal paths, thrift compatibility fields, and affected tests/regression expectations were covered. No additional user review focus was provided. This was a static review; builds and tests were inspected but not executed.

Review converged after 1 round(s).

hello-stephen · 2026-11-02T18:15:36Z

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	76.44% (2005/2623)
Line Coverage	64.95% (36147/55652)
Region Coverage	52.93% (34602/65367)
Branch Coverage	55.75% (10772/19322)

### What problem does this PR solve? Issue Number: N/A Related PR: apache#69683 Problem Summary: The split-consumer close concurrency test initializes the large Env and frontend RPC service mocks inside its 30-second method timeout. CI logs show RPC static initialization still running after that budget, before the producer thread is created. Move these class fixtures to BeforeAll, following existing bounded-concurrency tests, so cold initialization does not consume the queue-race budget. Keep the original 30-second timeout, five-second phase bounds, close-race assertions, RPC idempotence, and executor cleanup unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test: controlled 1-second budget fails before the fixture move and passes afterward; 63 related FE tests pass with the original 30-second limit; full FE packaging and Checkstyle via build.sh --fe - Behavior changed: No - Does this need documentation: No

924060929 · 2026-11-02T19:28:55Z

run buildall

924060929 · 2026-11-02T19:28:56Z

/review

hello-stephen · 2026-11-02T19:53:19Z

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	76.44% (2005/2623)
Line Coverage	64.90% (36116/55652)
Region Coverage	52.77% (34491/65367)
Branch Coverage	55.73% (10769/19322)

github-actions

Static review of PR 69991 at head 58ebd37 converged after 2 bounded rounds. I verified two new P1 issues: Hive TABLESAMPLE targets are recomputed per asynchronous partition batch, changing table-wide ROWS/PERCENT sampling under the new defaults; and RemoteSplitSourceConnector sets its close latch before the close RPC succeeds, so a transient failure cannot retry, can leave the FE consumer queue live, and can cancel or stall the remaining scan. Existing P1/P2 threads were revalidated against this head and are fixed or pre-existing under the documented upgrade order, so no existing blocking IDs are carried forward. The queue lifecycle, positive wait validation, MaxCompute continuation scheduling, exact-planning guards, and optional Thrift compatibility fields were also checked with no additional findings. No user-provided focus points were supplied. This was a static-only review: changed tests and regression coverage were inspected, but builds and tests were not executed.

Review converged after 2 round(s).

github-actions · 2026-11-02T19:56:48Z

                        try {
                            List<Split> allFiles = Lists.newArrayList();
                            getFileSplitByPartitions(
-                                    cache, Collections.singletonList(partition), allFiles, bindBrokerName,


[P1] Preserve one table-wide TABLESAMPLE target across partition batches. startSplit() now calls getFileSplitByPartitions() once per Lists.partition batch, but that method runs selectFiles(fileCaches) with the full TABLESAMPLE(N ROWS) target each time. A 32-partition table with the default batch size 16 therefore selects roughly 2N rows/files, whereas the non-batch path computes one target over all partitions; percent sampling also changes through per-batch rounding. Carry the remaining sample budget or materialize the sample once before batching, and add a multi-batch ROWS regression test.

Fixed in 4294427. TABLESAMPLE now opts out of lazy partition batching at the common eligibility gate. The existing eager Hive path selects files once across all pruned partitions, preserving a single ROWS byte budget, percentage rounding and seeded shuffle. The metadata-path tests drive the real planning-mode decision over 32 partitions at batch sizes 16/2/1. Both ROWS and percent reproduced two selected files instead of one before the fix; all 236 related FE tests and full FE packaging/Checkstyle now pass locally.

github-actions · 2026-11-02T19:56:50Z

+
+Status RemoteSplitSourceConnector::close() {
+    stop();
+    // Older FEs do not advertise support for the source-close request.


[P1] Keep source-close cleanup retryable until FE acknowledges it. _close_requested is set before _fetch_batch(), so a transient transport/FE failure leaves the source registered when the request did not reach FE, while all later close calls return immediately. On a large scan this can refill that backend's bounded queue and stall split delivery to other backends after a local LIMIT; the non-OK close status also cancels the query. Set the latch only after success or retain a bounded retry/best-effort cleanup path, and avoid turning a transient cleanup failure into the scan's close status.

Fixed in 4294427. A separate close mutex serializes cleanup without waiting for the consuming-fetch lock, and the completed flag is set only after acknowledgement. Close performs a bounded retry, using the standard ThriftRpcHelper reconnect/retry path only for this idempotent request; ordinary consuming fetches are not retried. FE already returns OK when the source was removed, so a lost reply is safe. Exhausted cleanup errors deliberately still propagate to query cancellation: silently ignoring them could strand the finite queue and block other BEs. A later close remains retryable after failure. Tests cover loss before FE execution, a lost reply after execution, bounded transport/FE failures and recovery; all 63 focused BE ASAN tests, 236 related FE tests and full BE ASAN/FE packaging pass locally. This is a focused BE test target, not the complete BE UT suite.

### What problem does this PR solve? Issue Number: N/A Related PR: apache#69683, apache#69991 Problem Summary: Hive lazy partition listing applies TABLESAMPLE independently to every scheduling batch, resetting the ROWS byte target and percentage rounding. A sampled scan now uses the existing complete-metadata path to select files once across all pruned partitions, preserving its seeded table-wide selection. Remote split-source cleanup previously recorded completion before FE acknowledgement, so a temporary RPC failure could prevent subsequent cleanup and strand a bounded producer queue. Serialize close acknowledgements independently of consuming fetches and retry only the idempotent cleanup request through the standard Thrift reconnect path. Mark completion after acknowledgement; exhausted errors still cancel the query so other backends cannot wait indefinitely behind an unclosed queue. ### Release note Fix Hive TABLESAMPLE over-selection with partition batching and recover split-source cleanup from temporary RPC failures. ### Check List (For Author) - Test: Unit tests and packaging - 236 related FE tests passed; full FE packaging and Checkstyle passed. - 63 focused BE ASAN tests passed on the final lock/test-helper implementation. - Full BE ASAN packaging (including Java scanning extensions) passed. - clang-format 16 and git diff --check passed. Changed-line clang-tidy found no remaining new diagnostics; it remains blocked by the pre-existing core/types.h NOLINTEND and forced gtest float.h header diagnostics. - BE tests use a focused native target; the complete BE UT suite and external-service regressions were not run in this cycle. Local CMake build adaptations are excluded from this commit. - Behavior changed: Yes. Sample files are selected once over all pruned partitions; source closure retries temporary failures and records completion only after acknowledgement. Ordinary consuming fetches retain their existing transport-error behavior. - Does this need documentation: No

924060929 · 2026-11-02T21:45:41Z

run buildall

924060929 · 2026-11-02T21:45:43Z

/review

github-actions

Reviewed PR 69991 at the verified base a623df2 and head 4294427. Round 1 converged with complete changed-file coverage and no new substantiated inline finding.

The review traced the bounded FE split queues, scanner stop propagation, owner/source close lifecycle, idempotent close retry, FE source removal, Hive/Hudi/Iceberg/MaxCompute scheduling, exact-planning guards, TABLESAMPLE, EXPLAIN VERBOSE, SQL block-rule admission, and the new optional Thrift fields across branch-4.2, branch-4.1, branch-4.0, and branch-3.1. Existing inline concerns were revalidated against this head and found fixed or covered. The owner-level close-latch concern is the same lifecycle issue as the existing source-close thread; exhausted cleanup errors cancel the query and FE removes the source, so it is not reposted.

No additional user review focus was supplied. Review evidence is static only: changed tests and regression coverage were inspected, but no builds or tests were executed.

Review converged after 1 round(s).

hello-stephen · 2026-11-02T23:44:17Z

BE UT Coverage Report

Increment line coverage 71.28% (67/94) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	64.75% (30331/46840)
Line Coverage	49.62% (320911/646739)
Region Coverage	45.41% (258628/569567)
Branch Coverage	47.08% (122261/259705)

… explain ### What problem does this PR solve? Issue Number: N/A Related PR: apache#69683, apache#69991 Problem Summary: The Iceberg Variant remote-gather fixture asserts inputSplitNum=2 to prove that its selected snapshot contributes both shredded and unshredded files. With external batching enabled by default, ordinary EXPLAIN intentionally reports a sampled approximate split count, and the external regression fails on approximate inputSplitNum=1 before executing the mixed-file result/profile checks. Select EXPLAIN VERBOSE for this single action through the existing regression DSL so the shared exact-planning guard enumerates both files. Preserve the exact split-count, merge-exchange and projected-path assertions, all subsequent data/profile checks, and default batching for executable queries. ### Release note None ### Check List (For Author) - Test: Existing FE unit tests and direct regression-call-chain review - The current-head external CI failure and exact revision were captured from build 1089454. - Both testVerboseExplainRequiresExactFilePlanning and testOrdinaryExplainKeepsTheRealBatchPlan passed using run-fe-ut.sh (2 tests, no failures/errors/skips). - Integrated FE Checkstyle, source hash verification and git diff --check passed. - The live external regression was not run locally: the configured Iceberg REST and MinIO endpoints are unavailable. New-head CI must validate the complete suite. - No FE/BE repackaging was needed for this regression-only DSL change. - Behavior changed: No product behavior change. Only the fixture EXPLAIN requests exact planning; executable queries retain their batching settings. - Does this need documentation: No

924060929 · 2026-11-03T00:09:31Z

run buildall

924060929 · 2026-11-03T00:09:32Z

/review

github-actions

Static review of PR 69991 at head 442f8a3 converged after 2 bounded rounds. No new P0-P3 defect was substantiated, so there are no inline comments. The review covered all 33 changed paths, including BE scanner/source-close lifecycle and concurrency, FE bounded split queues and INCOMPLETE polling, exact-planning gates for Hive/Hudi/Iceberg/MaxCompute, Hive sampling and per-partition estimates, MaxCompute continuation/rejection accounting, SessionVariable validation, and both optional thrift fields against the supported release branches. The close/fetch paths use atomic stop visibility, independent close serialization, idempotent acknowledged cleanup, and coordinator cleanup on exhausted close failure; bounded queue publication handles stopped consumers and offer/close races. Positive wait and batch-size validation prevents the prior retry hot loop, and the optional supports_close/close_source fields remain capability-gated for rolling compatibility. Existing P1 threads 4443305397, 4443305711, 4443306004, 4443306341, 4443306717, 4447064294, and 4447064651 were independently revalidated at this head as fixed or deliberately compatibility-gated, so none is carried forward in existing_blocking_comment_ids. The supplied review_focus.txt contains no additional user focus. Changed tests and regressions were inspected as evidence; no builds or tests were executed under the static-only review contract.

Review converged after 2 round(s).

hello-stephen · 2026-11-03T02:08:22Z

BE UT Coverage Report

Increment line coverage 71.28% (67/94) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	64.76% (30332/46840)
Line Coverage	49.64% (321039/646739)
Region Coverage	45.43% (258782/569567)
Branch Coverage	47.08% (122280/259705)

1

hello-stephen · 2026-11-03T03:47:14Z

BE Regression && UT Coverage Report

Increment line coverage 74.47% (70/94) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	76.29% (34705/45491)
Line Coverage	61.18% (392341/641339)
Region Coverage	57.96% (330603/570424)
Branch Coverage	58.90% (152774/259393)

github-actions · 2026-11-03T03:48:13Z

PR approved by at least one committer and no changes requested.

924060929 requested a review from yiguolei as a code owner November 2, 2026 10:16

github-actions Bot requested changes Nov 2, 2026

View reviewed changes

github-actions Bot reviewed Nov 2, 2026

View reviewed changes

github-actions Bot previously requested changes Nov 2, 2026

View reviewed changes

github-actions Bot reviewed Nov 2, 2026

View reviewed changes

github-actions Bot reviewed Nov 3, 2026

View reviewed changes

yiguolei approved these changes Nov 3, 2026

View reviewed changes

yiguolei merged commit fa07b7c into apache:branch-4.2 Nov 3, 2026
29 of 31 checks passed

github-actions Bot added the approved Indicates a PR has been approved by one committer. label Nov 3, 2026

Conversation

924060929 commented Nov 2, 2026

Uh oh!

hello-stephen commented Nov 2, 2026

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

github-actions Bot commented Nov 2, 2026

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hello-stephen commented Nov 2, 2026

BE UT Coverage Report

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

hello-stephen commented Nov 2, 2026

Cloud UT Coverage Report

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

924060929 commented Nov 2, 2026

Uh oh!

hello-stephen commented Nov 2, 2026

Cloud UT Coverage Report

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!