----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37681/#review96055 -----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/cache/util/BoundedLinkedHashMap.java (line 20) <https://reviews.apache.org/r/37681/#comment151247> is 5.7 a gemfire version? If so, do we need to mention it or can we just say it's deprecated? gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java (line 1141) <https://reviews.apache.org/r/37681/#comment151252> Not really an issue but just a question: Is there a reason why we use lists instead of sets? It seems like we have a lot of checks in the code (not just these methods) to see if a list contains something before adding it, where a set would be able to handle that? gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java (line 360) <https://reviews.apache.org/r/37681/#comment151254> This method also waits until no more requests are pending. When I first read the name I thought it just waited for the view to be seen before continuing but it looks like it waits until things have "settled." Not sure the best way to make this clearer, maybe a comment or waitForFinalView Thought I'd mention it to anyone else reviewing. gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java (line 498) <https://reviews.apache.org/r/37681/#comment151257> This might be the same as testOlderPreparedViewBeforeFirstViewInstalled. If so, feel free to delete testOlderPreparedViewBeforeFirstViewInstalled. - Jason Huynh On Aug. 21, 2015, 4:40 p.m., Bruce Schuchardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37681/ > ----------------------------------------------------------- > > (Updated Aug. 21, 2015, 4:40 p.m.) > > > Review request for geode, anilkumar gingade, Hitesh Khamesra, Jason Huynh, > Jianxia Chen, Lynn Gallinat, and Qihong Chen. > > > Repository: geode > > > Description > ------- > > Integration testing showed that we did not have adequate code coverage for > GMSJoinLeave in unit tests. This change set adds a number of new tests and > includes fixes for minor problems they exposed: > > Members were in the view multiple times. > Members were in the shutdownMembers collection multiple times. > Members could be in the crashedMembers collection multiple times. > View preparation was actually installing the view in the coordinator. > A rogue process could request removal of a member. > Quorum-loss notification needed to be tested. > Network partition detection needed to be tested. > Conflicting views during view preparation needed to be tested. > Coordinator election when the old coordinator leaves needed to be tested. > > There were also some logging issues: > > Views were not being logged when received. > Quorum-loss calculation results were not being logged. > > > Diffs > ----- > > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/util/BoundedLinkedHashMap.java > 1abf24f > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java > d3075bb > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/LeaveRequestMessage.java > 61183a8 > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java > 656230a > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java > 76d83f7 > > gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/MembershipManagerHelper.java > a4eaa40 > > gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java > 4eec06a > > Diff: https://reviews.apache.org/r/37681/diff/ > > > Testing > ------- > > GMSJoinLeaveJUnitTest, hydra integration testing. > > > Thanks, > > Bruce Schuchardt > >
