Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.1.0-incubating
    • Component/s: rocketmq-client
    • Labels:
      None

      Description

      For some exception scenario, the log is not abundant enough to diagnose the problem.

      Some works should be done to improve the log messages

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user Jaskey opened a pull request:

          https://github.com/apache/incubator-rocketmq/pull/90

          ROCKETMQ-172log improvement for rocketmq client

          JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-172?jql=project%20%3D%20ROCKETMQ

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/Jaskey/incubator-rocketmq ROCKETMQ-172-log-improvement

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/incubator-rocketmq/pull/90.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #90


          commit 633ac28b8abc63e388d6ebe6081c41371b69ed29
          Author: Jaskey <linjunjie1103@gmail.com>
          Date: 2017-04-11T11:54:26Z

          improve error message when pull exception


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/90 ROCKETMQ-172 log improvement for rocketmq client JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-172?jql=project%20%3D%20ROCKETMQ You can merge this pull request into a Git repository by running: $ git pull https://github.com/Jaskey/incubator-rocketmq ROCKETMQ-172 -log-improvement Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/90.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #90 commit 633ac28b8abc63e388d6ebe6081c41371b69ed29 Author: Jaskey <linjunjie1103@gmail.com> Date: 2017-04-11T11:54:26Z improve error message when pull exception
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11033662/badge)](https://coveralls.io/builds/11033662)

          Coverage increased (+0.01%) to 31.855% when pulling *633ac28b8abc63e388d6ebe6081c41371b69ed29 on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11033662/badge)](https://coveralls.io/builds/11033662 ) Coverage increased (+0.01%) to 31.855% when pulling * 633ac28b8abc63e388d6ebe6081c41371b69ed29 on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11033662/badge)](https://coveralls.io/builds/11033662)

          Coverage increased (+0.01%) to 31.855% when pulling *633ac28b8abc63e388d6ebe6081c41371b69ed29 on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11033662/badge)](https://coveralls.io/builds/11033662 ) Coverage increased (+0.01%) to 31.855% when pulling * 633ac28b8abc63e388d6ebe6081c41371b69ed29 on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11033662/badge)](https://coveralls.io/builds/11033662)

          Coverage increased (+0.01%) to 31.855% when pulling *633ac28b8abc63e388d6ebe6081c41371b69ed29 on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11033662/badge)](https://coveralls.io/builds/11033662 ) Coverage increased (+0.01%) to 31.855% when pulling * 633ac28b8abc63e388d6ebe6081c41371b69ed29 on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vesense commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user vesense commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 LGTM
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vongosling commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111316295

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java —
          @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) {
          }
          } else {
          if (!responseFuture.isSendRequestOK()) {

          • pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause()));
              • End diff –

          Please replace {} with +

          Show
          githubbot ASF GitHub Bot added a comment - Github user vongosling commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111316295 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java — @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) { } } else { if (!responseFuture.isSendRequestOK()) { pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause())); End diff – Please replace {} with +
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11085103/badge)](https://coveralls.io/builds/11085103)

          Coverage increased (+2.8%) to 34.664% when pulling *790924535b168cc88b5b064c9aa2722aed8b4fee on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11085103/badge)](https://coveralls.io/builds/11085103 ) Coverage increased (+2.8%) to 34.664% when pulling * 790924535b168cc88b5b064c9aa2722aed8b4fee on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11085103/badge)](https://coveralls.io/builds/11085103)

          Coverage increased (+2.8%) to 34.664% when pulling *790924535b168cc88b5b064c9aa2722aed8b4fee on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11085103/badge)](https://coveralls.io/builds/11085103 ) Coverage increased (+2.8%) to 34.664% when pulling * 790924535b168cc88b5b064c9aa2722aed8b4fee on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11085103/badge)](https://coveralls.io/builds/11085103)

          Coverage increased (+2.8%) to 34.664% when pulling *790924535b168cc88b5b064c9aa2722aed8b4fee on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11085103/badge)](https://coveralls.io/builds/11085103 ) Coverage increased (+2.8%) to 34.664% when pulling * 790924535b168cc88b5b064c9aa2722aed8b4fee on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111552422

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java —
          @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) {
          }
          } else {
          if (!responseFuture.isSendRequestOK()) {

          • pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause()));
              • End diff –

          But this is not a sl4j 's logging, it's a exception, so direct string is needed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111552422 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java — @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) { } } else { if (!responseFuture.isSendRequestOK()) { pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause())); End diff – But this is not a sl4j 's logging, it's a exception, so direct string is needed.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11085446/badge)](https://coveralls.io/builds/11085446)

          Coverage increased (+2.8%) to 34.604% when pulling *c46e4c6e8bccaee3aeee2202fa04db378563f9f3 on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11085446/badge)](https://coveralls.io/builds/11085446 ) Coverage increased (+2.8%) to 34.604% when pulling * c46e4c6e8bccaee3aeee2202fa04db378563f9f3 on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111700110

          — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java —
          @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) {

          if (update) {
          Collections.shuffle(addrs);
          + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old);
          — End diff –

          NEW, OLD look ugly to me
          How about
          ```
          log.info("name server address updated from [{}] to [{}]", addrs, old);
          ```
          ?

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111700110 — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java — @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) { if (update) { Collections.shuffle(addrs); + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old); — End diff – NEW, OLD look ugly to me How about ``` log.info("name server address updated from [{}] to [{}] ", addrs, old); ``` ?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111700022

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java —
          @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) {
          }
          } else {
          if (!responseFuture.isSendRequestOK()) {

          • pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause()));
            + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause()));
              • End diff –

          Or ".request: " -> ". Request: "

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111700022 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java — @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) { } } else { if (!responseFuture.isSendRequestOK()) { pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause())); + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause())); End diff – Or ".request: " -> ". Request: "
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111699928

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java —
          @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) {
          }
          } else {
          if (!responseFuture.isSendRequestOK()) {

          • pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause()));
            + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause()));
              • End diff –

          How about writing it this way?
          `new MQClientException("send request [" + request+ "] failed to " + addr, responseFuture.getCause())`

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111699928 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java — @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) { } } else { if (!responseFuture.isSendRequestOK()) { pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause())); + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause())); End diff – How about writing it this way? `new MQClientException("send request [" + request+ "] failed to " + addr, responseFuture.getCause())`
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111699977

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java —
          @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) {
          }
          } else {
          if (!responseFuture.isSendRequestOK())

          { - pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause())); + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause())); }

          else if (responseFuture.isTimeout()) {

          • pullCallback.onException(new MQClientException("wait response timeout " + responseFuture.getTimeoutMillis() + "ms",
            + pullCallback.onException(new MQClientException("wait response from " + addr + " timeout :" + responseFuture.getTimeoutMillis() + "ms" + " request: " + request,
              • End diff –

          " request: " -> ", request: "

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111699977 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java — @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) { } } else { if (!responseFuture.isSendRequestOK()) { - pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause())); + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause())); } else if (responseFuture.isTimeout()) { pullCallback.onException(new MQClientException("wait response timeout " + responseFuture.getTimeoutMillis() + "ms", + pullCallback.onException(new MQClientException("wait response from " + addr + " timeout :" + responseFuture.getTimeoutMillis() + "ms" + " request: " + request, End diff – " request: " -> ", request: "
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701497

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java —
          @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) {
          }
          } else {
          if (!responseFuture.isSendRequestOK()) {

          • pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause()));
            + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause()));
              • End diff –

          I would modify the typo , your NO2. advice

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701497 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java — @@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture responseFuture) { } } else { if (!responseFuture.isSendRequestOK()) { pullCallback.onException(new MQClientException("send request failed", responseFuture.getCause())); + pullCallback.onException(new MQClientException("send request failed to " + addr + ".request: " + request, responseFuture.getCause())); End diff – I would modify the typo , your NO2. advice
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701589

          — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java —
          @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) {

          if (update) {
          Collections.shuffle(addrs);
          + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old);
          — End diff –

          @shroman , but OLD , NEW has been already exists in rocketmq's log. please see ConsumerGroupInfo.java for one sample.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701589 — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java — @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) { if (update) { Collections.shuffle(addrs); + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old); — End diff – @shroman , but OLD , NEW has been already exists in rocketmq's log. please see ConsumerGroupInfo.java for one sample.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701802

          — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java —
          @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) {

          if (update) {
          Collections.shuffle(addrs);
          + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old);
          — End diff –

          just a suggestion to make it prettier

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701802 — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java — @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) { if (update) { Collections.shuffle(addrs); + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old); — End diff – just a suggestion to make it prettier
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11106460/badge)](https://coveralls.io/builds/11106460)

          Coverage increased (+2.7%) to 34.586% when pulling *ec7c8cdf0581f8ff51ba0e27dc1cbbde9fefdfb9 on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11106460/badge)](https://coveralls.io/builds/11106460 ) Coverage increased (+2.7%) to 34.586% when pulling * ec7c8cdf0581f8ff51ba0e27dc1cbbde9fefdfb9 on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11106460/badge)](https://coveralls.io/builds/11106460)

          Coverage increased (+2.7%) to 34.586% when pulling *ec7c8cdf0581f8ff51ba0e27dc1cbbde9fefdfb9 on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11106460/badge)](https://coveralls.io/builds/11106460 ) Coverage increased (+2.7%) to 34.586% when pulling * ec7c8cdf0581f8ff51ba0e27dc1cbbde9fefdfb9 on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11106460/badge)](https://coveralls.io/builds/11106460)

          Coverage increased (+2.7%) to 34.586% when pulling *ec7c8cdf0581f8ff51ba0e27dc1cbbde9fefdfb9 on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11106460/badge)](https://coveralls.io/builds/11106460 ) Coverage increased (+2.7%) to 34.586% when pulling * ec7c8cdf0581f8ff51ba0e27dc1cbbde9fefdfb9 on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11107831/badge)](https://coveralls.io/builds/11107831)

          Coverage increased (+2.7%) to 34.492% when pulling *1e8b575530f32bdb1fac17c906760b947385517d on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11107831/badge)](https://coveralls.io/builds/11107831 ) Coverage increased (+2.7%) to 34.492% when pulling * 1e8b575530f32bdb1fac17c906760b947385517d on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11107831/badge)](https://coveralls.io/builds/11107831)

          Coverage increased (+2.7%) to 34.492% when pulling *1e8b575530f32bdb1fac17c906760b947385517d on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11107831/badge)](https://coveralls.io/builds/11107831 ) Coverage increased (+2.7%) to 34.492% when pulling * 1e8b575530f32bdb1fac17c906760b947385517d on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          [![Coverage Status](https://coveralls.io/builds/11107831/badge)](https://coveralls.io/builds/11107831)

          Coverage increased (+2.7%) to 34.492% when pulling *1e8b575530f32bdb1fac17c906760b947385517d on Jaskey:ROCKETMQ-172-log-improvement* into *a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 [! [Coverage Status] ( https://coveralls.io/builds/11107831/badge)](https://coveralls.io/builds/11107831 ) Coverage increased (+2.7%) to 34.492% when pulling * 1e8b575530f32bdb1fac17c906760b947385517d on Jaskey: ROCKETMQ-172 -log-improvement * into * a8fa05e8c0b0e57b6e2278747a5f6a8111447a8e on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          @shroman Please review again

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 @shroman Please review again
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on a diff in the pull request:

          https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111720177

          — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java —
          @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) {

          if (update) {
          Collections.shuffle(addrs);
          + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old);
          — End diff –

          Perhaps we can post another PR for some logging format, currently I would just retain the current way

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111720177 — Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java — @@ -321,6 +321,7 @@ public void updateNameServerAddressList(List<String> addrs) { if (update) { Collections.shuffle(addrs); + log.info("name server address updated. NEW : {} , OLD: {}",addrs,old); — End diff – Perhaps we can post another PR for some logging format, currently I would just retain the current way
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vongosling commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          LGTM +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 LGTM +1
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c183e0d4026770a68bedce02507446431cdf6265 in incubator-rocketmq's branch refs/heads/develop from Jaskey Lam
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=c183e0d ]

          ROCKETMQ-172log improvement for rocketmq client closes apache/incubator-rocketmq#90

          Show
          jira-bot ASF subversion and git services added a comment - Commit c183e0d4026770a68bedce02507446431cdf6265 in incubator-rocketmq's branch refs/heads/develop from Jaskey Lam [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=c183e0d ] ROCKETMQ-172 log improvement for rocketmq client closes apache/incubator-rocketmq#90
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          @Jaskey This PR has been merged

          Show
          githubbot ASF GitHub Bot added a comment - Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 @Jaskey This PR has been merged
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/90

          @dongeforever @vongosling @shroman

          Thank you guys!

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 @dongeforever @vongosling @shroman Thank you guys!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey closed the pull request at:

          https://github.com/apache/incubator-rocketmq/pull/90

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/90
          Hide
          zander dongeforever added a comment -

          Github user Jaskey closed the pull request at:
          https://github.com/apache/incubator-rocketmq/pull/90

          Show
          zander dongeforever added a comment - Github user Jaskey closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/90

            People

            • Assignee:
              Jaskey Jaskey Lam
              Reporter:
              Jaskey Jaskey Lam
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development