Skip to content

[branch-4.0] pick some arrow memory leak fix to branch-4.0 - #66462

Merged
morningman merged 2 commits into
apache:branch-4.0from
yiguolei:branch-4.0
Aug 5, 2026
Merged

[branch-4.0] pick some arrow memory leak fix to branch-4.0#66462
morningman merged 2 commits into
apache:branch-4.0from
yiguolei:branch-4.0

Conversation

@yiguolei

@yiguoleiyiguolei commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

pick #64799#66437

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • 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

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

morningmanand others added 2 commits August 5, 2026 11:22
@morningman@yiguolei
[fix](arrow-flight) Keep coordinator alive across GetFlightInfo/DoGet…
514da84
… for external table scan (apache#64799)
Issue Number: closeapache#62259
Related PR: apache#64797
Problem Summary:
Arrow Flight SQL queries against Iceberg (and other external) tables in
batch split mode crashed the BE / failed with `Split source X is
released`.
Arrow Flight executes a query in two phases: `GetFlightInfo` (plan +
submit to BE) and `DoGet` (the client pulls results from the BE). For an
external table scan in batch split mode, the BE keeps scanning during
`DoGet` and lazily fetches file splits from the FE via the
`fetchSplitBatch` RPC, using an async `SplitSource` that the FE
coordinator holds (through its scan nodes).
The FE closed the coordinator at the end of `GetFlightInfo`
(`StmtExecutor.executeAndSendResult`'s `finally` → `Coordinator.close()`
→ `ScanNode.stop()` → `SplitSourceManager.removeSplitSource()`) and also
unregistered it (`FlightSqlConnectProcessor.close()` →
`StmtExecutor.finalizeQuery()`). So by the time the BE called
`fetchSplitBatch` during `DoGet`, the `SplitSource` was already gone.
The MySQL protocol is unaffected because plan + execute share one
request, so the coordinator stays alive until all results are consumed.
This PR keeps the coordinator (and its `SplitSource`) alive across the
two phases and cleans it up reliably:
- **StmtExecutor**: for an Arrow Flight query that produces results on
the BE (`coordBase == coord`), mark it deferred, register the executor
on the `ConnectContext`, and skip the eager `Coordinator.close()` in the
`finally`. A failed query (whose `exec()` threw) is not deferred and is
closed as before.
- **ConnectContext**: hold the deferred executors and add
`closeFlightSqlDeferredExecutors()`, which closes their coordinators
(releasing the `SplitSource` and the query queue slot) and unregisters
the queries.
- **FlightSqlConnectProcessor.close()**: do not finalize deferred
executors.
- **DorisFlightSqlProducer**: finalize the previous query's deferred
coordinator when the next query starts on the connection.
- **FlightSqlConnectPoolMgr.unregisterConnection()**: finalize deferred
coordinators when the connection is torn down. All teardown paths
(idle/query timeout, bearer token expiry, explicit `CloseSession`) reach
here, so an abandoned connection cannot leak the coordinator.
Non-Arrow-Flight paths (MySQL, internal tables, point queries) are
unchanged: `deferredForArrowFlight` can only become true for
`ARROW_FLIGHT_SQL`.
The BE-side error-path hardening (so any `fetchSplitBatch` failure fails
gracefully instead of crashing the BE) is handled separately in apache#64797.
@yiguolei
[bugfix](arrowflight) fix some off heap memory leak on FE (apache#66437)
6f2d667
### What problem does this PR solve?
Issue Number: apache#65615
@hello-stephen

hello-stephen commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

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:

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

@yiguolei

yiguolei commented Aug 5, 2026

Copy link
Copy Markdown
ContributorAuthor

run buildall

@hello-stephen

hello-stephen commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 75.00% (36/48) 🎉
Increment coverage report
Complete coverage report

@hulincuphulincup mentioned this pull request Aug 5, 2026
Open
3 tasks
@morningman
morningman merged commit 058dc08 into apache:branch-4.0Aug 5, 2026
27 of 31 checks passed
yiguolei added a commit to yiguolei/incubator-doris that referenced this pull request Aug 18, 2026
@yiguolei
[branch-4.0] pick some arrow memory leak fix to branch-4.0 (apache#66462
dfc98fe
)
### What problem does this PR solve?
pick apache#64799apache#66437
yiguolei added a commit to yiguolei/incubator-doris that referenced this pull request Aug 19, 2026
@yiguolei
[branch-4.0] pick some arrow memory leak fix to branch-4.0 (apache#66462
d9abb30
)
### What problem does this PR solve?
pick apache#64799apache#66437
yiguolei added a commit that referenced this pull request Aug 20, 2026
@yiguolei
[branch-4.0] pick some arrow memory leak fix to branch-4.0 (#66462) (#…
6fbbcc8
…66871)
### What problem does this PR solve?
pick #64799#66437
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@morningmanmorningmanmorningman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yiguolei@hello-stephen@morningman