[RIP-64] Heartbeat Optimization #6724
Conversation
| @@ -64,6 +65,7 @@ public class ClientConfig { | |||
| private boolean decodeReadBody = Boolean.parseBoolean(System.getProperty(DECODE_READ_BODY, "true")); | |||
| private boolean decodeDecompressBody = Boolean.parseBoolean(System.getProperty(DECODE_DECOMPRESS_BODY, "true")); | |||
| private boolean vipChannelEnabled = Boolean.parseBoolean(System.getProperty(SEND_MESSAGE_WITH_VIP_CHANNEL_PROPERTY, "false")); | |||
| private boolean useHeartbeatV2 = Boolean.parseBoolean(System.getProperty(HEART_BEAT_V2, "true")); | |||
There was a problem hiding this comment.
HEART_BEAT_V2 default values false
|
Could you add unit and compatibility test |
Thanks very much for your review . No problem，I will add this part . |
|
The test report on heartbeat optimization is as follows link |
Codecov Report
@@ Coverage Diff @@
## develop #6724 +/- ##
=============================================
- Coverage 42.93% 42.65% -0.28%
- Complexity 8995 9124 +129
=============================================
Files 1104 1126 +22
Lines 78380 80062 +1682
Branches 10207 10423 +216
=============================================
+ Hits 33654 34153 +499
- Misses 40512 41629 +1117
- Partials 4214 4280 +66
... and 136 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @Override | ||
| public String toString() { | ||
| return "HeartbeatData [clientID=" + clientID + ", producerDataSet=" + producerDataSet | ||
| + ", consumerDataSet=" + consumerDataSet + "]"; | ||
| } | ||
|
|
||
| public int computeHeartbeatFingerprint() { | ||
| HeartbeatData heartbeatDataCopy = JSON.parseObject(JSON.toJSONString(this), HeartbeatData.class); |
There was a problem hiding this comment.
the performance issues in the code, especially when there is a large MQ cluster and many subscribed topics
There was a problem hiding this comment.
Thank for your review at first，when a cid subscribe 500 topics, the computeHeartbeatFingerprint cost 70ms(In a 30s heartbeat cycle, it's not very much, but our broker can free up a lot of computation and reduce network bandwidth pressure).
| boolean isBrokerSupportV2 = brokerSupportV2HeartbeatSet.contains(addr); | ||
| HeartbeatV2Result heartbeatV2Result = null; | ||
| if (isBrokerSupportV2 && null != brokerAddrHeartbeatFingerprintTable.get(addr) && brokerAddrHeartbeatFingerprintTable.get(addr) == currentHeartbeatFingerprint) { | ||
| heartbeatV2Result = this.mQClientAPIImpl.sendHeartbeatV2(addr, heartbeatDataWithoutSub, 3000); |
There was a problem hiding this comment.
How to replace 3000 with clientConfig.getMqClientApiTimeout()?
There was a problem hiding this comment.
Thank you very much for your review, take it ~
| } | ||
| log.info("sendHeartbeatToAllBrokerV2 simple brokerName: {} subChange: {} brokerAddrHeartbeatFingerprintTable: {}", brokerName, heartbeatV2Result.isSubChange(), JSON.toJSONString(brokerAddrHeartbeatFingerprintTable)); | ||
| } else { | ||
| heartbeatV2Result = this.mQClientAPIImpl.sendHeartbeatV2(addr, heartbeatDataWithSub, 3000); |
There was a problem hiding this comment.
Thank you very much for your review, take it ~
| continue; | ||
| } | ||
| } | ||
| if (null != consumerGroupHeartbeatTable.get(consumerData.getGroupName()) && consumerGroupHeartbeatTable.get(consumerData.getGroupName()) != heartbeatData.getHeartbeatFingerprint()) { |
There was a problem hiding this comment.
Suggest line breaks for better codestyle
There was a problem hiding this comment.
Thank you for your review, one line is also a kind of beauty～
There was a problem hiding this comment.
Could use StringUtils.equals()
| this.brokerController.getTopicConfigManager().createTopicInSendMessageBackMethod(newTopic, subscriptionGroupConfig.getRetryQueueNums(), PermName.PERM_WRITE | PermName.PERM_READ, hasOrderTopicSub, topicSysFlag); | ||
| boolean changed = false; | ||
| if (heartbeatData.isWithoutSub()) { | ||
| changed = this.brokerController.getConsumerManager().registerConsumerWithoutSub(consumerData.getGroupName(), clientChannelInfo, consumerData.getConsumeType(), consumerData.getMessageModel(), consumerData.getConsumeFromWhere(), isNotifyConsumerIdsChangedEnable); |
|
LGTM |
Which Issue(s) This PR Fixes
Fixes #6720
Brief Description
How Did You Test This Change?