You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/08/06 11:27:02 UTC

[GitHub] [ignite] Vladsz83 opened a new pull request #8124: Extend discotest_test

Vladsz83 opened a new pull request #8124:
URL: https://github.com/apache/ignite/pull/8124


   Add:
   - Test of exactly coordinator failure.
   - Test of not-coordinator failure.
   - Test of two nodes failure.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [ignite] ivandasch commented on a change in pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8124:
URL: https://github.com/apache/ignite/pull/8124#discussion_r466351739



##########
File path: modules/ducktests/tests/ignitetest/services/utils/concurrent.py
##########
@@ -0,0 +1,48 @@
+# 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.
+
+"""
+This module contains concurrent utils.
+"""
+
+import threading
+
+
+class CountDownLatch(object):
+    """
+    A count-down latch.
+    """
+    def __init__(self, count=1):
+        self.count = count
+        self.lock = threading.Condition()
+
+    def count_down(self):
+        """
+        Decreases the latch counter.
+        """
+        self.lock.acquire()

Review comment:
       Use with construction instead of direct acquire() and release(). Also, Condition() is not lock, this is condition variable, so preferred name is cond_var




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [ignite] Vladsz83 commented on a change in pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #8124:
URL: https://github.com/apache/ignite/pull/8124#discussion_r466363111



##########
File path: modules/ducktests/tests/ignitetest/tests/discovery_test.py
##########
@@ -120,8 +148,20 @@ def __basic_test__(self, version, with_zk=False):
         data = {'Ignite cluster start time (s)': self.monotonic() - start}
         self.stage("Topology is ready")
 
-        # Node failure detection
-        fail_node, survived_node = self.choose_random_node_to_kill(self.servers)
+        if nodes_to_kill > self.servers.num_nodes - 1:
+            raise Exception("Too many nodes to kill: " + str(nodes_to_kill))
+
+        if with_zk:
+            node_chooser = lambda nodes: random.sample(nodes, nodes_to_kill)
+        elif coordinator:
+            node_chooser = lambda nodes: \
+                next(node for node in nodes if node.discovery_info().node_id == nodes[0].discovery_info().coordinator)
+        else:
+            node_chooser = lambda nodes: \
+                random.sample([n for n in self.servers.nodes if n.discovery_info().node_id
+                               != self.servers.nodes[0].discovery_info().coordinator], nodes_to_kill)

Review comment:
       Fixed '!='
   As for the lambdas, only 2 left. Think it is ok now. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [ignite] Vladsz83 commented on a change in pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #8124:
URL: https://github.com/apache/ignite/pull/8124#discussion_r466361923



##########
File path: modules/ducktests/tests/ignitetest/services/utils/concurrent.py
##########
@@ -0,0 +1,48 @@
+# 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.
+
+"""
+This module contains concurrent utils.
+"""
+
+import threading
+
+
+class CountDownLatch(object):
+    """
+    A count-down latch.
+    """
+    def __init__(self, count=1):
+        self.count = count
+        self.lock = threading.Condition()
+
+    def count_down(self):
+        """
+        Decreases the latch counter.
+        """
+        self.lock.acquire()

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.

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



[GitHub] [ignite] Vladsz83 closed pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
Vladsz83 closed pull request #8124:
URL: https://github.com/apache/ignite/pull/8124


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [ignite] ivandasch commented on a change in pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8124:
URL: https://github.com/apache/ignite/pull/8124#discussion_r466355556



##########
File path: modules/ducktests/tests/ignitetest/tests/discovery_test.py
##########
@@ -120,8 +148,20 @@ def __basic_test__(self, version, with_zk=False):
         data = {'Ignite cluster start time (s)': self.monotonic() - start}
         self.stage("Topology is ready")
 
-        # Node failure detection
-        fail_node, survived_node = self.choose_random_node_to_kill(self.servers)
+        if nodes_to_kill > self.servers.num_nodes - 1:
+            raise Exception("Too many nodes to kill: " + str(nodes_to_kill))
+
+        if with_zk:
+            node_chooser = lambda nodes: random.sample(nodes, nodes_to_kill)
+        elif coordinator:
+            node_chooser = lambda nodes: \
+                next(node for node in nodes if node.discovery_info().node_id == nodes[0].discovery_info().coordinator)
+        else:
+            node_chooser = lambda nodes: \
+                random.sample([n for n in self.servers.nodes if n.discovery_info().node_id
+                               != self.servers.nodes[0].discovery_info().coordinator], nodes_to_kill)

