You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/06/27 07:41:18 UTC

[GitHub] [rocketmq] shengminw opened a new pull request, #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

shengminw opened a new pull request, #4521:
URL: https://github.com/apache/rocketmq/pull/4521

   [ISSUE#4520](https://github.com/apache/rocketmq/issues/4520)
   
   ## What is the purpose of the change
   In order to make maxMessageSize modification take effect after broker is started, I implenment the function of adjusting maxMessageSize dynamicly.
   ## Brief changelog
   1. add updateEncoderBufferCapacity(int newMaxMessageBodySize), in MessageExtEncoder.
   2. add updateMaxMessageSize(PutMessageThreadLocal putMessageThreadLocal), before encode buffer.
   3. add junit test  "testDynamicMaxMessageSize()"


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] dongeforever merged pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
dongeforever merged PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] shengminw commented on pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
shengminw commented on PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521#issuecomment-1168311439

   @dongeforever thanks, I will add it.


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] codecov-commenter commented on pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521#issuecomment-1168255430

   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4521?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#4521](https://codecov.io/gh/apache/rocketmq/pull/4521?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (716bcee) into [develop](https://codecov.io/gh/apache/rocketmq/commit/da01deb961807c68b102aeccb1f06d9721ca700e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (da01deb) will **decrease** coverage by `0.05%`.
   > The diff coverage is `76.92%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #4521      +/-   ##
   =============================================
   - Coverage      48.05%   47.99%   -0.06%     
   - Complexity      5108     5110       +2     
   =============================================
     Files            649      649              
     Lines          43026    43055      +29     
     Branches        5626     5632       +6     
   =============================================
   - Hits           20675    20666       -9     
   - Misses         19838    19876      +38     
     Partials        2513     2513              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4521?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../java/org/apache/rocketmq/store/MultiDispatch.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL011bHRpRGlzcGF0Y2guamF2YQ==) | `64.89% <71.42%> (+0.12%)` | :arrow_up: |
   | [...pache/rocketmq/store/dledger/DLedgerCommitLog.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2RsZWRnZXIvRExlZGdlckNvbW1pdExvZy5qYXZh) | `75.29% <71.42%> (-0.06%)` | :arrow_down: |
   | [...main/java/org/apache/rocketmq/store/CommitLog.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0NvbW1pdExvZy5qYXZh) | `76.74% <83.33%> (+0.29%)` | :arrow_up: |
   | [...rg/apache/rocketmq/common/stats/StatsSnapshot.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNTbmFwc2hvdC5qYXZh) | `84.61% <0.00%> (-15.39%)` | :arrow_down: |
   | [.../apache/rocketmq/common/stats/MomentStatsItem.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvTW9tZW50U3RhdHNJdGVtLmphdmE=) | `38.09% <0.00%> (-9.53%)` | :arrow_down: |
   | [...ache/rocketmq/common/stats/MomentStatsItemSet.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvTW9tZW50U3RhdHNJdGVtU2V0LmphdmE=) | `39.13% <0.00%> (-8.70%)` | :arrow_down: |
   | [...va/org/apache/rocketmq/logging/inner/Appender.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9BcHBlbmRlci5qYXZh) | `29.21% <0.00%> (-7.87%)` | :arrow_down: |
   | [...tmq/namesrv/processor/DefaultRequestProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bmFtZXNydi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbmFtZXNydi9wcm9jZXNzb3IvRGVmYXVsdFJlcXVlc3RQcm9jZXNzb3IuamF2YQ==) | `66.04% <0.00%> (-7.17%)` | :arrow_down: |
   | [...va/org/apache/rocketmq/common/stats/StatsItem.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNJdGVtLmphdmE=) | `50.00% <0.00%> (-5.84%)` | :arrow_down: |
   | [...ache/rocketmq/broker/topic/TopicConfigManager.java](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdG9waWMvVG9waWNDb25maWdNYW5hZ2VyLmphdmE=) | `54.61% <0.00%> (-4.62%)` | :arrow_down: |
   | ... and [29 more](https://codecov.io/gh/apache/rocketmq/pull/4521/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/4521?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4521?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [da01deb...716bcee](https://codecov.io/gh/apache/rocketmq/pull/4521?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] shengminw commented on pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
shengminw commented on PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521#issuecomment-1168667474

   @dongeforever 
   The message encoding buffer in openmessaging/DLedger is of fixed size. https://github.com/openmessaging/dledger/blob/master/src/main/java/io/openmessaging/storage/dledger/store/file/DLedgerMmapFileStore.java
   ```java
   this.localEntryBuffer = ThreadLocal.withInitial(() -> {
       return ByteBuffer.allocate(4194304);
   });
   ```
   So temporarily remove the adaptation of DLeger mode.


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] shengminw commented on a diff in pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
shengminw commented on code in PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521#discussion_r910563214


##########
store/src/main/java/org/apache/rocketmq/store/CommitLog.java:
##########
@@ -604,6 +604,14 @@ private String generateKey(StringBuilder keyBuilder, MessageExt messageExt) {
         return keyBuilder.toString();
     }
 
+    public void updateMaxMessageSize(PutMessageThreadLocal putMessageThreadLocal) {
+        // dynamically adjust maxMessageSize, but not support DLedger mode temporarily
+        int newMaxMessageSize = this.defaultMessageStore.getMessageStoreConfig().getMaxMessageSize();
+        if (putMessageThreadLocal.getEncoder().getMaxMessageBodySize() != newMaxMessageSize) {
+            putMessageThreadLocal.getEncoder().updateEncoderBufferCapacity(newMaxMessageSize);
+        }

Review Comment:
   ok, it's neccessary to add this check.



-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls commented on pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521#issuecomment-1170502662

   
   [![Coverage Status](https://coveralls.io/builds/50478955/badge)](https://coveralls.io/builds/50478955)
   
   Coverage increased (+0.2%) to 52.157% when pulling **2663fd13290672872a4ad839b2042a1054b034bb on shengminw:local** into **cdfa5ced7e8152e0b359fdeeb2694be641eb5e29 on apache:develop**.
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] dongeforever commented on a diff in pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
dongeforever commented on code in PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521#discussion_r910559096


##########
store/src/main/java/org/apache/rocketmq/store/CommitLog.java:
##########
@@ -604,6 +604,14 @@ private String generateKey(StringBuilder keyBuilder, MessageExt messageExt) {
         return keyBuilder.toString();
     }
 
+    public void updateMaxMessageSize(PutMessageThreadLocal putMessageThreadLocal) {
+        // dynamically adjust maxMessageSize, but not support DLedger mode temporarily
+        int newMaxMessageSize = this.defaultMessageStore.getMessageStoreConfig().getMaxMessageSize();
+        if (putMessageThreadLocal.getEncoder().getMaxMessageBodySize() != newMaxMessageSize) {
+            putMessageThreadLocal.getEncoder().updateEncoderBufferCapacity(newMaxMessageSize);
+        }

Review Comment:
   It is better to do some checks to prevent negeative values.
   
   if (newMaxMessageSize < 10) {
     return
   }



-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] dongeforever commented on pull request #4521: [ISSUE#4520] [Optimization] Implenment adjusting maxMessageSize dynamicly

Posted by GitBox <gi...@apache.org>.
dongeforever commented on PR #4521:
URL: https://github.com/apache/rocketmq/pull/4521#issuecomment-1168298914

   @shengminw LGTM.
   
   And it may also need a test for DLedger Mode.


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org