ai-yang opened a new pull request, #10937:
URL: https://github.com/apache/rocketmq/pull/10937
### Which Issue(s) This PR Fixes
- Fixes #10935
### Brief Description
`MQClientManager` can return the same `ProduceAccumulator` to multiple
producers that share a RocketMQ client ID. Previously, every
`DefaultMQProducer.shutdown()` stopped that shared accumulator, even while
another producer was running. Under low traffic, small synchronous auto-batched
sends could then wait indefinitely, while asynchronous messages remained queued
without timeout flushing.
Keeping the guard threads alive is necessary but not sufficient: a pending
batch also retains the producer that created it. A second producer must not
join that batch after the first producer shuts down.
This change:
- reference-counts started producer owners; starts the guards for the first
owner and stops them after the last owner;
- tracks each producer's ownership with an `AtomicBoolean`, making repeated
shutdown release idempotent;
- includes the concrete producer's identity in both synchronous and
asynchronous aggregation keys, including explicit-queue sends; a replacement
instance with the same group is still a different owner;
- releases held bytes exactly once after send completion or a synchronous
send-initiation failure, including when a user failure callback throws;
- removes the exact completed batch from the maps so producer-specific keys
do not retain completed batches or remove a newer replacement.
There is no public API or wire-format change.
### Applicability and tradeoffs
Automatic batching is disabled by default (`autoBatch=false`). The
shared-lifecycle failure requires producers sharing an accumulator and one
shutting down while another remains active; it is not a claim that ordinary
non-batched sends are affected.
Producer identity intentionally prevents cross-producer coalescing, because
producers may have different executors, hooks, credentials, and lifetimes.
Batching within one producer is preserved. The aggregation/throughput tradeoff
has not been benchmarked. This does not promise graceful draining of the
producer that is shutting down, nor redesign user-callback
exception/notification semantics.
### How Did You Test This Change?
Latest local verification (2026-09-13), head
`f3901017f89509118835cd90b40109b3b51aa9c3`, JDK 8 / Maven 3.8.1:
```bash
mvn -pl client -am -DskipITs \
-Dtest=ProduceAccumulatorTest,DefaultMQProducerTest,ProduceAccumulatorLifecycleTest
\
-Dsurefire.failIfNoSpecifiedTests=false clean test
```
The local run used offline mode and settings pointing to the existing
dependency cache; no project build configuration was changed and no
Checkstyle/SpotBugs skip flags were used.
- `ProduceAccumulatorTest`: 5 passed, including real guard lifetime and
repeated shutdown.
- `DefaultMQProducerTest`: 41 passed.
- `ProduceAccumulatorLifecycleTest`: 7 passed: stopped/running producer
isolation, explicit queue, same-group replacement, completed-map cleanup,
synchronous initiation failure, inline callback followed by throw, and throwing
failure callback.
- Total: **53 tests, 0 failures/errors/skips**; all four reactor modules
succeeded.
- Checkstyle and SpotBugs enabled; `git diff --check` passed.
The new ownership tests disable background guards and explicitly flush real
batches, making batch ownership deterministic; the existing accumulator suite
separately exercises guard lifecycle. Re-running these seven new cases against
the previous PR's production classes (`ce2e9b171`) produced **6 failures / 7
tests**; the updated production classes passed all seven. This comparison is
against the previous PR head, not a fresh full-suite run of current upstream
`develop`.
The original guard-lifetime regression also failed on unmodified `develop`
at `e348efa66` before the reference-count fix. The PR retains that base and
adds a fast-forward follow-up commit; it was not rebased. CI for the new head
must be checked independently of these local results.
--
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]