Review comment:
       I suppose, that != should remain on previous 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.

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



[GitHub] [ignite] ivandasch commented on a change in pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8124:
URL: https://github.com/apache/ignite/pull/8124#discussion_r466356953



##########
File path: modules/ducktests/tests/ignitetest/tests/discovery_test.py
##########
@@ -139,24 +179,33 @@ def __basic_test__(self, version, with_zk=False):
 
         data["node_disco_info"] = disco_infos
 
-        self.servers.stop_node(fail_node, clean_shutdown=False)
-
         start = self.monotonic()
-        self.servers.await_event_on_node("Node FAILED", random.choice(survived_node), 60, True)
+
+        ids_to_wait = [node.discovery_info().node_id for node in failed_nodes]
+
+        self.servers.stop_nodes_async(failed_nodes, clean_shutdown=False, delay_ms=delay_ms)
+
+        for failed_id in ids_to_wait:
+            self.stage("Waiting for stopping " + failed_id)
+
+            self.servers.await_event_on_node("Node FAILED: TcpDiscoveryNode \\[id=" + failed_id, survived_node, 30,
+                                             from_the_beginning=True)
 
         data['Failure of node detected in time (s)'] = self.monotonic() - start

Review comment:
       May be change this report string? Write how many nodes failed?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [ignite] ivandasch commented on a change in pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8124:
URL: https://github.com/apache/ignite/pull/8124#discussion_r466355556



##########
File path: modules/ducktests/tests/ignitetest/tests/discovery_test.py
##########
@@ -120,8 +148,20 @@ def __basic_test__(self, version, with_zk=False):
         data = {'Ignite cluster start time (s)': self.monotonic() - start}
         self.stage("Topology is ready")
 
-        # Node failure detection
-        fail_node, survived_node = self.choose_random_node_to_kill(self.servers)
+        if nodes_to_kill > self.servers.num_nodes - 1:
+            raise Exception("Too many nodes to kill: " + str(nodes_to_kill))
+
+        if with_zk:
+            node_chooser = lambda nodes: random.sample(nodes, nodes_to_kill)
+        elif coordinator:
+            node_chooser = lambda nodes: \
+                next(node for node in nodes if node.discovery_info().node_id == nodes[0].discovery_info().coordinator)
+        else:
+            node_chooser = lambda nodes: \
+                random.sample([n for n in self.servers.nodes if n.discovery_info().node_id
+                               != self.servers.nodes[0].discovery_info().coordinator], nodes_to_kill)

Review comment:
       I suppose, that != should remain on previous line
   May be better to  introduce separate functions instead of lambda's ?
   They seem too long.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [ignite] Vladsz83 commented on a change in pull request #8124: Extend discotest_test

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #8124:
URL: https://github.com/apache/ignite/pull/8124#discussion_r466362458



##########
File path: modules/ducktests/tests/ignitetest/tests/discovery_test.py
##########
@@ -139,24 +179,33 @@ def __basic_test__(self, version, with_zk=False):
 
         data["node_disco_info"] = disco_infos
 
-        self.servers.stop_node(fail_node, clean_shutdown=False)
-
         start = self.monotonic()
-        self.servers.await_event_on_node("Node FAILED", random.choice(survived_node), 60, True)
+
+        ids_to_wait = [node.discovery_info().node_id for node in failed_nodes]
+
+        self.servers.stop_nodes_async(failed_nodes, clean_shutdown=False, delay_ms=delay_ms)
+
+        for failed_id in ids_to_wait:
+            self.stage("Waiting for stopping " + failed_id)
+
+            self.servers.await_event_on_node("Node FAILED: TcpDiscoveryNode \\[id=" + failed_id, survived_node, 30,
+                                             from_the_beginning=True)
 
         data['Failure of node detected in time (s)'] = self.monotonic() - start

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.

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