Details

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

      Description

      1. in MQClientManager#getAndCreateMQClientInstance line 57
      when generate a new key-value pair for client in factoryTable, if there was no mapping for the clientId previously, putIfAbsent will return null, and then output:
      "log.warn("Previous MQClientInstance has created for clientId:[{}]", clientId);"
      this is wrong, it should output in condition prev != null

      2. in SubscriptionGroupManager#deleteSubscriptionGroupConfig line 187
      if program execute:
      log.warn("delete subscription group failed, subscription group: {} not exist", old);
      variable old is always null , it's useless, so it's better to print groupName

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user a2888409 opened a pull request:

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

          ROCKETMQ-37 Log output information is not accurate

          Jira issue: https://issues.apache.org/jira/browse/ROCKETMQ-37

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

          $ git pull https://github.com/a2888409/incubator-rocketmq master

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

          https://github.com/apache/incubator-rocketmq/pull/33.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 #33


          commit 633eca2665edca4999b28f3f57857649d821c52c
          Author: qianzhenyu <122274659@qq.com>
          Date: 2017-01-09T08:54:31Z

          Fix-37 ROCKETMQ-37 Log output information is not accurate


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user a2888409 opened a pull request: https://github.com/apache/incubator-rocketmq/pull/33 ROCKETMQ-37 Log output information is not accurate Jira issue: https://issues.apache.org/jira/browse/ROCKETMQ-37 You can merge this pull request into a Git repository by running: $ git pull https://github.com/a2888409/incubator-rocketmq master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/33.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 #33 commit 633eca2665edca4999b28f3f57857649d821c52c Author: qianzhenyu <122274659@qq.com> Date: 2017-01-09T08:54:31Z Fix-37 ROCKETMQ-37 Log output information is not accurate
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhouxinyu commented on the issue:

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

          It seems ok, thanks @a2888409

          Please @vongosling @lizhanhui help review this PR.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/33 It seems ok, thanks @a2888409 Please @vongosling @lizhanhui help review this PR.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/incubator-rocketmq/pull/33#discussion_r95966853

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientManager.java —
          @@ -53,8 +53,9 @@ public MQClientInstance getAndCreateMQClientInstance(final ClientConfig clientCo
          MQClientInstance prev = this.factoryTable.putIfAbsent(clientId, instance);
          if (prev != null)

          { instance = prev; - }

          else {
          log.warn("Previous MQClientInstance has created for clientId:[{}]", clientId);
          + } else {
          + log.info("new MQClientInstance has created for clientId:[{}]", clientId);
          — End diff –

          It will be best if using active tune here. Something like, Returned previous MQClientInstance for clientId: ..., Created new MQClientInstance for clientId: ...

          Show
          githubbot ASF GitHub Bot added a comment - Github user lizhanhui commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/33#discussion_r95966853 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientManager.java — @@ -53,8 +53,9 @@ public MQClientInstance getAndCreateMQClientInstance(final ClientConfig clientCo MQClientInstance prev = this.factoryTable.putIfAbsent(clientId, instance); if (prev != null) { instance = prev; - } else { log.warn("Previous MQClientInstance has created for clientId: [{}] ", clientId); + } else { + log.info("new MQClientInstance has created for clientId: [{}] ", clientId); — End diff – It will be best if using active tune here. Something like, Returned previous MQClientInstance for clientId: ..., Created new MQClientInstance for clientId: ...
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/incubator-rocketmq/pull/33#discussion_r96159731

          — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientManager.java —
          @@ -53,8 +53,9 @@ public MQClientInstance getAndCreateMQClientInstance(final ClientConfig clientCo
          MQClientInstance prev = this.factoryTable.putIfAbsent(clientId, instance);
          if (prev != null)

          { instance = prev; - }

          else {
          log.warn("Previous MQClientInstance has created for clientId:[{}]", clientId);
          + } else {
          + log.info("new MQClientInstance has created for clientId:[{}]", clientId);
          — End diff –

          thanks, i have fixed it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user a2888409 commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/33#discussion_r96159731 — Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientManager.java — @@ -53,8 +53,9 @@ public MQClientInstance getAndCreateMQClientInstance(final ClientConfig clientCo MQClientInstance prev = this.factoryTable.putIfAbsent(clientId, instance); if (prev != null) { instance = prev; - } else { log.warn("Previous MQClientInstance has created for clientId: [{}] ", clientId); + } else { + log.info("new MQClientInstance has created for clientId: [{}] ", clientId); — End diff – thanks, i have fixed it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vongosling commented on the issue:

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

          alright

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

          Commit 69361f60b70436996e3479e2ed4473f4f1ca2af1 in incubator-rocketmq's branch refs/heads/master from QianZhenyu
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=69361f6 ]

          ROCKETMQ-37 Polish log output information in MQClientManager, closes apache/incubator-rocketmq#33, closes apache/incubator-rocketmq#26

          Show
          jira-bot ASF subversion and git services added a comment - Commit 69361f60b70436996e3479e2ed4473f4f1ca2af1 in incubator-rocketmq's branch refs/heads/master from QianZhenyu [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=69361f6 ] ROCKETMQ-37 Polish log output information in MQClientManager, closes apache/incubator-rocketmq#33, closes apache/incubator-rocketmq#26
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/33
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 69361f60b70436996e3479e2ed4473f4f1ca2af1 in incubator-rocketmq's branch refs/heads/ROCKETMQ-57 from QianZhenyu
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=69361f6 ]

          ROCKETMQ-37 Polish log output information in MQClientManager, closes apache/incubator-rocketmq#33, closes apache/incubator-rocketmq#26

          Show
          jira-bot ASF subversion and git services added a comment - Commit 69361f60b70436996e3479e2ed4473f4f1ca2af1 in incubator-rocketmq's branch refs/heads/ ROCKETMQ-57 from QianZhenyu [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=69361f6 ] ROCKETMQ-37 Polish log output information in MQClientManager, closes apache/incubator-rocketmq#33, closes apache/incubator-rocketmq#26

            People

            • Assignee:
              vongosling vongosling
              Reporter:
              qianzhenyu QianZhenyu
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development