Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
biningo commented on code in PR #8071:
URL: https://github.com/apache/rocketmq/pull/8071#discussion_r1598683474
##
client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageConcurrentlyService.java:
##
@@ -190,35 +189,11 @@ public void submitConsumeRequest(
final MessageQueue messageQueue,
final boolean dispatchToConsume) {
final int consumeBatchSize =
this.defaultMQPushConsumer.getConsumeMessageBatchMaxSize();
-if (msgs.size() <= consumeBatchSize) {
-ConsumeRequest consumeRequest = new ConsumeRequest(msgs,
processQueue, messageQueue);
-try {
-this.consumeExecutor.submit(consumeRequest);
-} catch (RejectedExecutionException e) {
-this.submitConsumeRequestLater(consumeRequest);
-}
-} else {
-for (int total = 0; total < msgs.size(); ) {
-List msgThis = new ArrayList<>(consumeBatchSize);
-for (int i = 0; i < consumeBatchSize; i++, total++) {
-if (total < msgs.size()) {
-msgThis.add(msgs.get(total));
-} else {
-break;
-}
-}
-
-ConsumeRequest consumeRequest = new ConsumeRequest(msgThis,
processQueue, messageQueue);
-try {
-this.consumeExecutor.submit(consumeRequest);
-} catch (RejectedExecutionException e) {
-for (; total < msgs.size(); total++) {
-msgThis.add(msgs.get(total));
-}
-
-this.submitConsumeRequestLater(consumeRequest);
-}
-}
+int batch = ((msgs.size() - 1) / consumeBatchSize) + 1;
Review Comment:
Oh yeah
It's not very readable here. I've optimized it. Thanks for your suggestion.
--
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: commits-unsubscr...@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
biningo commented on code in PR #8071:
URL: https://github.com/apache/rocketmq/pull/8071#discussion_r1598683474
##
client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageConcurrentlyService.java:
##
@@ -190,35 +189,11 @@ public void submitConsumeRequest(
final MessageQueue messageQueue,
final boolean dispatchToConsume) {
final int consumeBatchSize =
this.defaultMQPushConsumer.getConsumeMessageBatchMaxSize();
-if (msgs.size() <= consumeBatchSize) {
-ConsumeRequest consumeRequest = new ConsumeRequest(msgs,
processQueue, messageQueue);
-try {
-this.consumeExecutor.submit(consumeRequest);
-} catch (RejectedExecutionException e) {
-this.submitConsumeRequestLater(consumeRequest);
-}
-} else {
-for (int total = 0; total < msgs.size(); ) {
-List msgThis = new ArrayList<>(consumeBatchSize);
-for (int i = 0; i < consumeBatchSize; i++, total++) {
-if (total < msgs.size()) {
-msgThis.add(msgs.get(total));
-} else {
-break;
-}
-}
-
-ConsumeRequest consumeRequest = new ConsumeRequest(msgThis,
processQueue, messageQueue);
-try {
-this.consumeExecutor.submit(consumeRequest);
-} catch (RejectedExecutionException e) {
-for (; total < msgs.size(); total++) {
-msgThis.add(msgs.get(total));
-}
-
-this.submitConsumeRequestLater(consumeRequest);
-}
-}
+int batch = ((msgs.size() - 1) / consumeBatchSize) + 1;
Review Comment:
Oh yeah!
It's not very readable here. I've optimized it. Thanks for your suggestion.
--
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: commits-unsubscr...@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
drpmma commented on code in PR #8071:
URL: https://github.com/apache/rocketmq/pull/8071#discussion_r1598084837
##
client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageConcurrentlyService.java:
##
@@ -190,35 +189,11 @@ public void submitConsumeRequest(
final MessageQueue messageQueue,
final boolean dispatchToConsume) {
final int consumeBatchSize =
this.defaultMQPushConsumer.getConsumeMessageBatchMaxSize();
-if (msgs.size() <= consumeBatchSize) {
-ConsumeRequest consumeRequest = new ConsumeRequest(msgs,
processQueue, messageQueue);
-try {
-this.consumeExecutor.submit(consumeRequest);
-} catch (RejectedExecutionException e) {
-this.submitConsumeRequestLater(consumeRequest);
-}
-} else {
-for (int total = 0; total < msgs.size(); ) {
-List msgThis = new ArrayList<>(consumeBatchSize);
-for (int i = 0; i < consumeBatchSize; i++, total++) {
-if (total < msgs.size()) {
-msgThis.add(msgs.get(total));
-} else {
-break;
-}
-}
-
-ConsumeRequest consumeRequest = new ConsumeRequest(msgThis,
processQueue, messageQueue);
-try {
-this.consumeExecutor.submit(consumeRequest);
-} catch (RejectedExecutionException e) {
-for (; total < msgs.size(); total++) {
-msgThis.add(msgs.get(total));
-}
-
-this.submitConsumeRequestLater(consumeRequest);
-}
-}
+int batch = ((msgs.size() - 1) / consumeBatchSize) + 1;
Review Comment:
This line of code could be clearer. Readability is more important than
reducing the number of lines.
--
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: commits-unsubscr...@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
codecov-commenter commented on PR #8071: URL: https://github.com/apache/rocketmq/pull/8071#issuecomment-2106545711 ## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/8071?dropdown=coverage=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 42.90%. Comparing base [(`159a603`)](https://app.codecov.io/gh/apache/rocketmq/commit/159a603219e363e5f991a0c85213f6dc13f0a9be?dropdown=coverage=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) to head [(`b8a638d`)](https://app.codecov.io/gh/apache/rocketmq/pull/8071?dropdown=coverage=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop#8071 +/- ## = - Coverage 42.94% 42.90% -0.05% + Complexity 1038710372 -15 = Files 1270 1270 Lines 8869488679 -15 Branches 1140111397 -4 = - Hits 3809238045 -47 - Misses 4591445940 +26 - Partials4688 4694 +6 ``` [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/rocketmq/pull/8071?dropdown=coverage=pr=continue_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). -- 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: commits-unsubscr...@rocketmq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
biningo commented on PR #8071: URL: https://github.com/apache/rocketmq/pull/8071#issuecomment-2105924740 > Merge develop branch to fix the ut. @biningo OK, I've merged. Thanks for your help! :) -- 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: commits-unsubscr...@rocketmq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
cserwen commented on PR #8071: URL: https://github.com/apache/rocketmq/pull/8071#issuecomment-2103750693 Merge develop branch to fix the ut. @biningo -- 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: commits-unsubscr...@rocketmq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
biningo commented on PR #8071: URL: https://github.com/apache/rocketmq/pull/8071#issuecomment-2084678644 Maven test fails in Ubuntu, but it seems to have nothing to do with this PR. @humkum Can you help me? Thanks! -- 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: commits-unsubscr...@rocketmq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
biningo commented on PR #8071: URL: https://github.com/apache/rocketmq/pull/8071#issuecomment-2079365193 In submitConsumeRequest submitConsumeRequestLater is unnecessary. -- 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: commits-unsubscr...@rocketmq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [ISSUE #8070] Optimize the submitConsumeRequest and remove unnecessary exception catching [rocketmq]
biningo commented on PR #8071: URL: https://github.com/apache/rocketmq/pull/8071#issuecomment-2079358580 If the number of messages or the consumeBatchSize is 0, it will work fine. -- 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: commits-unsubscr...@rocketmq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
