You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by av...@apache.org on 2021/03/29 12:31:26 UTC
[ignite] branch ignite-ducktape updated: IGNITE-14333 Services
should be stopped only once (#8894)
This is an automated email from the ASF dual-hosted git repository.
av pushed a commit to branch ignite-ducktape
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/ignite-ducktape by this push:
new ab881ba IGNITE-14333 Services should be stopped only once (#8894)
ab881ba is described below
commit ab881bac95c61b0fe66d0e2a12c0b0b4624c11ef
Author: Anton Vinogradov <av...@apache.org>
AuthorDate: Mon Mar 29 15:31:08 2021 +0300
IGNITE-14333 Services should be stopped only once (#8894)
---
.../ducktests/tests/ignitetest/services/ignite.py | 4 --
.../ducktests/tests/ignitetest/services/spark.py | 37 ++++++++------
.../ignitetest/services/utils/background_thread.py | 10 ++--
.../ignitetest/services/utils/ducktests_service.py | 58 ++++++++++++++++++++++
.../ignitetest/services/utils/ignite_aware.py | 50 +++++++------------
.../tests/ignitetest/services/zk/zookeeper.py | 21 +++-----
.../tests/ignitetest/utils/ignite_test.py | 14 ++++--
modules/ducktests/tests/tox.ini | 3 ++
8 files changed, 121 insertions(+), 76 deletions(-)
diff --git a/modules/ducktests/tests/ignitetest/services/ignite.py b/modules/ducktests/tests/ignitetest/services/ignite.py
index cc1f418..f4da888 100644
--- a/modules/ducktests/tests/ignitetest/services/ignite.py
+++ b/modules/ducktests/tests/ignitetest/services/ignite.py
@@ -36,10 +36,6 @@ class IgniteService(IgniteAwareService):
super().__init__(context, config, num_nodes, startup_timeout_sec, shutdown_timeout_sec, modules=modules,
jvm_opts=jvm_opts, full_jvm_opts=full_jvm_opts)
- def clean_node(self, node, **kwargs):
- node.account.kill_java_processes(self.APP_SERVICE_CLASS, clean_shutdown=False, allow_fail=True)
- node.account.ssh("rm -rf -- %s" % self.persistent_root, allow_fail=False)
-
def thread_dump(self, node):
"""
Generate thread dump on node.
diff --git a/modules/ducktests/tests/ignitetest/services/spark.py b/modules/ducktests/tests/ignitetest/services/spark.py
index 1986331..af0c09b 100644
--- a/modules/ducktests/tests/ignitetest/services/spark.py
+++ b/modules/ducktests/tests/ignitetest/services/spark.py
@@ -21,14 +21,14 @@ import os.path
from distutils.version import LooseVersion
from ducktape.cluster.remoteaccount import RemoteCommandError
-from ducktape.services.service import Service
-from ignitetest.services.utils.path import PathAware
+# pylint: disable=abstract-method
+from ignitetest.services.utils.ducktests_service import DucktestsService
from ignitetest.services.utils.log_utils import monitor_log
+from ignitetest.services.utils.path import PathAware
-# pylint: disable=abstract-method
-class SparkService(Service, PathAware):
+class SparkService(DucktestsService, PathAware):
"""
Start a spark node.
"""
@@ -56,6 +56,14 @@ class SparkService(Service, PathAware):
def globals(self):
return self.context.globals
+ @property
+ def config_file(self):
+ return None
+
+ @property
+ def log_config_file(self):
+ return None
+
def start(self, **kwargs):
super().start(**kwargs)
@@ -113,18 +121,21 @@ class SparkService(Service, PathAware):
if len(self.pids(node)) == 0:
raise Exception("No process ids recorded on node %s" % node.account.hostname)
- def stop_node(self, node, **kwargs):
- if node == self.nodes[0]:
- node.account.ssh(os.path.join(self.home_dir, "sbin", "stop-master.sh"))
+ def stop_node(self, node, force_stop=False, **kwargs):
+ if force_stop:
+ node.account.kill_java_processes(self.java_class_name(node), clean_shutdown=False, allow_fail=True)
else:
- node.account.ssh(os.path.join(self.home_dir, "sbin", "stop-slave.sh"))
+ if node == self.nodes[0]:
+ node.account.ssh(os.path.join(self.home_dir, "sbin", "stop-master.sh"))
+ else:
+ node.account.ssh(os.path.join(self.home_dir, "sbin", "stop-slave.sh"))
def clean_node(self, node, **kwargs):
"""
Clean spark persistence files
"""
- node.account.kill_java_processes(self.java_class_name(node),
- clean_shutdown=False, allow_fail=True)
+ super().clean_node(node, **kwargs)
+
node.account.ssh("rm -rf -- %s" % self.persistent_root, allow_fail=False)
def pids(self, node):
@@ -168,9 +179,3 @@ class SparkService(Service, PathAware):
userID=node.account.user,
instance=1,
host=node.account.hostname)
-
- def kill(self):
- """
- Kills the service.
- """
- self.stop()
diff --git a/modules/ducktests/tests/ignitetest/services/utils/background_thread.py b/modules/ducktests/tests/ignitetest/services/utils/background_thread.py
index ea9e124..2dedfd3 100644
--- a/modules/ducktests/tests/ignitetest/services/utils/background_thread.py
+++ b/modules/ducktests/tests/ignitetest/services/utils/background_thread.py
@@ -17,14 +17,14 @@
Background thread service.
"""
-from abc import ABCMeta, abstractmethod
import threading
import traceback
+from abc import ABCMeta, abstractmethod
-from ducktape.services.service import Service
+from ignitetest.services.utils.ducktests_service import DucktestsService
-class BackgroundThreadService(Service, metaclass=ABCMeta):
+class BackgroundThreadService(DucktestsService, metaclass=ABCMeta):
"""BackgroundThreadService allow to start nodes simultaneously using pool of threads."""
def __init__(self, context, num_nodes=None, cluster_spec=None, **kwargs):
@@ -86,7 +86,7 @@ class BackgroundThreadService(Service, metaclass=ABCMeta):
self._propagate_exceptions()
- def stop(self, **kwargs):
+ def stop(self, force_stop=False, **kwargs):
alive_workers = sum(1 for worker in self.worker_threads.values() if worker.is_alive())
if alive_workers > 0:
self.logger.debug(
@@ -94,7 +94,7 @@ class BackgroundThreadService(Service, metaclass=ABCMeta):
self.logger.debug("%s" % str(self.worker_threads))
- super().stop(**kwargs)
+ super().stop(force_stop, **kwargs)
self._propagate_exceptions()
diff --git a/modules/ducktests/tests/ignitetest/services/utils/ducktests_service.py b/modules/ducktests/tests/ignitetest/services/utils/ducktests_service.py
new file mode 100644
index 0000000..3e94554
--- /dev/null
+++ b/modules/ducktests/tests/ignitetest/services/utils/ducktests_service.py
@@ -0,0 +1,58 @@
+# 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.
+
+"""
+Base service for all services.
+"""
+
+from abc import ABCMeta
+
+from ducktape.services.service import Service
+
+
+class DucktestsService(Service, metaclass=ABCMeta):
+ """DucktestsService provides common semantic for all services."""
+
+ def __init__(self, context, num_nodes=None, cluster_spec=None, **kwargs):
+ super().__init__(context, num_nodes, cluster_spec, **kwargs)
+
+ self.stopped = False
+
+ def start(self, **kwargs):
+ self.stopped = False
+
+ super().start(**kwargs)
+
+ # pylint: disable=W0221
+ def stop(self, force_stop=False, **kwargs):
+ if self.stopped:
+ return
+
+ self.stopped = True
+
+ super().stop(force_stop=force_stop, **kwargs)
+
+ # pylint: disable=W0221
+ def stop_node(self, node, force_stop=False, **kwargs):
+ super().stop_node(node, force_stop=force_stop, **kwargs)
+
+ def kill(self):
+ """
+ Kills the service.
+ """
+ self.stop(force_stop=True)
+
+ def clean_node(self, node, **kwargs):
+ assert self.stopped
diff --git a/modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py b/modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
index 2b8e0d2..38449a1 100644
--- a/modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
+++ b/modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
@@ -31,17 +31,15 @@ from ducktape.utils.util import wait_until
from ignitetest.services.utils.background_thread import BackgroundThreadService
from ignitetest.services.utils.concurrent import CountDownLatch, AtomicValue
-from ignitetest.services.utils.path import IgnitePathAware
from ignitetest.services.utils.ignite_spec import resolve_spec
from ignitetest.services.utils.jmx_utils import ignite_jmx_mixin
from ignitetest.services.utils.log_utils import monitor_log
-from ignitetest.utils.enum import constructible
-
-
+from ignitetest.services.utils.path import IgnitePathAware
# pylint: disable=too-many-public-methods
from ignitetest.services.utils.ssl.connector_configuration import ConnectorConfiguration
from ignitetest.services.utils.ssl.ssl_params import get_ssl_params, is_ssl_enabled, IGNITE_SERVER_ALIAS, \
IGNITE_CLIENT_ALIAS
+from ignitetest.utils.enum import constructible
class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABCMeta):
@@ -76,7 +74,6 @@ class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABC
self.init_logs_attribute()
self.disconnected_nodes = []
- self.killed = False
@property
def version(self):
@@ -135,18 +132,19 @@ class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABC
ignite_jmx_mixin(node, self.spec, self.pids(node))
- def stop_async(self, **kwargs):
+ def stop_async(self, force_stop=False, **kwargs):
"""
Stop in async way.
"""
- super().stop(**kwargs)
+ super().stop(force_stop, **kwargs)
- def stop(self, **kwargs):
- if not self.killed:
- self.stop_async(**kwargs)
+ def stop(self, force_stop=False, **kwargs):
+ self.stop_async(force_stop, **kwargs)
+
+ # Making this async on FORCE_STOP to eliminate waiting on killing services on tear down.
+ # Waiting will happen on plain stop() call made by ducktape during same step.
+ if not force_stop:
self.await_stopped()
- else:
- self.logger.debug("Skipping node stop since it already killed.")
def await_stopped(self):
"""
@@ -164,36 +162,22 @@ class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABC
err_msg="Node %s's remote processes failed to stop in %d seconds" %
(str(node.account), self.shutdown_timeout_sec))
- def stop_node(self, node, **kwargs):
+ def stop_node(self, node, force_stop=False, **kwargs):
pids = self.pids(node)
for pid in pids:
- node.account.signal(pid, signal.SIGTERM, allow_fail=False)
-
- def kill(self):
- """
- Kills nodes.
- """
- self.logger.info("Killing IgniteAware(s) ...")
-
- for node in self.nodes:
- pids = self.pids(node)
-
- for pid in pids:
- node.account.signal(pid, signal.SIGKILL, allow_fail=False)
-
- for node in self.nodes:
- wait_until(lambda: not self.alive(node), timeout_sec=self.shutdown_timeout_sec,
- err_msg="Node %s's remote processes failed to be killed in %d seconds" %
- (str(node.account), self.shutdown_timeout_sec))
-
- self.killed = True
+ node.account.signal(pid, signal.SIGKILL if force_stop else signal.SIGTERM, allow_fail=False)
def clean(self, **kwargs):
self.__restore_iptables()
super().clean(**kwargs)
+ def clean_node(self, node, **kwargs):
+ super().clean_node(node, **kwargs)
+
+ node.account.ssh("rm -rf -- %s" % self.persistent_root, allow_fail=False)
+
def init_persistent(self, node):
"""
Init persistent directory.
diff --git a/modules/ducktests/tests/ignitetest/services/zk/zookeeper.py b/modules/ducktests/tests/ignitetest/services/zk/zookeeper.py
index bd6223b..cd76991 100644
--- a/modules/ducktests/tests/ignitetest/services/zk/zookeeper.py
+++ b/modules/ducktests/tests/ignitetest/services/zk/zookeeper.py
@@ -20,8 +20,9 @@ This module contains classes and utilities to start zookeeper cluster for testin
import os.path
from distutils.version import LooseVersion
-from ducktape.services.service import Service
from ducktape.utils.util import wait_until
+
+from ignitetest.services.utils.ducktests_service import DucktestsService
from ignitetest.services.utils.log_utils import monitor_log
from ignitetest.services.utils.path import PathAware
@@ -49,7 +50,7 @@ class ZookeeperSettings:
assert self.tick_time <= self.min_session_timeout // 2, "'tick_time' must be <= 'min_session_timeout' / 2"
-class ZookeeperService(Service, PathAware):
+class ZookeeperService(DucktestsService, PathAware):
"""
Zookeeper service.
"""
@@ -163,21 +164,13 @@ class ZookeeperService(Service, PathAware):
"""
return ','.join([node.account.hostname + ":" + str(2181) for node in self.nodes])
- def stop_node(self, node, **kwargs):
+ def stop_node(self, node, force_stop=False, **kwargs):
idx = self.idx(node)
self.logger.info("Stopping %s node %d on %s" % (type(self).__name__, idx, node.account.hostname))
- node.account.kill_process("zookeeper", allow_fail=False)
+ node.account.kill_process("zookeeper", clean_shutdown=not force_stop, allow_fail=False)
def clean_node(self, node, **kwargs):
+ super().clean_node(node, **kwargs)
+
self.logger.info("Cleaning Zookeeper node %d on %s", self.idx(node), node.account.hostname)
- if self.alive(node):
- self.logger.warn("%s %s was still alive at cleanup time. Killing forcefully..." %
- (self.__class__.__name__, node.account))
- node.account.kill_process("zookeeper", clean_shutdown=False, allow_fail=True)
node.account.ssh(f"rm -rf -- {self.persistent_root}", allow_fail=False)
-
- def kill(self):
- """
- Kills the service.
- """
- self.stop()
diff --git a/modules/ducktests/tests/ignitetest/utils/ignite_test.py b/modules/ducktests/tests/ignitetest/utils/ignite_test.py
index 5c9fcad..6530eab 100644
--- a/modules/ducktests/tests/ignitetest/utils/ignite_test.py
+++ b/modules/ducktests/tests/ignitetest/utils/ignite_test.py
@@ -21,8 +21,10 @@ from time import monotonic
from ducktape.cluster.remoteaccount import RemoteCommandError
from ducktape.tests.test import Test
-
# pylint: disable=W0223
+from ignitetest.services.utils.ducktests_service import DucktestsService
+
+
class IgniteTest(Test):
"""
Basic ignite test.
@@ -42,17 +44,21 @@ class IgniteTest(Test):
"""
return monotonic()
- # pylint: disable=W0212
def tearDown(self):
- self.logger.debug("Killing all services to speed-up the tearing down.")
+ self.logger.debug("Killing all runned services to speed-up the tearing down.")
+ # pylint: disable=W0212
for service in self.test_context.services._services.values():
+ assert isinstance(service, DucktestsService)
+
try:
service.kill()
except RemoteCommandError:
pass # Process may be already self-killed on segmentation.
- self.logger.debug("All services killed.")
+ assert service.stopped
+
+ self.logger.debug("All runned services killed.")
super().tearDown()
diff --git a/modules/ducktests/tests/tox.ini b/modules/ducktests/tests/tox.ini
index ce136b1..5efb136 100644
--- a/modules/ducktests/tests/tox.ini
+++ b/modules/ducktests/tests/tox.ini
@@ -53,6 +53,9 @@ ignore-imports=yes
[FORMAT]
max-line-length=120
+[DESIGN]
+max-parents=10
+
[flake8]
max-line-length=120