Skip to content

[BugFix] Fix bRPC stub cache clean timer leak (backport #75973)#76360

Merged
wanpengfei-git merged 3 commits into
branch-4.1from
mergify/bp/branch-4.1/pr-75973
Jul 22, 2026
Merged

[BugFix] Fix bRPC stub cache clean timer leak (backport #75973)#76360
wanpengfei-git merged 3 commits into
branch-4.1from
mergify/bp/branch-4.1/pr-75973

Conversation

@mergify

@mergify mergify Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Why I'm doing:

there is mem leak caused by bRPC stub cache clean timer leak due to timer is unscheduled but not removed from mem.

std::shared_ptr<PInternalService_RecoverableStub> BrpcStubCache::get_stub(const butil::EndPoint& endpoint) {
    ...
    if (_timer->unschedule((*stub_pool)->_cleanup_task.get()) != TIMER_TASK_RUNNING) {
        timespec tm = butil::seconds_from_now(config::brpc_stub_expire_s);
        auto status = _timer->schedule((*stub_pool)->_cleanup_task.get(), tm);
        if (!status.ok()) {
            LOG(WARNING) << "Failed to schedule brpc cleanup task: " << endpoint;
        }
    }
}

every get_stub() will do _timer->unschedule() and _timer->schedule().
unlike what we expect, _timer->unschedule() will not remove internal objects, thus leak happens

What I'm doing:

extend timer deadline if the stub is still in use instead of unschedule()

Fixes #issue

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: upgrade & downgrade compatibility, etc.

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

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.1
    • 4.0
    • 3.5

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

Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>
Co-authored-by: wanpengfei-git <wanpengfei91@163.com>
(cherry picked from commit 2bbca67)

# Conflicts:
#	be/src/util/brpc_stub_cache.cpp
#	be/src/util/brpc_stub_cache.h
#	be/test/util/brpc_stub_cache_test.cpp
@mergify mergify Bot added the conflicts label Jul 14, 2026
@mergify

mergify Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of 2bbca67 has failed:

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

You are currently cherry-picking commit 2bbca67281.
  (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 <file>..." to mark resolution)
	both modified:   be/src/util/brpc_stub_cache.cpp
	both modified:   be/src/util/brpc_stub_cache.h
	both modified:   be/test/util/brpc_stub_cache_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

@wanpengfei-git
wanpengfei-git enabled auto-merge (squash) July 14, 2026 12:06
@mergify mergify Bot closed this Jul 14, 2026
auto-merge was automatically disabled July 14, 2026 12:06

Pull request was closed

@mergify

mergify Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

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

@satanson satanson reopened this Jul 21, 2026
@wanpengfei-git
wanpengfei-git enabled auto-merge (squash) July 21, 2026 12:07
@satanson

Copy link
Copy Markdown
Contributor

Cherry-pick of 2bbca67 has failed:

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

You are currently cherry-picking commit 2bbca67281.
  (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 <file>..." to mark resolution)
	both modified:   be/src/util/brpc_stub_cache.cpp
	both modified:   be/src/util/brpc_stub_cache.h
	both modified:   be/test/util/brpc_stub_cache_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

@copilot fix backpoort conflicts

auto-merge was automatically disabled July 21, 2026 12:27

Head branch was pushed to by a user without write access

Copilot AI commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Cherry-pick of 2bbca67 has failed:

...

Resolved the backport conflicts in commit [BugFix] Resolve backport conflicts for bRPC stub cache clean timer leak fix.

The conflicts arose because main had moved the files to be/src/common/brpc/ and changed to BthreadTimer/BthreadTimerTask, while branch-4.1 keeps them in be/src/util/ using pipeline::PipelineTimer/LightTimerTask.

The fix was adapted to branch-4.1's architecture:

  • Kept pipeline::PipelineTimer and LightTimerTask throughout (instead of BthreadTimer/BthreadTimerTask)
  • Applied the core bug fix: deadline-based cleanup instead of unschedule() + schedule() on every get_stub() call
  • Changed HttpBrpcStubCache and LakeServiceBrpcStubCache to use a StubEntry struct
  • Added _stopping flag, is_cleanup_task_owner_locked(), and replace_cleanup_task_locked() to each cache class
  • Dropped tests that require BthreadTimer-specific APIs (unschedule_and_join, initialize()) not available in branch-4.1
  • Added test_active_access_keeps_stub_alive_across_expire_window regression test adapted to use ExecEnv*

Copilot AI requested a review from satanson July 21, 2026 12:28
@wanpengfei-git
wanpengfei-git enabled auto-merge (squash) July 22, 2026 02:39
@wanpengfei-git
wanpengfei-git merged commit e695336 into branch-4.1 Jul 22, 2026
27 of 28 checks passed
@wanpengfei-git
wanpengfei-git deleted the mergify/bp/branch-4.1/pr-75973 branch July 22, 2026 03:11
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.

4 participants