Skip to content

[ISSUE #11091] Fix remoting sub-server timeout scanning and scheduler shutdown - #11092

Open
qianye1001 wants to merge 1 commit into
apache:developfrom
qianye1001:codex/fix-remoting-subserver-lifecycle
Open

[ISSUE #11091] Fix remoting sub-server timeout scanning and scheduler shutdown#11092
qianye1001 wants to merge 1 commit into
apache:developfrom
qianye1001:codex/fix-remoting-subserver-lifecycle

Conversation

@qianye1001

Copy link
Copy Markdown
Contributor

Which Issue(s) This PR Fixes

Fixes #11091

Brief Description

Server-initiated async requests through a SubRemotingServer can remain pending indefinitely after a successful write if no response arrives: the shared housekeeping timer scans only the parent response table. Scan every registered remoting server with the existing timer so expired requests trigger timeout callbacks and release their response-table entries and async permits.

Also shut down the remoting-code distribution scheduler when the parent server shuts down, preventing its periodic task and NettyServerScheduler thread from surviving server shutdown. No additional threads are introduced.

How Did You Test This Change?

  • Confirmed both regressions on unmodified develop: the sub-server timeout callback never completed, and the scheduler remained running after shutdown.
  • Added lifecycle tests using the real housekeeping timer and simulated successful channel writes without responses. They cover the parent and two sub servers, timeout callbacks, response-table cleanup, async permit release, preservation of unexpired requests, and scheduler termination.
  • JDK 11: mvn -pl remoting -am -Dtest=NettyRemotingServerLifecycleTest,NettyRemotingServerTest,SubRemotingServerTest,RemotingServerTest,NettyRemotingAbstractTest -Dsurefire.failIfNoSpecifiedTests=false test, including Checkstyle and SpotBugs.

@lollipopjin lollipopjin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Correct fix for two resource-leak bugs in the remoting layer. The housekeeping timer now scans all registered servers (parent + sub-servers), preventing async requests from hanging indefinitely. The scheduler is properly shut down, preventing thread leaks after server shutdown.

Findings

  • [Info] NettyRemotingServer.java:268 — The loop for (NettyRemotingAbstract server : remotingServerTable.values()) correctly ensures all sub-servers' response tables are scanned. The variable name server shadows the outer class instance, but the scope is small and the intent is clear.
  • [Info] NettyRemotingServer.java:339 — Adding this.scheduledExecutorService.shutdown() in the shutdown path is correct and prevents the scheduler thread from surviving server shutdown. The placement after timer.stop() maintains the existing shutdown order.
  • [Info] The lifecycle test is well-structured — it covers timeout scanning across parent + 2 sub-servers, verifies response table cleanup, async permit release, and scheduler termination. The tearDown method defensively cleans up even if the shutdown test fails on unfixed code.

Verdict

LGTM. Both fixes are minimal, targeted, and well-tested. The changes address real resource leaks that could cause request hangs and thread accumulation in long-running brokers.


Automated review by github-manager-bot

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.89%. Comparing base (4fd0e3b) to head (0cd3d0d).

Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #11092      +/-   ##
=============================================
- Coverage      48.92%   48.89%   -0.03%     
+ Complexity     13846    13842       -4     
=============================================
  Files           1382     1382              
  Lines         101602   101605       +3     
  Branches       13213    13214       +1     
=============================================
- Hits           49709    49682      -27     
- Misses         45866    45885      +19     
- Partials        6027     6038      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Sub-server RPC timeouts are not scanned and remoting scheduler survives shutdown

4 participants