From 00fd9257158cd9b0aa7450b66762ad9c2c31f08b Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 24 Dec 2024 14:14:49 +0300 Subject: [PATCH] Node.start is refactored [Victoria Shepard' ideas are used, #149] - Save an orignal text of 'does not exist' error - When we reach maximum retry attempt of restarts - We log an error message - We raise exception "Cannot start node after multiple attempts" - A new port number is not tranlating into string - Reorganization TestgresTests.test_port_conflict is updated. --- testgres/node.py | 100 +++++++++++++++++++++++++++---------------- tests/test_simple.py | 2 +- 2 files changed, 63 insertions(+), 39 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index dff47cf6..7121339f 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -790,48 +790,72 @@ def start(self, params=[], wait=True): "-w" if wait else '-W', # --wait or --no-wait "start"] + params # yapf: disable - log_files0 = self._collect_log_files() - assert type(log_files0) == dict # noqa: E721 + def LOCAL__start_node(): + _, _, error = execute_utility(_params, self.utils_log_file, verbose=True) + assert type(error) == str # noqa: E721 + if error and 'does not exist' in error: + raise Exception(error) - nAttempt = 0 - timeout = 1 - while True: - assert nAttempt >= 0 - assert nAttempt < __class__._C_MAX_START_ATEMPTS - nAttempt += 1 + def LOCAL__raise_cannot_start_node(from_exception, msg): + assert isinstance(from_exception, Exception) + assert type(msg) == str # noqa: E721 + files = self._collect_special_files() + raise_from(StartNodeException(msg, files), from_exception) + + def LOCAL__raise_cannot_start_node__std(from_exception): + assert isinstance(from_exception, Exception) + LOCAL__raise_cannot_start_node(from_exception, 'Cannot start node') + + if not self._should_free_port: try: - exit_status, out, error = execute_utility(_params, self.utils_log_file, verbose=True) - if error and 'does not exist' in error: - raise Exception + LOCAL__start_node() except Exception as e: - assert nAttempt > 0 - assert nAttempt <= __class__._C_MAX_START_ATEMPTS - if self._should_free_port and nAttempt < __class__._C_MAX_START_ATEMPTS: + LOCAL__raise_cannot_start_node__std(e) + else: + assert self._should_free_port + assert __class__._C_MAX_START_ATEMPTS > 1 + + log_files0 = self._collect_log_files() + assert type(log_files0) == dict # noqa: E721 + + nAttempt = 0 + timeout = 1 + while True: + assert nAttempt >= 0 + assert nAttempt < __class__._C_MAX_START_ATEMPTS + nAttempt += 1 + try: + LOCAL__start_node() + except Exception as e: + assert nAttempt > 0 + assert nAttempt <= __class__._C_MAX_START_ATEMPTS + if nAttempt == __class__._C_MAX_START_ATEMPTS: + logging.error("Reached maximum retry attempts. Unable to start node.") + LOCAL__raise_cannot_start_node(e, "Cannot start node after multiple attempts") + log_files1 = self._collect_log_files() - if self._detect_port_conflict(log_files0, log_files1): - log_files0 = log_files1 - logging.warning( - "Detected an issue with connecting to port {0}. " - "Trying another port after a {1}-second sleep...".format(self.port, timeout) - ) - time.sleep(timeout) - timeout = min(2 * timeout, 5) - cur_port = self.port - new_port = utils.reserve_port() # can raise - try: - options = {'port': str(new_port)} - self.set_auto_conf(options) - except: # noqa: E722 - utils.release_port(new_port) - raise - self.port = new_port - utils.release_port(cur_port) - continue - - msg = 'Cannot start node' - files = self._collect_special_files() - raise_from(StartNodeException(msg, files), e) - break + if not self._detect_port_conflict(log_files0, log_files1): + LOCAL__raise_cannot_start_node__std(e) + + log_files0 = log_files1 + logging.warning( + "Detected a conflict with using the port {0}. " + "Trying another port after a {1}-second sleep...".format(self.port, timeout) + ) + time.sleep(timeout) + timeout = min(2 * timeout, 5) + cur_port = self.port + new_port = utils.reserve_port() # can raise + try: + options = {'port': str(new_port)} + self.set_auto_conf(options) + except: # noqa: E722 + utils.release_port(new_port) + raise + self.port = new_port + utils.release_port(cur_port) + continue + break self._maybe_start_logger() self.is_started = True return self diff --git a/tests/test_simple.py b/tests/test_simple.py index 93968466..9cf29c64 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1219,7 +1219,7 @@ def test_port_conflict(self): with self.assertRaises(StartNodeException) as ctx: node2.init().start() - self.assertIn("Cannot start node", str(ctx.exception)) + self.assertIn("Cannot start node after multiple attempts", str(ctx.exception)) self.assertEqual(node2.port, node1.port) self.assertTrue(node2._should_free_port)