[
https://issues.apache.org/jira/browse/GEODE-8298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17161327#comment-17161327
]
ASF GitHub Bot commented on GEODE-8298:
---------------------------------------
Bill merged pull request #5370:
URL: https://github.com/apache/geode/pull/5370
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> 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)
