Fix #18668: set interfaces must not be added when loopback mode nor loopback interface are used

[TomaszGaweda]

Fix #18668: set interfaces must not be added when loopback mode nor loopback interface are used

Restores old code of adding setInterfaces, without modification of other branches.

Checklist:

• Labels (Team:, Type:, Source:, Module:) and Milestone set
• Add Add to Release Notes label if changes should be mentioned in release notes or Not Release Notes content if changes are not relevant for release notes
• Request reviewers if possible
• New public APIs have @Nonnull/@Nullable annotations
• New public APIs have @since tags in Javadoc
• Send backports/forwardports if fix needs to be applied to past/future releases

[hz-devops-test]

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file

--------------------------
---------SUMMARY----------
--------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.1:checkstyle (default) on project hazelcast: An error has occurred in Checkstyle report generation. Failed during checkstyle execution: There is 1 error reported by Checkstyle 8.38 with /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-builder/checkstyle/checkstyle.xml ruleset. -> [Help 1]
--------------------------
---------ERRORS-----------
--------------------------
[ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-builder/hazelcast/src/main/java/com/hazelcast/internal/cluster/impl/MulticastService.java:127:68: '{' is not preceded with whitespace. [WhitespaceAround]
--------------------------

[TomaszGaweda]

Sorry, forgot to run checkstyle again after some rearrangement of code. Should be fixed now

[hz-devops-test]

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file

--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   MulticastDeserializationTest.testWithoutFilter:103->lambda$testWithoutFilter$0:103 Object was not deserialized
[INFO] 
[ERROR] Tests run: 3990, Failures: 1, Errors: 0, Skipped: 20
[INFO] 
[ERROR] There are test failures.

[TomaszGaweda]

@vbekiaris TBH I have no idea why this test fails, works on my machine. Do you have any ideas?

Edit: test seems to be flaky, fails also on my computer after few tries

[hz-devops-test]

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file

--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   MulticastDeserializationTest.testWithoutFilter:103->lambda$testWithoutFilter$0:103 Object was not deserialized
[INFO] 
[ERROR] Tests run: 3997, Failures: 1, Errors: 0, Skipped: 20
[INFO] 
[ERROR] There are test failures.

[TomaszGaweda]

I have no idea o.O

[vbekiaris]

I haven't properly reviewed this PR yet, however it feels like effects of multicast socket address binding are platform-dependent. I don't like exposing yet another config to users, but this seems like one of the cases where it's probably safer to make it user configurable as proposed in #18403. /cc @kwart wdyt?

[TomaszGaweda]

Sounds ok at first look, but up to know it.. just worked :) On 4.1.1 multicast works on our test environment (CentOS 7) and our local Windows machines. Also, test is about deserialization, while connectivity seems to have nothing to do with serialization.
I'm not against more general PR, just throwing my thoughts, as those fails are really strange

[kwart]

I agree with Vassilis, we hit repeatedly issues in this area. So we should allow users to explicitly force (or disable) the MulticastSocket.setInterface(...) call.
I'm suggesting a solution which adds a new cluster property in TomaszGaweda#8. Using the property allows simple backporting. Most users should not care about this configuration option and default multicast service behavior should work for them.
@vbekiaris Could you also take a look at TomaszGaweda#8?

The new property doesn't have a test coverage (yet). I'll think if there is a simple way to test it.
Update: tests were added in a new commit.

[vbekiaris]

@kwart thanks for looking into this. The approach in TomaszGaweda#8 looks good.
In terms of configuration, maybe we can add the cluster property as suggested in 4.2.z and add it as deprecated in master with additional proper config support in MulticastConfig.

[kwart]

Vassilis, I don't think this Java issue workaround deserves an extra field in the MulticastConfig. I would leave it as the cluster&system property only.

[TomaszGaweda]

Hi @kwart, your PR works great, thank you :)

[kwart]

@TomaszGaweda thanks for testing and fix confirmation!

[vbekiaris]

thanks @TomaszGaweda & @kwart for resolving the issue. Can we backport the fix to 4.2.z?