You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "alex-plekhanov (via GitHub)" <gi...@apache.org> on 2024/02/29 14:48:25 UTC

[PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

alex-plekhanov opened a new pull request, #11260:
URL: https://github.com/apache/ignite/pull/11260

   Thank you for submitting the pull request to the Apache Ignite.
   
   In order to streamline the review of the contribution 
   we ask you to ensure the following steps have been taken:
   
   ### The Contribution Checklist
   - [ ] There is a single JIRA ticket related to the pull request. 
   - [ ] The web-link to the pull request is attached to the JIRA ticket.
   - [ ] The JIRA ticket has the _Patch Available_ state.
   - [ ] The pull request body describes changes that have been made. 
   The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
   - [ ] The pull request title is treated as the final commit message. 
   The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
   - [ ] A reviewer has been mentioned through the JIRA comments 
   (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1522980203


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,136 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ducktape.mark import defaults
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    @defaults(fail=[True, False])
+    def dns_failure_test(self, ignite_version, fail):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, fail)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started()
+
+        assert len(ignites.alive_nodes) == 3, 'All nodes should be alive'
+
+        # Smoke test on full topology.

Review Comment:
   Not sure an additional non-cluster-issue test is necessary. Up to you.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1516123457


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########


Review Comment:
   Can we use anu Java test? it much more simplier to run and debug. Can we use the same system properties and the blocking implementations?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1516123457


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########


Review Comment:
   Can we use anu Java test? it much more simplier to run and debug. Can we use the same system properties and the blocking implementations? Or we could write test like this: https://github.com/apache/ignite/pull/11270/files#diff-6caf9c7788d4f02319af155cf51da2b13ebe85faab231b86b63d7446917fb487 
   
   WDYT ?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1521297432


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    def dns_failure_test(self, ignite_version):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, True)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started()
+
+        assert len(ignites.alive_nodes) == 3, 'All nodes should be alive'
+
+        # Smoke test on full topology.
+        app = IgniteApplicationService(
+            self.test_context,
+            ignite_config._replace(client_mode=True, discovery_spi=from_ignite_cluster(ignites)),
+            startup_timeout_sec=120,
+            java_class_name="org.apache.ignite.internal.ducktest.tests.smoke_test.SimpleApplication")
+
+        app.start()
+        app.stop()
+
+        ignites.stop()
+
+    def __prepare_service(self, ignite_config, num_nodes=1):
+        ignite = IgniteService(
+            self.test_context,
+            ignite_config,
+            startup_timeout_sec=120,
+            num_nodes=num_nodes)
+
+        bootclasspath = list(map(lambda lib: os.path.join(lib, "classes"), ignite.spec._module_libs("ducktests")))
+
+        # Note: Support of impl.prefix property was removed since java 18.
+        ignite.spec.jvm_opts.append("-Dimpl.prefix=BlockingDns")
+        ignite.spec.jvm_opts.append("-Xbootclasspath/a:" + ":".join(bootclasspath))
+
+        return ignite
+
+    @staticmethod
+    def __block_dns(ignite, timeout, fail, nodes=None):

Review Comment:
   This is not a regression test. This test is to show that problem exists and problem is fixed by the patch. I've added this option to test, but don't sure it's necessary, since we can always run this test manually with fail=false option. Result is the same.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1521278745


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    def dns_failure_test(self, ignite_version):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, True)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])

Review Comment:
   In this case node start time is much longer (timeouts should be fixed), but overall, solution is still workable.
   I don't think we need to fix resolveLocalAddresses and resolveLocalHost, since main goal of this ticket is to prevent node failures with minimal inconvenience. Fixing these methods is risky, since local host name can be used as consistentId in some cases, so, we should do our best to resolve this hostname properly.
   Also, there are some calls to resolve hostname inside third-party libraries (log4j, spring as far as I remember), which we can't avoid.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1516126336


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    def dns_failure_test(self, ignite_version):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, True)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])

