Conversation
…r initialized at the time of sending a message.
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
Reviewed PR #783. Changes look reasonable overall.
Notes
\n- [Info] Synchronized blocks added — ensure lock ordering is consistent to avoid deadlocks
Automated review by github-manager-bot
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
This PR moves producer initialization from lazy (on first send) to eager (at Spring context startup via InitializingBean). This is a meaningful behavioral change with trade-offs worth discussing.
Findings
-
[Warning]
RocketMQClientTemplate.java— TheinitProducer()method is now called duringafterPropertiesSet(), meaning the producer is built at Spring startup. If the broker is unreachable at startup, the application will fail to start. Previously, lazy init would defer this failure to first send. Consider whether this is acceptable for all deployment scenarios (e.g., blue-green deployments where broker may not be ready when app container starts). -
[Info]
RocketMQClientTemplate.java— ThegetProducer()now throwsIllegalStateExceptionif producer is null, replacing the previous silent lazy-init fallback. This is a good improvement for fail-fast behavior, but the error message could be more descriptive — consider adding context like"Producer not initialized; ensure initProducer() completed successfully". -
[Info]
ExtTemplateResetConfiguration.java— CallingrocketMQTemplate.initProducer()after setting the producer builder is correct, but consider adding a guard to prevent double initialization ifinitProducer()is called multiple times (e.g., via reset + Spring lifecycle overlap). -
[Info] Sample version bumps (2.3.2/2.3.6 → 2.3.7-SNAPSHOT) are mixed in with the core change. Consider separating version bumps into a dedicated commit for cleaner history.
Suggestions
// In initProducer(), add a guard:
public void initProducer() {
synchronized (RocketMQClientTemplate.class) {
if (this.producer != null) {
log.debug("Producer already initialized, skipping re-initialization");
return;
}
// ... existing init logic
}
}Automated review by github-manager-bot
…r initialized at the time of sending a message.
What is the purpose of the change
XXXXX
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily. Notice,
it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyleto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.