[ISSUE #10739] Complete proxy futures when processor executors reject tasks

[ai-yang]

Which Issue(s) This PR Fixes

• Fixes [Bug] Proxy processor futures can remain pending when completion tasks are rejected #10739

Brief Description

FutureUtils.appendNextFuture now retains the stage returned by whenCompleteAsync and propagates completion-task scheduling failures to the future returned to the caller.

The Proxy producer and consumer processor executors now use an explicit AbortPolicy. This is required because the shared default DiscardOldestPolicy can silently discard completion tasks during shutdown or saturation, leaving no rejection for FutureUtils to propagate. Other RocketMQ thread pools keep their existing rejection behavior.

Together, these changes ensure that Proxy request futures reach an exceptional terminal state instead of remaining pending when processor completion work is rejected.

How Did You Test This Change?

• On the unmodified JDK 8 baseline, the helper-level and real producer-pool shutdown regressions each failed at the expected pending-future assertion in 5/5 runs.
• Added a deterministic real-pool saturation regression using one worker and one queue slot. Latches occupy the worker and fill the queue without sleeps or randomized scheduling.
• Temporarily restoring the old DiscardOldestPolicy made the saturation regression fail 1/1 at the returned future must not remain pending; restoring AbortPolicy made it pass.
• On the refreshed head, FutureUtilsTest passed 1/1 and DefaultMessagingProcessorTest passed 6/6. All 11 modules in the targeted proxy -am reactor succeeded with Checkstyle and SpotBugs enabled.
• The earlier full JDK 8 proxy -am test reactor also passed: Broker 752 tests (0 failures, 0 errors, 4 skipped) and Proxy 304 tests (0 failures, 0 errors, 3 skipped).
• git diff --check passed.

[RockteMQ-AI]

Findings

• [Info] FutureUtils.java:27-34 — The fix is correct. completionFuture.whenComplete(...) runs synchronously in the completing thread (not submitted to the executor), so it reliably catches RejectedExecutionException without needing another executor. Good design.

• [Info] DefaultMessagingProcessor.java:89,98 — Switching from the default DiscardOldestPolicy to AbortPolicy is the right call here. Under high load or shutdown, tasks that were previously silently discarded will now surface as RejectedExecutionException, which FutureUtils can properly propagate. This is a behavioral change, but it converts silent data loss into explicit failure — the correct tradeoff for a messaging system.

• [Info] FutureUtilsTest.java — Test coverage is solid. The shut-down executor pattern reliably reproduces the rejection scenario, and asserting CompletionException wrapping RejectedExecutionException validates the full propagation chain.

• [Info] DefaultMessagingProcessorTest.java — Good integration-level test that verifies the real executor chain propagates rejections through the future.

Suggestions

• Minor: Consider whether other thread pools in the Proxy module that also use appendNextFuture (or similar future-chaining patterns) might need the same AbortPolicy treatment. A quick audit of ThreadPoolMonitor.createAndMonitor call sites in the proxy module could reveal similar latent issues.

Verdict

Well-structured bug fix that addresses both the symptom (pending futures) and root cause (silent task discarding). The two-level approach ensures robustness even if one layer is misconfigured.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Summary

Defensive fix with proper validation and test coverage. LGTM.

---

Automated review by github-manager-bot

[ai-yang]

Refreshed onto current develop (e348efa66) and force-pushed one signed-off commit (2d561e1e0). The missing saturation evidence is now covered by a deterministic one-worker/one-slot test: old DiscardOldestPolicy fails with a pending future, while AbortPolicy completes it exceptionally. Affected tests pass 7/7 and the targeted 11-module reactor, Checkstyle, SpotBugs, and git diff --check all pass.

@lollipopjin @lizhimins, could you please take a human review when convenient, especially of the executor rejection-policy change and its overload semantics?

[codecov-commenter]

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 48.52%. Comparing base (e348efa) to head (2d561e1).

Files with missing lines	Patch %	Lines
.../org/apache/rocketmq/common/utils/FutureUtils.java	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10740      +/-   ##
=============================================
- Coverage      48.58%   48.52%   -0.07%     
+ Complexity     13678    13655      -23     
=============================================
  Files           1381     1381              
  Lines         101475   101479       +4     
  Branches       13190    13190              
=============================================
- Hits           49304    49244      -60     
- Misses         46170    46217      +47     
- Partials        6001     6018      +17     

☔ 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.

[RockteMQ-AI]

Summary

This PR modifies 4 files (185 lines).

Key changes reviewed. Please see inline comments for specific suggestions.

---

Automated review by "github-manager-bot"