Review Comment:
   What if node starts with blocked DNS? Do we have test for it? Check the stacktrace:
   
   _getHostByAddr:-1, Inet6AddressImpl 
   getHostByAddr:936, InetAddress$PlatformNameService 
   getHostFromNameService:659, InetAddress 
   getHostName:602, InetAddress 
   getHostName:574, InetAddress 
   addresses:2343, IgniteUtils 
   resolveLocalAddresses:2302, IgniteUtils 
   resolveLocalAddresses:2265, IgniteUtils (org.apache.ignite.internal.util)
   getNodeAttributes:843, TcpCommunicationConfigInitializer (org.apache.ignite.spi.communication.tcp.internal)
   parseNodeAttributes:734, GridManagerAdapter
   startSpi:285, GridManagerAdapter
   start:487, GridIoManager
   startManager:1772, IgniteKernal
   start:1007, IgniteKernal
   start0:1725, IgnitionEx$IgniteNamedInstance
   start:1647, IgnitionEx$IgniteNamedInstance
   start0:1089, IgnitionEx
   start:599, IgnitionEx_
   
   
   What if `getHostByAddr:-1, Inet6AddressImpl (java.net)` is blocked ? Do we need to fix `resolveLocalAddresses()` and `resolveLocalHost()`?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1516126336


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    def dns_failure_test(self, ignite_version):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, True)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])

Review Comment:
   What if node starts with blocked DNS? Do we have test for it? Check the stacktrace:
   
   `getHostByAddr:-1, Inet6AddressImpl 
   getHostByAddr:936, InetAddress$PlatformNameService 
   getHostFromNameService:659, InetAddress 
   getHostName:602, InetAddress 
   getHostName:574, InetAddress 
   addresses:2343, IgniteUtils 
   resolveLocalAddresses:2302, IgniteUtils 
   resolveLocalAddresses:2265, IgniteUtils
   getNodeAttributes:843, TcpCommunicationConfigInitializer
   parseNodeAttributes:734, GridManagerAdapter
   startSpi:285, GridManagerAdapter
   start:487, GridIoManager
   startManager:1772, IgniteKernal
   start:1007, IgniteKernal
   start0:1725, IgnitionEx$IgniteNamedInstance
   start:1647, IgnitionEx$IgniteNamedInstance
   start0:1089, IgnitionEx
   start:599, IgnitionEx
   `
   
   What if `getHostByAddr:-1, Inet6AddressImpl (java.net)` is blocked ?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1521280813


