> On Aug. 21, 2015, 5:12 p.m., Jason Huynh wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/cache/util/BoundedLinkedHashMap.java, > > line 20 > > <https://reviews.apache.org/r/37681/diff/2/?file=1046780#file1046780line20> > > > > is 5.7 a gemfire version? If so, do we need to mention it or can we > > just say it's deprecated?
I hadn't realized this class was a public API and had removed this deprecation notice. Darrel is going to remove the class so I copied it into GMSMembershipManager and reverted the deprecation change. > On Aug. 21, 2015, 5:12 p.m., Jason Huynh wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java, > > line 1143 > > <https://reviews.apache.org/r/37681/diff/2/?file=1046781#file1046781line1143> > > > > 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? Good point. The members collection needs to be a list but the other two don't. Let's change that. > On Aug. 21, 2015, 5:12 p.m., Jason Huynh wrote: > > gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java, > > line 360 > > <https://reviews.apache.org/r/37681/diff/2/?file=1046786#file1046786line360> > > > > 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. I'll rename it to waitForViewAndNoRequestsInProgress. Yes, I did want to make sure there weren't any subsequent views being created after we see the view we're looking for. - Bruce ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37681/#review96055 ----------------------------------------------------------- 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 > >
