[jira] [Closed] (GEODE-8298) member version comparison sense inconsistent when deciding on multicast

["Nabarun Nag (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GEODE-8298?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nabarun Nag closed GEODE-8298.
------------------------------

> 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
>             Fix For: 1.14.0
>
>
> 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)