##########
modules/ducktests/src/main/java/java/net/DnsBlocker.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package java.net;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.text.SimpleDateFormat;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.Scanner;
+
+/** */
+public class DnsBlocker {
+    /** */
+    private static final String BLOCK_DNS_FILE = "/tmp/block_dns";
+
+    /** */
+    public static final DnsBlocker INSTANCE = new DnsBlocker();
+
+    /** */
+    private DnsBlocker() {
+        // No-op.
+    }
+
+    /**
+     * Check and block hostname resolve request if needed.
+     * @param impl Implementation.
+     * @param hostname Hostname.
+     */
+    public void onHostResolve(InetAddressImpl impl, String hostname) throws UnknownHostException {
+        if (!impl.loopbackAddress().getHostAddress().equals(hostname))
+            check(hostname);
+    }
+
+    /**
+     * Check and block address resolve request if needed.
+     * @param impl Implementation.
+     * @param addr Address.
+     */
+    public void onAddrResolve(InetAddressImpl impl, byte[] addr) throws UnknownHostException {
+        if (!Arrays.equals(impl.loopbackAddress().getAddress(), addr))
+            check(InetAddress.getByAddress(addr).toString());
+

Review Comment:
   Fixed



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1516515576


##########
modules/ducktests/src/main/java/java/net/DnsBlocker.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package java.net;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.text.SimpleDateFormat;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.Scanner;
+
+/** */
+public class DnsBlocker {
+    /** */
+    private static final String BLOCK_DNS_FILE = "/tmp/block_dns";
+
+    /** */
+    public static final DnsBlocker INSTANCE = new DnsBlocker();
+
+    /** */
+    private DnsBlocker() {
+        // No-op.
+    }
+
+    /**
+     * Check and block hostname resolve request if needed.
+     * @param impl Implementation.
+     * @param hostname Hostname.
+     */
+    public void onHostResolve(InetAddressImpl impl, String hostname) throws UnknownHostException {
+        if (!impl.loopbackAddress().getHostAddress().equals(hostname))
+            check(hostname);
+    }
+
+    /**
+     * Check and block address resolve request if needed.
+     * @param impl Implementation.
+     * @param addr Address.
+     */
+    public void onAddrResolve(InetAddressImpl impl, byte[] addr) throws UnknownHostException {
+        if (!Arrays.equals(impl.loopbackAddress().getAddress(), addr))
+            check(InetAddress.getByAddress(addr).toString());
+

Review Comment:
   empty line



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #11260: IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable
URL: https://github.com/apache/ignite/pull/11260


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1522951414


##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -9763,37 +9703,37 @@ public static Collection<InetSocketAddress> toSocketAddresses(ClusterNode node,
     }
 
     /**
-     * Returns tha list of resolved socket addresses. First addresses are resolved by host names,
-     * if this attempt fails then the addresses are resolved by ip addresses.
+     * Returns the list of resolved socket addresses.
      *
      * @param addrs Addresses.
      * @param hostNames Host names.
      * @param port Port.
      * @return Socket addresses for given addresses and host names.
      */
-    public static Collection<InetSocketAddress> toSocketAddresses(Collection<String> addrs,
-        Collection<String> hostNames, int port) {
+    public static Collection<InetSocketAddress> toSocketAddresses(
+        Collection<String> addrs,
+        Collection<String> hostNames, int port
+    ) {
         Set<InetSocketAddress> res = new HashSet<>(addrs.size());
 
-        Iterator<String> hostNamesIt = hostNames.iterator();
+        boolean hasAddr = false;
 
         for (String addr : addrs) {
-            String hostName = hostNamesIt.hasNext() ? hostNamesIt.next() : null;
+            InetSocketAddress inetSockAddr = createResolved(addr, port);
+            res.add(inetSockAddr);
 
-            if (!F.isEmpty(hostName)) {
-                InetSocketAddress inetSockAddr = createResolved(hostName, port);
+            if (!inetSockAddr.isUnresolved() && !inetSockAddr.getAddress().isLoopbackAddress())

Review Comment:
   Why we ignore loopbacks? Nodes can be launched on the same host. Do we need an external parameter here? To include or exclude loopback as a valid address? See `getEffectiveNodeAddresses(TcpDiscoveryNode node, boolean sameHost)`. It has similar parameter  `boolean sameHost`



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1522924994


##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -9763,37 +9703,37 @@ public static Collection<InetSocketAddress> toSocketAddresses(ClusterNode node,
     }
 
     /**
-     * Returns tha list of resolved socket addresses. First addresses are resolved by host names,
-     * if this attempt fails then the addresses are resolved by ip addresses.
+     * Returns the list of resolved socket addresses.
      *
      * @param addrs Addresses.
      * @param hostNames Host names.
      * @param port Port.
      * @return Socket addresses for given addresses and host names.
      */
-    public static Collection<InetSocketAddress> toSocketAddresses(Collection<String> addrs,
-        Collection<String> hostNames, int port) {
+    public static Collection<InetSocketAddress> toSocketAddresses(
+        Collection<String> addrs,
+        Collection<String> hostNames, int port

Review Comment:
   `int port` to the next line?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1516126336


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    def dns_failure_test(self, ignite_version):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, True)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])

Review Comment:
   What if node starts with blocked DNS? Do we have test for it? Check the stacktrace:
   
   `getHostByAddr:-1, Inet6AddressImpl (java.net)
   getHostByAddr:936, InetAddress$PlatformNameService (java.net)
   getHostFromNameService:659, InetAddress (java.net)
   getHostName:602, InetAddress (java.net)
   getHostName:574, InetAddress (java.net)
   addresses:2343, IgniteUtils (org.apache.ignite.internal.util)
   resolveLocalAddresses:2302, IgniteUtils (org.apache.ignite.internal.util)
   resolveLocalAddresses:2265, IgniteUtils (org.apache.ignite.internal.util)
   getNodeAttributes:843, TcpCommunicationConfigInitializer (org.apache.ignite.spi.communication.tcp.internal)
   parseNodeAttributes:734, GridManagerAdapter (org.apache.ignite.internal.managers)
   startSpi:285, GridManagerAdapter (org.apache.ignite.internal.managers)
   start:487, GridIoManager (org.apache.ignite.internal.managers.communication)
   startManager:1772, IgniteKernal (org.apache.ignite.internal)
   start:1007, IgniteKernal (org.apache.ignite.internal)
   start0:1725, IgnitionEx$IgniteNamedInstance (org.apache.ignite.internal)
   start:1647, IgnitionEx$IgniteNamedInstance (org.apache.ignite.internal)
   start0:1089, IgnitionEx (org.apache.ignite.internal)
   start:599, IgnitionEx (org.apache.ignite.internal)`
   
   What if `getHostByAddr:-1, Inet6AddressImpl (java.net)` is blocked ?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1516126336


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    def dns_failure_test(self, ignite_version):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, True)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])

Review Comment:
   What if node starts with blocked DNS? Do we have test for it? Check the stacktrace:
   
   _getHostByAddr:-1, Inet6AddressImpl 
   getHostByAddr:936, InetAddress$PlatformNameService 
   getHostFromNameService:659, InetAddress 
   getHostName:602, InetAddress 
   getHostName:574, InetAddress 
   addresses:2343, IgniteUtils 
   resolveLocalAddresses:2302, IgniteUtils 
   resolveLocalAddresses:2265, IgniteUtils (org.apache.ignite.internal.util)
   getNodeAttributes:843, TcpCommunicationConfigInitializer (org.apache.ignite.spi.communication.tcp.internal)
   parseNodeAttributes:734, GridManagerAdapter
   startSpi:285, GridManagerAdapter
   start:487, GridIoManager
   startManager:1772, IgniteKernal
   start:1007, IgniteKernal
   start0:1725, IgnitionEx$IgniteNamedInstance
   start:1647, IgnitionEx$IgniteNamedInstance
   start0:1089, IgnitionEx
   start:599, IgnitionEx_
   
   
   What if `getHostByAddr:-1, Inet6AddressImpl (java.net)` is blocked ?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1522951414


##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -9763,37 +9703,37 @@ public static Collection<InetSocketAddress> toSocketAddresses(ClusterNode node,
     }
 
     /**
-     * Returns tha list of resolved socket addresses. First addresses are resolved by host names,
-     * if this attempt fails then the addresses are resolved by ip addresses.
+     * Returns the list of resolved socket addresses.
      *
      * @param addrs Addresses.
      * @param hostNames Host names.
      * @param port Port.
      * @return Socket addresses for given addresses and host names.
      */
-    public static Collection<InetSocketAddress> toSocketAddresses(Collection<String> addrs,
-        Collection<String> hostNames, int port) {
+    public static Collection<InetSocketAddress> toSocketAddresses(
+        Collection<String> addrs,
+        Collection<String> hostNames, int port
+    ) {
         Set<InetSocketAddress> res = new HashSet<>(addrs.size());
 
-        Iterator<String> hostNamesIt = hostNames.iterator();
+        boolean hasAddr = false;
 
         for (String addr : addrs) {
-            String hostName = hostNamesIt.hasNext() ? hostNamesIt.next() : null;
+            InetSocketAddress inetSockAddr = createResolved(addr, port);
+            res.add(inetSockAddr);
 
-            if (!F.isEmpty(hostName)) {
-                InetSocketAddress inetSockAddr = createResolved(hostName, port);
+            if (!inetSockAddr.isUnresolved() && !inetSockAddr.getAddress().isLoopbackAddress())

Review Comment:
   Why we ignore a localhost? Nodes can be launched on the same host. Do we need an external parameter here? To include or exclude localhost as an valid address? See `getEffectiveNodeAddresses(TcpDiscoveryNode node, boolean sameHost)`. It has similar parameter  `boolean sameHost`



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1522951414


##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -9763,37 +9703,37 @@ public static Collection<InetSocketAddress> toSocketAddresses(ClusterNode node,
     }
 
     /**
-     * Returns tha list of resolved socket addresses. First addresses are resolved by host names,
-     * if this attempt fails then the addresses are resolved by ip addresses.
+     * Returns the list of resolved socket addresses.
      *
      * @param addrs Addresses.
      * @param hostNames Host names.
      * @param port Port.
      * @return Socket addresses for given addresses and host names.
      */
-    public static Collection<InetSocketAddress> toSocketAddresses(Collection<String> addrs,
-        Collection<String> hostNames, int port) {
+    public static Collection<InetSocketAddress> toSocketAddresses(
+        Collection<String> addrs,
+        Collection<String> hostNames, int port
+    ) {
         Set<InetSocketAddress> res = new HashSet<>(addrs.size());
 
-        Iterator<String> hostNamesIt = hostNames.iterator();
+        boolean hasAddr = false;
 
         for (String addr : addrs) {
-            String hostName = hostNamesIt.hasNext() ? hostNamesIt.next() : null;
+            InetSocketAddress inetSockAddr = createResolved(addr, port);
+            res.add(inetSockAddr);
 
-            if (!F.isEmpty(hostName)) {
-                InetSocketAddress inetSockAddr = createResolved(hostName, port);
+            if (!inetSockAddr.isUnresolved() && !inetSockAddr.getAddress().isLoopbackAddress())

Review Comment:
   Why we ignore loopbacks? Nodes can be launched on the same host. Do we need an external parameter here? To include or exclude loopback as a valid address? See `getEffectiveNodeAddresses(TcpDiscoveryNode node, boolean sameHost)`. It has similar parameter  `boolean sameHost`



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1518303999


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    def dns_failure_test(self, ignite_version):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, True)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started()
+
+        assert len(ignites.alive_nodes) == 3, 'All nodes should be alive'
+
+        # Smoke test on full topology.
+        app = IgniteApplicationService(
+            self.test_context,
+            ignite_config._replace(client_mode=True, discovery_spi=from_ignite_cluster(ignites)),
+            startup_timeout_sec=120,
+            java_class_name="org.apache.ignite.internal.ducktest.tests.smoke_test.SimpleApplication")
+
+        app.start()
+        app.stop()
+
+        ignites.stop()
+
+    def __prepare_service(self, ignite_config, num_nodes=1):
+        ignite = IgniteService(
+            self.test_context,
+            ignite_config,
+            startup_timeout_sec=120,
+            num_nodes=num_nodes)
+
+        bootclasspath = list(map(lambda lib: os.path.join(lib, "classes"), ignite.spec._module_libs("ducktests")))
+
+        # Note: Support of impl.prefix property was removed since java 18.
+        ignite.spec.jvm_opts.append("-Dimpl.prefix=BlockingDns")
+        ignite.spec.jvm_opts.append("-Xbootclasspath/a:" + ":".join(bootclasspath))
+
+        return ignite
+
+    @staticmethod
+    def __block_dns(ignite, timeout, fail, nodes=None):

Review Comment:
   `fail` is always `true`. Do we need a test with `false`?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1523230759


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########
@@ -0,0 +1,136 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Module contains DNS service failure test.
+"""
+import os
+import socket
+
+from ducktape.mark import defaults
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.control_utility import ControlUtility
+from ignitetest.services.utils.ignite_aware import IgniteAwareService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils import ignite_versions
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import IgniteVersion, DEV_BRANCH
+from ignitetest.utils import cluster
+
+
+class DnsFailureTest(IgniteTest):
+    """
+    Test DNS service failure.
+    """
+
+    @cluster(num_nodes=4)
+    @ignite_versions(str(DEV_BRANCH))
+    @defaults(fail=[True, False])
+    def dns_failure_test(self, ignite_version, fail):
+        """
+        DNS failure test.
+        """
+        # Replace hosts with IP addresses.
+        for node in self.test_context.cluster.nodes:
+            node.account.externally_routable_ip = socket.gethostbyname(node.account.externally_routable_ip)
+
+        version = IgniteVersion(ignite_version)
+
+        ignite_config = IgniteConfiguration(
+            version=version,
+            data_storage=DataStorageConfiguration(
+                default=DataRegionConfiguration(
+                    persistence_enabled=True
+                )
+            )
+        )
+
+        ignites = self.__prepare_service(ignite_config, 3)
+
+        self.__unblock_dns(ignites)
+
+        # Start nodes one-by-one to reproduce the problem.
+        ignites.start_node(ignites.nodes[0])
+        ignites.await_started([ignites.nodes[0]])
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started([ignites.nodes[0], ignites.nodes[1]])
+        ignites.start_node(ignites.nodes[2])
+        ignites.await_started()
+
+        control_utility = ControlUtility(ignites)
+        control_utility.activate()
+
+        self.__block_dns(ignites, 20000, fail)
+
+        ignites.stop_node(ignites.nodes[1])
+
+        ignites.await_event("Node left topology", 60, from_the_beginning=True,
+                            nodes=[ignites.nodes[0], ignites.nodes[2]])
+
+        assert ignites.alive(ignites.nodes[0]), 'Node 0 should be alive'
+        assert ignites.alive(ignites.nodes[2]), 'Node 2 should be alive'
+
+        self.__unblock_dns(ignites, [ignites.nodes[1]])
+
+        ignites.start_node(ignites.nodes[1])
+        ignites.await_started()
+
+        assert len(ignites.alive_nodes) == 3, 'All nodes should be alive'
+
+        # Smoke test on full topology.

Review Comment:
   Here we check that communication protocol has not problems with DNS (node leave/join tests discovery protocol)



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1523226438


##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -9763,37 +9703,37 @@ public static Collection<InetSocketAddress> toSocketAddresses(ClusterNode node,
     }
 
     /**
-     * Returns tha list of resolved socket addresses. First addresses are resolved by host names,
-     * if this attempt fails then the addresses are resolved by ip addresses.
+     * Returns the list of resolved socket addresses.
      *
      * @param addrs Addresses.
      * @param hostNames Host names.
      * @param port Port.
      * @return Socket addresses for given addresses and host names.
      */
-    public static Collection<InetSocketAddress> toSocketAddresses(Collection<String> addrs,
-        Collection<String> hostNames, int port) {
+    public static Collection<InetSocketAddress> toSocketAddresses(
+        Collection<String> addrs,
+        Collection<String> hostNames, int port
+    ) {
         Set<InetSocketAddress> res = new HashSet<>(addrs.size());
 
-        Iterator<String> hostNamesIt = hostNames.iterator();
+        boolean hasAddr = false;
 
         for (String addr : addrs) {
-            String hostName = hostNamesIt.hasNext() ? hostNamesIt.next() : null;
+            InetSocketAddress inetSockAddr = createResolved(addr, port);
+            res.add(inetSockAddr);
 
-            if (!F.isEmpty(hostName)) {
-                InetSocketAddress inetSockAddr = createResolved(hostName, port);
+            if (!inetSockAddr.isUnresolved() && !inetSockAddr.getAddress().isLoopbackAddress())

Review Comment:
   We don't ignore loopbacks. Here we just check that any resolved external IP address exists and if not, we try to resolve it by provided host names.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1523213858


##########
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##########
@@ -9763,37 +9703,37 @@ public static Collection<InetSocketAddress> toSocketAddresses(ClusterNode node,
     }
 
     /**
-     * Returns tha list of resolved socket addresses. First addresses are resolved by host names,
-     * if this attempt fails then the addresses are resolved by ip addresses.
+     * Returns the list of resolved socket addresses.
      *
      * @param addrs Addresses.
      * @param hostNames Host names.
      * @param port Port.
      * @return Socket addresses for given addresses and host names.
      */
-    public static Collection<InetSocketAddress> toSocketAddresses(Collection<String> addrs,
-        Collection<String> hostNames, int port) {
+    public static Collection<InetSocketAddress> toSocketAddresses(
+        Collection<String> addrs,
+        Collection<String> hostNames, int port

Review Comment:
   Fixed



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-21630 Fix cluster failure on topology change when DNS service is unavailable [ignite]

Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #11260:
URL: https://github.com/apache/ignite/pull/11260#discussion_r1521267793


##########
modules/ducktests/tests/ignitetest/tests/dns_failure_test.py:
##########


Review Comment:
   We can't use the same system properties and the same implementation. This approach only can be used in different JVM and different jar for blocking DNS implementation (to use classes in this jar by bootstrap classloader). So we need some new framework to do that. I think it's not worth it. Moreover we need to test DNS with different hosts and ducktest is the only option here, since locally we will test only localhost addresses. 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org