morningman commented on PR #68712:
URL: https://github.com/apache/doris/pull/68712#issuecomment-5978499430
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#68712
commit: 57146a7b8c67b1dd35f267b4101ef1a033c79f87
base: 0dcd2a31a7bb00ba9a817c2600f64e770efea54d
reviewed_at: 2026-10-04T17:25+08:00
reviewer: morningman
model: claude-fable-5-1
effort: max
findings: {blocker: 0, major: 0, minor: 1, nit: 2}
rounds: 1
converged: true
```
**Notes for maintainers**
- Residual risk, already discussed in thread r4175757746:
`be/src/exec/scan/scanner_scheduler.cpp:282-283` — the memory limiter budgets
`Block::allocated_bytes()` only, so the BE JVM heap held by JNI readers is
invisible to the admission this PR lets rise to 16 per instance. The merge
order after #68710 is procedural (nothing in code enforces it) and #68713's
heap admission is opt-in, so a heavy paimon merge-on-read or fluss primary-key
read can fail with `OutOfMemoryError` at the default concurrency where it ran
at one scanner before; `max_file_scanners_concurrency` is the knob.
- F-01 (Minor, observability) `be/src/exec/scan/scanner_context.cpp:174-177`
— the adaptive ceiling now binds by default but is VLOG-only; the profile has
`MaxScannerThreadNum`/`RunningScannerPeak` and nothing for the ceiling, so "why
did my scan run below its maximum" needs a verbose rerun. Worth a
high-water-mark counter before a backport.
- F-03 (Nit, test-coverage)
`regression-test/suites/external_table_p0/cache/test_file_cache_query_limit.groovy:57`
— the suite pins scanners per instance but not `parallel_pipeline_task_num`;
the condition-cache suites the PR cites pin both, and the best-effort per-query
bound can still trip on a core-rich regression box.
- Not verified locally: no build or test run. Evidence at this exact head
comes from CI: BE UT build 1062129 (14,322 passed) and External Regression
build 1062136 (703/703, including the adjusted suite); the earlier External
failure (build 1062095) was on the PR's first commit, before the groovy pin.
<sub>Reviewed locally with the `doris-repo-review` pipeline. Repository
policy may accept this receipt for the matching commit; it is not a human
Apache approval.</sub>
<!-- doris-repo-review:v1:end -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
