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