Uploaded image for project: 'Apache RocketMQ'
  1. Apache RocketMQ
  2. ROCKETMQ-160

SendHeartBeart log may not be triggered in the same expected period

    Details

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

      Description

          private void sendHeartbeatToAllBroker() {
              final HeartbeatData heartbeatData = this.prepareHeartbeatData();
              final boolean producerEmpty = heartbeatData.getProducerDataSet().isEmpty();
              final boolean consumerEmpty = heartbeatData.getConsumerDataSet().isEmpty();
              if (producerEmpty && consumerEmpty) {
                  log.warn("sending heartbeat, but no consumer and no producer");
                  return;
              }
      
              long times = this.storeTimesTotal.getAndIncrement();//here every time when the method is call, the times will increase even though acatully no heartbeat is sent
              Iterator<Entry<String, HashMap<Long, String>>> it = this.brokerAddrTable.entrySet().iterator();
              while (it.hasNext()) {
                  Entry<String, HashMap<Long, String>> entry = it.next();
                  String brokerName = entry.getKey();
                  HashMap<Long, String> oneTable = entry.getValue();
                  if (oneTable != null) {
                      for (Map.Entry<Long, String> entry1 : oneTable.entrySet()) {
                          Long id = entry1.getKey();
                          String addr = entry1.getValue();
                          if (addr != null) {
                              if (consumerEmpty) {
                                  if (id != MixAll.MASTER_ID)
                                      continue;
                              }
      
                              try {
                                  this.mQClientAPIImpl.sendHearbeat(addr, heartbeatData, 3000);
                                  if (times % 20 == 0) {//since the first call is times !=1, the heart beat log for the first heart beat could be missed
                                      log.info("send heart beat to broker[{} {} {}] success", brokerName, id, addr);
                                      log.info(heartbeatData.toString());
                                  }
                              } catch (Exception e) {
                                  if (this.isBrokerInNameServer(addr)) {
                                      log.error("send heart beat to broker exception", e);
                                  } else {
                                      log.info("send heart beat to broker[{} {} {}] exception, because the broker not up, forget it", brokerName,
                                          id, addr);
                                  }
                              }
                          }
                      }
                  }
              }
          }
      

        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/86

          ROCKETMQ-160SendHeartBeat may not be logged in the expected period

          JIRA:https://issues.apache.org/jira/browse/ROCKETMQ-160

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

          $ git pull https://github.com/Jaskey/incubator-rocketmq ROCKETMQ-160-heartbeat-log

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

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


          commit fab38018a89ace83a079f0f39c233dff91735e14
          Author: Jaskey <linjunjie1103@gmail.com>
          Date: 2017-04-01T07:38:49Z

          ROCKETMQ-160-SendHeartBeart log may not be triggered in the same expected period


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/86 ROCKETMQ-160 SendHeartBeat may not be logged in the expected period JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-160 You can merge this pull request into a Git repository by running: $ git pull https://github.com/Jaskey/incubator-rocketmq ROCKETMQ-160 -heartbeat-log Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/86.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 #86 commit fab38018a89ace83a079f0f39c233dff91735e14 Author: Jaskey <linjunjie1103@gmail.com> Date: 2017-04-01T07:38:49Z ROCKETMQ-160 -SendHeartBeart log may not be triggered in the same expected period
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

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

          Coverage decreased (-0.004%) to 31.879% when pulling *fab38018a89ace83a079f0f39c233dff91735e14 on Jaskey:ROCKETMQ-160-heartbeat-log* into *45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 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/86 [! [Coverage Status] ( https://coveralls.io/builds/10880912/badge)](https://coveralls.io/builds/10880912 ) Coverage decreased (-0.004%) to 31.879% when pulling * fab38018a89ace83a079f0f39c233dff91735e14 on Jaskey: ROCKETMQ-160 -heartbeat-log * into * 45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 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/86

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

          Coverage decreased (-0.004%) to 31.879% when pulling *fab38018a89ace83a079f0f39c233dff91735e14 on Jaskey:ROCKETMQ-160-heartbeat-log* into *45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 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/86 [! [Coverage Status] ( https://coveralls.io/builds/10880912/badge)](https://coveralls.io/builds/10880912 ) Coverage decreased (-0.004%) to 31.879% when pulling * fab38018a89ace83a079f0f39c233dff91735e14 on Jaskey: ROCKETMQ-160 -heartbeat-log * into * 45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 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/86

          @lizhanhui @zhouxinyu
          Any guys come to review this trival changes?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @lizhanhui @zhouxinyu Any guys come to review this trival changes?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on the issue:

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

          @shroman

          For your first point, I would like to remain the existing flow? Because the heart beat sent may be failed and if we only mark the success one, we maybe miss the log for long turn when send heartbeat is failed for many times and then become success. But anyway this is a minor issue , I will follow your advice if you guys think it is better.

          For your second point, I agree and I will proceed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @shroman For your first point, I would like to remain the existing flow? Because the heart beat sent may be failed and if we only mark the success one, we maybe miss the log for long turn when send heartbeat is failed for many times and then become success. But anyway this is a minor issue , I will follow your advice if you guys think it is better. For your second point, I agree and I will proceed.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhouxinyu commented on the issue:

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

          IMO, log SendHeartBeat records of `all brokers` every 20 times is reasonable.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 IMO, log SendHeartBeat records of `all brokers` every 20 times is reasonable.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on the issue:

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

          @zhouxinyu , hmm, it depends on what we need.

          The issue I post is that , the log is not doing as expected when the addr table is empty. Even though log for all broker is reasonable, a empty check will be needed.

          I will follow your advice after more guys review.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @zhouxinyu , hmm, it depends on what we need. The issue I post is that , the log is not doing as expected when the addr table is empty. Even though log for all broker is reasonable, a empty check will be needed. I will follow your advice after more guys review.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on the issue:

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

          @Jaskey Incrementing and logging heartbeats on success seems reasonable to me. All failures are logged in `catch` clause.

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @Jaskey Incrementing and logging heartbeats on success seems reasonable to me. All failures are logged in `catch` clause.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on the issue:

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

          @lizhanhui yes it seems reasonable to send heartbeat for all broker as one time.

          Please review the updated pr which only
          1. check if the beartbeat table is empty, only log and calculate times when it is not empty
          2. rename the heartbeat times variable

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @lizhanhui yes it seems reasonable to send heartbeat for all broker as one time. Please review the updated pr which only 1. check if the beartbeat table is empty, only log and calculate times when it is not empty 2. rename the heartbeat times variable
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

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

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

          Coverage increased (+0.04%) to 37.891% when pulling *d86e994c68d8373895c554d543168ef4b5e5d426 on Jaskey:ROCKETMQ-160-heartbeat-log* into *6a9628b3c3e6835e37baf7b58ad9300364d4d384 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/86 [! [Coverage Status] ( https://coveralls.io/builds/11211057/badge)](https://coveralls.io/builds/11211057 ) Coverage increased (+0.04%) to 37.891% when pulling * d86e994c68d8373895c554d543168ef4b5e5d426 on Jaskey: ROCKETMQ-160 -heartbeat-log * into * 6a9628b3c3e6835e37baf7b58ad9300364d4d384 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/86

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

          Coverage increased (+0.04%) to 37.891% when pulling *d86e994c68d8373895c554d543168ef4b5e5d426 on Jaskey:ROCKETMQ-160-heartbeat-log* into *6a9628b3c3e6835e37baf7b58ad9300364d4d384 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/86 [! [Coverage Status] ( https://coveralls.io/builds/11211057/badge)](https://coveralls.io/builds/11211057 ) Coverage increased (+0.04%) to 37.891% when pulling * d86e994c68d8373895c554d543168ef4b5e5d426 on Jaskey: ROCKETMQ-160 -heartbeat-log * into * 6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vsair commented on the issue:

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

          IMO, it's good and reasonable that counter should record the heartbeat's real request.

          Show
          githubbot ASF GitHub Bot added a comment - Github user vsair commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 IMO, it's good and reasonable that counter should record the heartbeat's real request.
          Hide
          jira-bot ASF subversion and git services added a comment -

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

          ROCKETMQ-160SendHeartBeat may not be logged in the expected period closes apache/incubator-rocketmq#86

          Show
          jira-bot ASF subversion and git services added a comment - Commit 04c8925d6da77a41cef746a9c6478a407c4c9edd in incubator-rocketmq's branch refs/heads/develop from Jaskey Lam [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=04c8925 ] ROCKETMQ-160 SendHeartBeat may not be logged in the expected period closes apache/incubator-rocketmq#86
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Jaskey commented on the issue:

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

          @dongeforever
          hiw does this pr go, is it been merged?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @dongeforever hiw does this pr go, is it been merged?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever commented on the issue:

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

          @Jaskey Yeah. it 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/86 @Jaskey Yeah. it has been merged
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

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

          ROCKETMQ-160SendHeartBeat may not be logged in the expected period closes apache/incubator-rocketmq#86

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1f4b893ce70c615daa0c8fce948cef776c108d92 in incubator-rocketmq's branch refs/heads/master from Jaskey Lam [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=1f4b893 ] ROCKETMQ-160 SendHeartBeat may not be logged in the expected period closes apache/incubator-rocketmq#86

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development