You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@rocketmq.apache.org by Jaskey <gi...@git.apache.org> on 2017/04/01 07:45:15 UTC

[GitHub] incubator-rocketmq pull request #86: [ROCKETMQ-160]SendHeartBeat may not be ...

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 <li...@gmail.com>
Date:   2017-04-01T07:38:49Z

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

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by dongeforever <gi...@git.apache.org>.

Github user dongeforever commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/86
  
    @Jaskey Yeah. it has been merged


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by Jaskey <gi...@git.apache.org>.

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by shroman <gi...@git.apache.org>.

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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by Jaskey <gi...@git.apache.org>.

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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by coveralls <gi...@git.apache.org>.

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**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by Jaskey <gi...@git.apache.org>.

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?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by coveralls <gi...@git.apache.org>.

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**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by Jaskey <gi...@git.apache.org>.

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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by zhouxinyu <gi...@git.apache.org>.

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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

Posted by vsair <gi...@git.apache.org>.

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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---