[
https://issues.apache.org/jira/browse/GEODE-8298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17161328#comment-17161328
]
ASF subversion and git services commented on GEODE-8298:
--------------------------------------------------------
Commit fd76cc0b7dbf97dfb84d11e67b37e33c0a9e7fb2 in geode's branch
refs/heads/develop from Kamilla Aslami
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=fd76cc0 ]
GEODE-8298: Fix multicast version detection (#5370)
Membership decides whether or not use multicast based on the versions
of view members and surprise members. Surprise member version comparison
was backward. Correct version comparison by unifying view and surprise
member processing.
Co-authored-by: Kamilla Aslami <kamilla.1...@gmail.com>
Co-authored-by: Bill Burcham <bill.burc...@gmail.com>
> member version comparison sense inconsistent when deciding on multicast
> -----------------------------------------------------------------------
>
> Key: GEODE-8298
> URL: https://issues.apache.org/jira/browse/GEODE-8298
> Project: Geode
> Issue Type: Bug
> Components: membership
> Reporter: Bill Burcham
> Assignee: Kamilla Aslami
> Priority: Major
> Labels: pull-request-available, starter
>
> Since about 2014 when we introduced the {{Version}} class to replace use of
> {{short}} s all over the place for serialization versions, these two loops in
> {{GMSMembership.processView()}} have used comparisons that disagree in sense:
> {code}
> // We perform the update under a global lock so that other
> // incoming events will not be lost in terms of our global view.
> latestViewWriteLock.lock();
> try {
> // first determine the version for multicast message serialization
> VersionOrdinal version = Version.CURRENT;
> for (final Entry<ID, Long> internalIDLongEntry : surpriseMembers
> .entrySet()) {
> ID mbr = internalIDLongEntry.getKey();
> final VersionOrdinal itsVersion = mbr.getVersionObject();
> if (itsVersion != null && version.compareTo(itsVersion) < 0) {
> version = itsVersion;
> }
> }
> for (ID mbr : newView.getMembers()) {
> final VersionOrdinal itsVersion = mbr.getVersionObject();
> if (itsVersion != null && itsVersion.compareTo(version) < 0) {
> version = mbr.getVersionObject();
> }
> }
> disableMulticastForRollingUpgrade = !version.equals(Version.CURRENT);
> {code}
> The goal here is to find the oldest version and if that version is older than
> our local version we disable multicast. So we want to put the minimum into
> {{version}}. So the first loop's comparison is wrong and the second one is
> right.
> While we are in here let's combine the two loops using
> {{Stream.concat(surpriseMembers.entrySet().stream().map(entry->entry.getKey()),
> newView.getMembers().stream()).forEach(member -> ...)}}.
> Alternatives are described here:
> https://www.baeldung.com/java-combine-multiple-collections
> Once we have the combined {{Iterable}} we can use something like
> {{Collections.min()}} to find the minimum in one swell foop and this whole
> thing collapses to one or two declarative expressions.
> When this story is complete, the functionality will be in a separate method
> and we'll have a unit test for it.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
