[messere1]

Which Issue(s) This PR Fixes

Fixes #11433

Brief Description

A suspended lite long-polling request is only reachable through pollingMap — both notifyMessageArriving() and the timeout sweep in run() look its queue up there. Two paths can remove a queue that still holds a live suspended request, leaving the request unanswered by the broker until the client-side remoting timeout fires:

1. Race with cleanUnusedResource() (empty-queue removal introduced by [ISSUE #10071] Fix PopLiteLongPollingService#cleanUnusedResource #10072): polling() runs on processor threads as get → size-check → add. If the service thread's cleanup removes the (routinely empty) queue between the get and the add, the add lands in an orphaned queue while polling() still returns POLLING_SUC.
2. Capacity eviction: pollingMap is bounded by popPollingMapSize distinct keys; inserting beyond capacity silently evicts an LRU entry whose queue may still hold suspended requests.

Fix, keeping #10072's cleanup semantics:

• In polling(), after a successful queue.add(request), re-establish the mapping with pollingMap.putIfAbsent(pollingKey, queue). If another polling thread has meanwhile re-created a replacement queue for the same key, fail the attempt with POLLING_FULL (the client is answered immediately and re-polls) instead of suspending into an orphaned queue.
• Register an EvictionListener that wakes up all requests of a queue evicted at capacity, so they are answered early instead of hanging.

How Did You Test This Change?

Red/green in PopLiteLongPollingServiceTest (deterministic, latch-gated interleaving — no sleeps, no race loops):

1. testPolling_requestSurvivesQueueCleanupRace
   • RED (pre-fix): assertSame(queue, pollingMap.get(clientId)) fails — expected same:<[PopRequest{... suspended ...}]> was not:<null>; the request for which polling() returned POLLING_SUC lives in a queue that neither notifyMessageArriving() nor the timeout sweep can find.
   • GREEN (post-fix): the mapping is restored, notifyMessageArriving() returns true and wakes the request, totalPollingNum is balanced.
2. testPolling_evictedQueueWakesSuspendedRequests
   • RED (pre-fix): Wanted but not invoked: pullMessageExecutor.submit(...) — the evicted client's request is never woken.
   • GREEN (post-fix): exactly one wake-up is submitted for the evicted client's request.

Full runs:

• mvn -pl broker test -Dtest='PopLiteLongPollingServiceTest' → Tests run: 14, Failures: 0, Errors: 0 (12 pre-existing + 2 new).
• mvn -pl broker test -Dtest='PopLiteMessageProcessorTest' (constructs/mocks the service's consumers) → Tests run: 23, Failures: 0, Errors: 0.
• mvn -pl broker checkstyle:check (project ruleset style/rmq_checkstyle.xml) → BUILD SUCCESS.

Note: the build stops after tests at jacoco:0.8.5:report with "Unsupported class file major version 61" — this is environmental (JaCoCo 0.8.5 vs Java 17 bytecode) and reproduces on a clean develop; the test phase itself passes.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes a race condition in PopLiteLongPollingService where suspended lite long-polling requests can become unreachable through both notifyMessageArriving() and the timeout sweep in run(). Two root causes are addressed: (1) cleanUnusedResource() removing an empty queue between get() and add() in polling(), and (2) capacity eviction silently dropping a queue that still holds live requests.

Findings

• [Info] PopLiteLongPollingService.java:248-252 — In the narrow race where putIfAbsent returns a different queue (liveQueue != queue), the request already added to the local queue becomes orphaned. Since the orphaned queue is no longer in pollingMap, neither notifyMessageArriving() nor the timeout sweep will ever find it. The client does get a POLLING_FULL response and re-polls, so there is no hang — but the PopRequest object (and its ChannelHandlerContext / RemotingCommand references) lingers until GC collects the orphaned queue. Consider removing the request from the old queue before returning POLLING_FULL (queue.remove(request)) to release those references eagerly. This is a very minor cleanup, not a correctness blocker.

• [Info] PopLiteLongPollingService.java:63-70 — The eviction listener drains the queue and wakes each request synchronously during the put that triggers eviction. If an evicted queue happens to hold many requests, this runs on the calling thread. In practice the per-key population is bounded, so this is unlikely to be a concern — worth keeping in mind if popPollingMapSize is ever configured very large.

Suggestions

The overall approach is sound and the fix is minimal. The two new tests (testPolling_requestSurvivesQueueCleanupRace and testPolling_evictedQueueWakesSuspendedRequests) are well-designed — deterministic, latch-gated, no sleeps. Good coverage of both race paths.

Cross-repo Note

No protocol or client-visible API changes. No coordination with apache/rocketmq-clients needed.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds a suspendedRequests map to track PopRequests when their queue is removed from pollingMap, ensuring suspended requests can still be woken up when messages arrive.

Findings

• [Info] PopLiteLongPollingService.java:61-72 — The suspendedRequests map is a good solution to the problem. The cleanup in the finally block ensures requests are moved to suspendedRequests before being removed from pollingMap.
• [Warning] PopLiteLongPollingService.java:148-155 — The logic for moving requests between pollingMap and suspendedRequests is complex. Consider adding a comment explaining the state transitions more clearly.
• [Info] PopLiteLongPollingServiceTest.java — Good test coverage including the race condition scenario.

Suggestions

• Consider extracting the request migration logic (lines 148-155) into a separate method with a descriptive name like migrateSuspendedRequests() for better readability.

Verdict

LGTM with minor suggestion. The fix addresses an important edge case in long-polling request handling.

---

Automated review by github-manager-bot