messere1 opened a new pull request, #11353:
URL: https://github.com/apache/rocketmq/pull/11353
## What changed
- `LiteSubscriptionRegistryImpl.excludeClientByLmqName` now reclaims the
evicted client's `Channel` entry from `clientChannels` when the eviction
removes its last liteTopic and drops it from `client2Subscription`
- the channel removal happens after `notifyUnsubscribeLite` (which still
needs the channel to deliver the eviction notice), and uses
`ConcurrentMap.remove(key, value)` so a channel freshly registered by a
concurrent re-subscribe is not clobbered
- clients that still hold other liteTopics keep their channel, exactly as
before
- updated the full-sync re-notify test to re-register the channel first,
mirroring the real `LiteSubscriptionCtlProcessor` flow (`COMPLETE_ADD` always
calls `updateClientChannel` before `addCompleteSubscription`)
## Why
Exclusive-mode takeover removes the loser's `LiteSubscription` from
`client2Subscription` as soon as its last liteTopic is taken over, but left its
`Channel` in `clientChannels`. Every other removal flow goes through
`removeCompleteSubscription`, which cleans both maps; the expiry cleaner
`cleanupExpiredSubscriptions` only walks `client2Subscription`, so once the
eviction had removed the subscription, the orphaned `clientChannels` entry was
unreachable for any cleanup until broker restart.
Under exclusive-mode client churn (every takeover evicts the previous
client, e.g. clients restarting with new clientIds) `clientChannels` grows
unboundedly, retaining netty `Channel` references, and `notifyUnsubscribeLite`
keeps resolving the stale channel.
## Impact
The change is local to the exclusive-eviction branch of the lite
subscription registry. Regular subscribe/unsubscribe flows, wildcard groups,
and non-exclusive groups are untouched; a client that still subscribes to other
liteTopics keeps its channel.
Fixes #11352
## Validation
- `mvn -pl broker -Dtest=LiteSubscriptionRegistryImplTest -Djacoco.skip=true
test`
- 48 tests, 0 failures, 0 errors (46 pre-existing + 2 new)
- `testExclusiveEviction_RemovesEvictedClientChannel` fails on the
pristine branch (`expected null, but was: Mock for Channel`) and passes with
the fix
- `testExclusiveEviction_KeepsChannelWhenClientRetainsOtherLmqs` guards
the boundary: the channel is kept while the client still holds other liteTopics
- `mvn -pl broker -Dtest='org.apache.rocketmq.broker.lite.*Test'
-Djacoco.skip=true test`
- 121 tests, 0 failures (whole lite package regression)
- `mvn -pl broker
-Dtest='AckMessageProcessorTest,LiteManagerProcessorTest,LiteSubscriptionCtlProcessorTest,PopLiteMessageProcessorEventLossTest,PopLiteMessageProcessorTest'
-Djacoco.skip=true test`
- 63 tests, 0 failures (lite pop/ack/ctl processor regression)
- `mvn -pl broker checkstyle:check
-Dcheckstyle.config.location=style/rmq_checkstyle.xml`
- 0 violations
JaCoCo is skipped locally because the repository's JaCoCo 0.8.5 agent does
not support Java 17 class files.
--
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]
