Skip to content

[BugFix] Cancel pipeline fragments when reaping expired external scan contexts (backport #76535) - #76697

Closed
mergify[bot] wants to merge 1 commit into
branch-3.2from
mergify/bp/branch-3.2/pr-76535
Closed

[BugFix] Cancel pipeline fragments when reaping expired external scan contexts (backport #76535)#76697
mergify[bot] wants to merge 1 commit into
branch-3.2from
mergify/bp/branch-3.2/pr-76535

Conversation

@mergify

@mergify mergify Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Why I'm doing:

When a Spark/Flink connector reader dies without calling close_scanner (task killed, stage aborted, executor lost), the abandoned scanner's ScanContext is reaped by the keep-alive GC in ExternalScanContextMgr::gc_expired_context(). The reaper cancels the fragment through the non-pipeline FragmentMgr — but external scan fragments opened by open_scanner always run on the pipeline engine (QueryOrchestrator::exec_external_plan_fragmentFragmentExecutor) and never register in the non-pipeline FragmentMgr, so FragmentMgr::cancel() finds no match and silently returns OK.

The GC therefore only shuts down the result queue. The pipeline FragmentContext keeps running/blocked, and its QueryContext keeps holding all buffered scan memory (GBs per query for wide-table scans) until the query deadline (query_timeout, 3600s by default in the connectors) finally cancels it via the driver poller — or until the BE restarts.

We hit this in production: a burst of abandoned connector scanners (Spark stage aborts during BE memory pressure, then app-level retries) kept 100–240 GB pinned in query_pool for hours. The log signature is conclusive: abandoned fragments do get gc expired scan context (the reaper ran and erased the context), yet they die exactly query_timeout seconds after open via [Driver] Timeout — the GC's cancel never took effect.

Note that clear_scan_context() (the close_scanner path) was already fixed to cancel through the pipeline managers in #20264; the GC path was left behind.

What I'm doing:

  • Extract the cancellation into a private _cancel_scan_context() — cancel through the pipeline QueryContextManager / FragmentContext plus clear the result queue — shared by both close_scanner (clear_scan_context) and the reaper, so the two paths can no longer diverge. The reaper passes a distinguishable cancel reason (cancelled by expired scan context gc).
  • Drop the non-pipeline FragmentMgr dependency from ExternalScanContextMgr: its only use was this no-op cancel (the member was already marked [[maybe_unused]]).
  • Add a test asserting the shared cancellation path actually cancels the registered pipeline fragment (exercised via clear_scan_context; the reaper loop itself is compiled out under BE_TEST).

With this fix an abandoned scanner is reclaimed within keep_alive_min (default 5 min) + one GC interval, instead of query_timeout (1 hour).

Known limitations (intentionally out of scope): a context stuck in an in-flight get_next (last_access_time == -1) is still skipped by the GC and only reclaimed at the query deadline.

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: abandoned external scanners are reclaimed at keep-alive expiry instead of query timeout

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

🤖 Generated with Claude Code


This is an automatic backport of pull request #76535 done by [Mergify](https://mergify.com).

… contexts (#76535)

Signed-off-by: gengjun-git <gengjun@starrocks.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit d58c924)

# Conflicts:
#	be/src/orchestration/orchestration_env.cpp
#	be/src/runtime/external_scan_context_mgr.cpp
#	be/src/runtime/external_scan_context_mgr.h
#	be/test/orchestration/external_scan_context_mgr_test.cpp
#	be/test/orchestration/external_scan_orchestrator_test.cpp
@mergify

mergify Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of d58c924 has failed:

On branch mergify/bp/branch-3.2/pr-76535
Your branch is up to date with 'origin/branch-3.2'.

You are currently cherry-picking commit d58c9249a9.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   be/src/orchestration/orchestration_env.cpp
	both modified:   be/src/runtime/external_scan_context_mgr.cpp
	both modified:   be/src/runtime/external_scan_context_mgr.h
	deleted by us:   be/test/orchestration/external_scan_context_mgr_test.cpp
	deleted by us:   be/test/orchestration/external_scan_orchestrator_test.cpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify Bot added the conflicts label Jul 22, 2026
@wanpengfei-git
wanpengfei-git enabled auto-merge (squash) July 22, 2026 06:09
@mergify mergify Bot closed this Jul 22, 2026
auto-merge was automatically disabled July 22, 2026 06:09

Pull request was closed

@mergify

mergify Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@mergify[bot]: Backport conflict, please reslove the conflict and resubmit the pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant