You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jianxia Chen <jc...@pivotal.io> on 2015/12/23 21:00:51 UTC
Review Request 41694: GEODE-708 Stats for Geode membership health
monitor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41694/
-----------------------------------------------------------
Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, Hitesh Khamesra, and Jason Huynh.
Repository: geode
Description
-------
Implementation for Geode membership health monitor with unit tests
Refactor the health monitor code a little bit (rename variables and remove unnecessary code)
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/Services.java 4484c00
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java 005b0ed
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitorStats.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitor.java 8b84dfe
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitorStats.java PRE-CREATION
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java d539374
Diff: https://reviews.apache.org/r/41694/diff/
Testing
-------
Thanks,
Jianxia Chen
Re: Review Request 41694: GEODE-708 Stats for Geode membership health
monitor
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41694/#review112085
-----------------------------------------------------------
Ship it!
A couple of very minor changes & you can check it in. I don't think the changes require retesting.
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java (line 142)
<https://reviews.apache.org/r/41694/#comment172396>
delete
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionStats.java (line 354)
<https://reviews.apache.org/r/41694/#comment172395>
this should say 'received' instead of 'sent'
- Bruce Schuchardt
On Dec. 28, 2015, 7:50 p.m., Jianxia Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41694/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2015, 7:50 p.m.)
>
>
> Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, Hitesh Khamesra, and Jason Huynh.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Implementation for Geode membership health monitor with unit tests
> Refactor the health monitor code a little bit (rename variables and remove unnecessary code)
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DMStats.java e79a40b
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionStats.java 804b507
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/LonerDistributionManager.java 60158d1
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java 005b0ed
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java d539374
>
> Diff: https://reviews.apache.org/r/41694/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jianxia Chen
>
>
Re: Review Request 41694: GEODE-708 Stats for Geode membership health
monitor
Posted by Jianxia Chen <jc...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41694/
-----------------------------------------------------------
(Updated Dec. 28, 2015, 7:50 p.m.)
Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, Hitesh Khamesra, and Jason Huynh.
Changes
-------
Move the stats to DMStats
Repository: geode
Description
-------
Implementation for Geode membership health monitor with unit tests
Refactor the health monitor code a little bit (rename variables and remove unnecessary code)
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DMStats.java e79a40b
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionStats.java 804b507
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/LonerDistributionManager.java 60158d1
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java 005b0ed
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java d539374
Diff: https://reviews.apache.org/r/41694/diff/
Testing
-------
Thanks,
Jianxia Chen
Re: Review Request 41694: GEODE-708 Stats for Geode membership health
monitor
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41694/#review111810
-----------------------------------------------------------
I reviewed a little of this but I see enough problems that we're going to need to figure out how to do this differently. We can't assume there is a GemFireCacheImpl around, for instance, because Admin members don't have a cache but need to use a DistributedSystem. I think the best thing to do would be to move the new statistics to DMStats, which is already installed in the GMS Services object and available to the health monitor. That is where the other GMS statistics are being recorded.
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/Services.java (line 171)
<https://reviews.apache.org/r/41694/#comment172066>
Can this be moved to the implementation class? Why should Services need to know to initialize the statistics of one of the services?
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java (line 60)
<https://reviews.apache.org/r/41694/#comment172067>
Referring to GemFireCache may be a problem if we have to split the membership module into another gradle project, which is being discussed.
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java (line 876)
<https://reviews.apache.org/r/41694/#comment172069>
This isn't going to be allowed in an Admin member, which has a DistributedSystem but does not have a cache.
W
- Bruce Schuchardt
On Dec. 23, 2015, 8 p.m., Jianxia Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41694/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2015, 8 p.m.)
>
>
> Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, Hitesh Khamesra, and Jason Huynh.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Implementation for Geode membership health monitor with unit tests
> Refactor the health monitor code a little bit (rename variables and remove unnecessary code)
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/Services.java 4484c00
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java 005b0ed
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitorStats.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitor.java 8b84dfe
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/HealthMonitorStats.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java d539374
>
> Diff: https://reviews.apache.org/r/41694/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jianxia Chen
>
>