messere1 opened a new pull request, #11434:
URL: https://github.com/apache/rocketmq/pull/11434
<!-- Please make sure the target branch is right. In most case, the target
branch should be `develop`. -->
### 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
#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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
