----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43059/#review117270 -----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2206) <https://reviews.apache.org/r/43059/#comment178430> I think you can just reuse the newRemovals collection instead of introducing another one that has the same purpose. gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2224) <https://reviews.apache.org/r/43059/#comment178424> Let's get rid if this variable ("now"). It's only used in the while() expression. Then the while() expression can be simplified while (System.currentTimeMillis() <= giveUpTime) gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2225) <https://reviews.apache.org/r/43059/#comment178419> remove debug log statement gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2243) <https://reviews.apache.org/r/43059/#comment178431> Here you should also look for new Leave requests, like was done before creating the futures. That will keep us from declaring a crash for a member that has actually sent a Leave request. gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2246) <https://reviews.apache.org/r/43059/#comment178417> remove or make debug-level gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2259) <https://reviews.apache.org/r/43059/#comment178421> I think this should just break out of the for-loop and the newRemovals (minus newLeaves) should be added back into the mbrs collection when the loop completes. gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2268) <https://reviews.apache.org/r/43059/#comment178422> Since there is already a while() loop with a timer I think this sleep should be pretty short - on the order of the 100ms you're using for the other sleep. gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 2279) <https://reviews.apache.org/r/43059/#comment178427> This needs braces. for () { statements } gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java (line 1115) <https://reviews.apache.org/r/43059/#comment178428> remove TODO - Bruce Schuchardt On Feb. 1, 2016, 8:27 p.m., Hitesh Khamesra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43059/ > ----------------------------------------------------------- > > (Updated Feb. 1, 2016, 8:27 p.m.) > > > Review request for geode, Bruce Schuchardt and Udo Kohlmeyer. > > > Repository: geode > > > Description > ------- > > ViewCreator thread sends another message when any member doesn't ack back to > prepared view. And then it waits on future as this can happen for > multiple members. > In this case, If those members are not responsive and other thread > already determined that, > then we don't need to wait for those members. Thus now viewCreator thread > checks > RemoveMember message for those members while waiting for response. > Another minor fix in same area. > And added unit test for that > > > Diffs > ----- > > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java > b6f6c12 > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java > 6b09214 > > gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java > 5b53290 > > Diff: https://reviews.apache.org/r/43059/diff/ > > > Testing > ------- > > > Thanks, > > Hitesh Khamesra > >
