Content-Length: 35913 | pFad | http://github.com/postgrespro/testgres/pull/165.patch

thub.com From e4c2e07b3ff0c4aa16188a60bb53f108e2d5c0ca Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 14:22:11 +0300 Subject: [PATCH 1/6] reserve_port and release_port are "pointers" to functions for a port numbers management. This need to replace port numbers management in unit tests. --- testgres/utils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/testgres/utils.py b/testgres/utils.py index a4ee7877..4bd232b1 100644 --- a/testgres/utils.py +++ b/testgres/utils.py @@ -34,7 +34,7 @@ def __init__(self, version: str) -> None: super().__init__(version) -def reserve_port(): +def internal__reserve_port(): """ Generate a new port and add it to 'bound_ports'. """ @@ -45,7 +45,7 @@ def reserve_port(): return port -def release_port(port): +def internal__release_port(port): """ Free port provided by reserve_port(). """ @@ -53,6 +53,10 @@ def release_port(port): bound_ports.discard(port) +reserve_port = internal__reserve_port +release_port = internal__release_port + + def execute_utility(args, logfile=None, verbose=False): """ Execute utility (pg_ctl, pg_dump etc). From 85d2aa3917f8210dce06531b563ec34c42e6b544 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 14:38:39 +0300 Subject: [PATCH 2/6] TestgresTests::test_the_same_port is corrected --- tests/test_simple.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_simple.py b/tests/test_simple.py index fade468c..6b04f8bd 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1051,10 +1051,18 @@ def test_parse_pg_version(self): def test_the_same_port(self): with get_new_node() as node: node.init().start() + self.assertTrue(node._should_free_port) + self.assertEqual(type(node.port), int) - with get_new_node() as node2: - node2.port = node.port - node2.init().start() + with get_new_node(port=node.port) as node2: + self.assertEqual(type(node2.port), int) + self.assertEqual(node2.port, node.port) + self.assertFalse(node2._should_free_port) + + with self.assertRaises(StartNodeException) as ctx: + node2.init().start() + + self.assertIn("Cannot start node", str(ctx.exception)) def test_simple_with_bin_dir(self): with get_new_node() as node: From 88371d1a610c62591d3b3ec2d5e88b7a2437ffb9 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 14:46:23 +0300 Subject: [PATCH 3/6] OsOperations::read_binary(self, filename, start_pos) is added It is a specialized function to read binary data from files. --- testgres/operations/local_ops.py | 11 ++++++++ testgres/operations/os_ops.py | 6 +++++ testgres/operations/remote_ops.py | 19 +++++++++++++ tests/test_local.py | 45 +++++++++++++++++++++++++++++++ tests/test_remote.py | 43 +++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 3e8ab8ca..65dc0965 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -308,6 +308,17 @@ def readlines(self, filename, num_lines=0, binary=False, encoding=None): buffers * max(2, int(num_lines / max(cur_lines, 1))) ) # Adjust buffer size + def read_binary(self, filename, start_pos): + assert type(filename) == str # noqa: E721 + assert type(start_pos) == int # noqa: E721 + assert start_pos >= 0 + + with open(filename, 'rb') as file: # open in a binary mode + file.seek(start_pos, os.SEEK_SET) + r = file.read() + assert type(r) == bytes # noqa: E721 + return r + def isfile(self, remote_file): return os.path.isfile(remote_file) diff --git a/testgres/operations/os_ops.py b/testgres/operations/os_ops.py index 34242040..82d44a4e 100644 --- a/testgres/operations/os_ops.py +++ b/testgres/operations/os_ops.py @@ -98,6 +98,12 @@ def read(self, filename, encoding, binary): def readlines(self, filename): raise NotImplementedError() + def read_binary(self, filename, start_pos): + assert type(filename) == str # noqa: E721 + assert type(start_pos) == int # noqa: E721 + assert start_pos >= 0 + raise NotImplementedError() + def isfile(self, remote_file): raise NotImplementedError() diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 00c50d93..9d72731d 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -340,6 +340,16 @@ def readlines(self, filename, num_lines=0, binary=False, encoding=None): return lines + def read_binary(self, filename, start_pos): + assert type(filename) == str # noqa: E721 + assert type(start_pos) == int # noqa: E721 + assert start_pos >= 0 + + cmd = "tail -c +{} {}".format(start_pos + 1, __class__._escape_path(filename)) + r = self.exec_command(cmd) + assert type(r) == bytes # noqa: E721 + return r + def isfile(self, remote_file): stdout = self.exec_command("test -f {}; echo $?".format(remote_file)) result = int(stdout.strip()) @@ -386,6 +396,15 @@ def db_connect(self, dbname, user, password=None, host="localhost", port=5432): ) return conn + def _escape_path(path): + assert type(path) == str # noqa: E721 + assert path != "" # Ok? + + r = "'" + r += path + r += "'" + return r + def normalize_error(error): if isinstance(error, bytes): diff --git a/tests/test_local.py b/tests/test_local.py index cb96a3bc..812b4030 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -1,4 +1,7 @@ +import os + import pytest +import re from testgres import ExecUtilException from testgres import LocalOperations @@ -52,3 +55,45 @@ def test_exec_command_failure__expect_error(self): assert error == b'/bin/sh: 1: nonexistent_command: not found\n' assert exit_status == 127 assert result == b'' + + def test_read_binary__spec(self): + """ + Test LocalOperations::read_binary. + """ + filename = __file__ # current file + + with open(filename, 'rb') as file: # open in a binary mode + response0 = file.read() + + assert type(response0) == bytes # noqa: E721 + + response1 = self.operations.read_binary(filename, 0) + assert type(response1) == bytes # noqa: E721 + assert response1 == response0 + + response2 = self.operations.read_binary(filename, 1) + assert type(response2) == bytes # noqa: E721 + assert len(response2) < len(response1) + assert len(response2) + 1 == len(response1) + assert response2 == response1[1:] + + response3 = self.operations.read_binary(filename, len(response1)) + assert type(response3) == bytes # noqa: E721 + assert len(response3) == 0 + + response4 = self.operations.read_binary(filename, len(response2)) + assert type(response4) == bytes # noqa: E721 + assert len(response4) == 1 + assert response4[0] == response1[len(response1) - 1] + + response5 = self.operations.read_binary(filename, len(response1) + 1) + assert type(response5) == bytes # noqa: E721 + assert len(response5) == 0 + + def test_read_binary__spec__unk_file(self): + """ + Test LocalOperations::read_binary with unknown file. + """ + + with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): + self.operations.read_binary("/dummy", 0) diff --git a/tests/test_remote.py b/tests/test_remote.py index c1a91bc6..c775f72d 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -1,6 +1,7 @@ import os import pytest +import re from testgres import ExecUtilException from testgres import RemoteOperations @@ -181,6 +182,48 @@ def test_read_binary_file(self): assert isinstance(response, bytes) + def test_read_binary__spec(self): + """ + Test RemoteOperations::read_binary. + """ + filename = __file__ # currnt file + + with open(filename, 'rb') as file: # open in a binary mode + response0 = file.read() + + assert type(response0) == bytes # noqa: E721 + + response1 = self.operations.read_binary(filename, 0) + assert type(response1) == bytes # noqa: E721 + assert response1 == response0 + + response2 = self.operations.read_binary(filename, 1) + assert type(response2) == bytes # noqa: E721 + assert len(response2) < len(response1) + assert len(response2) + 1 == len(response1) + assert response2 == response1[1:] + + response3 = self.operations.read_binary(filename, len(response1)) + assert type(response3) == bytes # noqa: E721 + assert len(response3) == 0 + + response4 = self.operations.read_binary(filename, len(response2)) + assert type(response4) == bytes # noqa: E721 + assert len(response4) == 1 + assert response4[0] == response1[len(response1) - 1] + + response5 = self.operations.read_binary(filename, len(response1) + 1) + assert type(response5) == bytes # noqa: E721 + assert len(response5) == 0 + + def test_read_binary__spec__unk_file(self): + """ + Test RemoteOperations::read_binary with unknown file. + """ + + with pytest.raises(ExecUtilException, match=re.escape("tail: cannot open '/dummy' for reading: No such file or directory")): + self.operations.read_binary("/dummy", 0) + def test_touch(self): """ Test touch for creating a new file or updating access and modification times of an existing file. From 4fe189445b0351692b57fc908d366e4dd82cb9e6 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 15:16:48 +0300 Subject: [PATCH 4/6] OsOperations::get_file_size(self, filename) is added It is a function to get a size of file. --- testgres/operations/local_ops.py | 5 +++ testgres/operations/os_ops.py | 3 ++ testgres/operations/remote_ops.py | 64 +++++++++++++++++++++++++++++++ tests/test_local.py | 21 ++++++++++ tests/test_remote.py | 21 ++++++++++ 5 files changed, 114 insertions(+) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 65dc0965..82d1711d 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -325,6 +325,11 @@ def isfile(self, remote_file): def isdir(self, dirname): return os.path.isdir(dirname) + def get_file_size(self, filename): + assert filename is not None + assert type(filename) == str # noqa: E721 + return os.path.getsize(filename) + def remove_file(self, filename): return os.remove(filename) diff --git a/testgres/operations/os_ops.py b/testgres/operations/os_ops.py index 82d44a4e..2ab41246 100644 --- a/testgres/operations/os_ops.py +++ b/testgres/operations/os_ops.py @@ -107,6 +107,9 @@ def read_binary(self, filename, start_pos): def isfile(self, remote_file): raise NotImplementedError() + def get_file_size(self, filename): + raise NotImplementedError() + # Processes control def kill(self, pid, signal): # Kill the process diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 9d72731d..9f88140c 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -360,6 +360,70 @@ def isdir(self, dirname): response = self.exec_command(cmd) return response.strip() == b"True" + def get_file_size(self, filename): + C_ERR_SRC = "RemoteOpertions::get_file_size" + + assert filename is not None + assert type(filename) == str # noqa: E721 + cmd = "du -b " + __class__._escape_path(filename) + + s = self.exec_command(cmd, encoding=get_default_encoding()) + assert type(s) == str # noqa: E721 + + if len(s) == 0: + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned an empty string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#001", + filename + ) + ) + + i = 0 + + while i < len(s) and s[i].isdigit(): + assert s[i] >= '0' + assert s[i] <= '9' + i += 1 + + if i == 0: + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned a bad formatted string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#002", + filename + ) + ) + + if i == len(s): + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned a bad formatted string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#003", + filename + ) + ) + + if not s[i].isspace(): + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned a bad formatted string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#004", + filename + ) + ) + + r = 0 + + for i2 in range(0, i): + ch = s[i2] + assert ch >= '0' + assert ch <= '9' + # Here is needed to check overflow or that it is a human-valid result? + r = (r * 10) + ord(ch) - ord('0') + + return r + def remove_file(self, filename): cmd = "rm {}".format(filename) return self.exec_command(cmd) diff --git a/tests/test_local.py b/tests/test_local.py index 812b4030..a8a0bde0 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -97,3 +97,24 @@ def test_read_binary__spec__unk_file(self): with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): self.operations.read_binary("/dummy", 0) + + def test_get_file_size(self): + """ + Test LocalOperations::get_file_size. + """ + filename = __file__ # current file + + sz0 = os.path.getsize(filename) + assert type(sz0) == int # noqa: E721 + + sz1 = self.operations.get_file_size(filename) + assert type(sz1) == int # noqa: E721 + assert sz1 == sz0 + + def test_get_file_size__unk_file(self): + """ + Test LocalOperations::get_file_size. + """ + + with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): + self.operations.get_file_size("/dummy") diff --git a/tests/test_remote.py b/tests/test_remote.py index c775f72d..be1a56bb 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -224,6 +224,27 @@ def test_read_binary__spec__unk_file(self): with pytest.raises(ExecUtilException, match=re.escape("tail: cannot open '/dummy' for reading: No such file or directory")): self.operations.read_binary("/dummy", 0) + def test_get_file_size(self): + """ + Test LocalOperations::get_file_size. + """ + filename = __file__ # current file + + sz0 = os.path.getsize(filename) + assert type(sz0) == int # noqa: E721 + + sz1 = self.operations.get_file_size(filename) + assert type(sz1) == int # noqa: E721 + assert sz1 == sz0 + + def test_get_file_size__unk_file(self): + """ + Test LocalOperations::get_file_size. + """ + + with pytest.raises(ExecUtilException, match=re.escape("du: cannot access '/dummy': No such file or directory")): + self.operations.get_file_size("/dummy") + def test_touch(self): """ Test touch for creating a new file or updating access and modification times of an existing file. From 28ac4252e5761394b74bb782e29b986f1ffd31d9 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 15:28:39 +0300 Subject: [PATCH 5/6] Port numbers management is improved (#164) - We don't release a port number that was defined by client - We only check log files to detect port number conflicts - We use slightly smarter log file checking A test is added. --- testgres/node.py | 89 +++++++++++++++++++++++++------- tests/test_simple.py | 117 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 17 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 0faf904b..7f5aa648 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -83,13 +83,13 @@ from .standby import First +from . import utils + from .utils import \ PgVer, \ eprint, \ get_bin_path, \ get_pg_version, \ - reserve_port, \ - release_port, \ execute_utility, \ options_string, \ clean_on_error @@ -158,7 +158,7 @@ def __init__(self, name=None, base_dir=None, port=None, conn_params: ConnectionP self.os_ops = LocalOperations(conn_params) self.host = self.os_ops.host - self.port = port or reserve_port() + self.port = port or utils.reserve_port() self.ssh_key = self.os_ops.ssh_key @@ -471,6 +471,28 @@ def _collect_special_files(self): return result + def _collect_log_files(self): + # dictionary of log files + size in bytes + + files = [ + self.pg_log_file + ] # yapf: disable + + result = {} + + for f in files: + # skip missing files + if not self.os_ops.path_exists(f): + continue + + file_size = self.os_ops.get_file_size(f) + assert type(file_size) == int # noqa: E721 + assert file_size >= 0 + + result[f] = file_size + + return result + def init(self, initdb_params=None, cached=True, **kwargs): """ Perform initdb for this node. @@ -722,6 +744,22 @@ def slow_start(self, replica=False, dbname='template1', username=None, max_attem OperationalError}, max_attempts=max_attempts) + def _detect_port_conflict(self, log_files0, log_files1): + assert type(log_files0) == dict # noqa: E721 + assert type(log_files1) == dict # noqa: E721 + + for file in log_files1.keys(): + read_pos = 0 + + if file in log_files0.keys(): + read_pos = log_files0[file] # the previous size + + file_content = self.os_ops.read_binary(file, read_pos) + file_content_s = file_content.decode() + if 'Is another postmaster already running on port' in file_content_s: + return True + return False + def start(self, params=[], wait=True): """ Starts the PostgreSQL node using pg_ctl if node has not been started. @@ -745,27 +783,42 @@ def start(self, params=[], wait=True): "-w" if wait else '-W', # --wait or --no-wait "start"] + params # yapf: disable - startup_retries = 5 + log_files0 = self._collect_log_files() + assert type(log_files0) == dict # noqa: E721 + + nAttempt = 0 + timeout = 1 while True: + nAttempt += 1 try: exit_status, out, error = execute_utility(_params, self.utils_log_file, verbose=True) if error and 'does not exist' in error: raise Exception except Exception as e: - files = self._collect_special_files() - if any(len(file) > 1 and 'Is another postmaster already ' - 'running on port' in file[1].decode() for - file in files): - logging.warning("Detected an issue with connecting to port {0}. " - "Trying another port after a 5-second sleep...".format(self.port)) - self.port = reserve_port() - options = {'port': str(self.port)} - self.set_auto_conf(options) - startup_retries -= 1 - time.sleep(5) - continue + if self._should_free_port and nAttempt < 5: + 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() # throw + 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 self._maybe_start_logger() @@ -930,8 +983,10 @@ def free_port(self): """ if self._should_free_port: + port = self.port self._should_free_port = False - release_port(self.port) + self.port = None + utils.release_port(port) def cleanup(self, max_attempts=3, full=False): """ diff --git a/tests/test_simple.py b/tests/test_simple.py index 6b04f8bd..0a09135c 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1064,6 +1064,123 @@ def test_the_same_port(self): self.assertIn("Cannot start node", str(ctx.exception)) + class tagPortManagerProxy: + sm_prev_testgres_reserve_port = None + sm_prev_testgres_release_port = None + + sm_DummyPortNumber = None + sm_DummyPortMaxUsage = None + + sm_DummyPortCurrentUsage = None + sm_DummyPortTotalUsage = None + + def __init__(self, dummyPortNumber, dummyPortMaxUsage): + assert type(dummyPortNumber) == int # noqa: E721 + assert type(dummyPortMaxUsage) == int # noqa: E721 + assert dummyPortNumber >= 0 + assert dummyPortMaxUsage >= 0 + + assert __class__.sm_prev_testgres_reserve_port is None + assert __class__.sm_prev_testgres_release_port is None + assert testgres.utils.reserve_port == testgres.utils.internal__reserve_port + assert testgres.utils.release_port == testgres.utils.internal__release_port + + __class__.sm_prev_testgres_reserve_port = testgres.utils.reserve_port + __class__.sm_prev_testgres_release_port = testgres.utils.release_port + + testgres.utils.reserve_port = __class__._proxy__reserve_port + testgres.utils.release_port = __class__._proxy__release_port + + assert testgres.utils.reserve_port == __class__._proxy__reserve_port + assert testgres.utils.release_port == __class__._proxy__release_port + + __class__.sm_DummyPortNumber = dummyPortNumber + __class__.sm_DummyPortMaxUsage = dummyPortMaxUsage + + __class__.sm_DummyPortCurrentUsage = 0 + __class__.sm_DummyPortTotalUsage = 0 + + def __enter__(self): + return self + + def __exit__(self, type, value, traceback): + assert __class__.sm_DummyPortCurrentUsage == 0 + + assert __class__.sm_prev_testgres_reserve_port is not None + assert __class__.sm_prev_testgres_release_port is not None + + assert testgres.utils.reserve_port == __class__._proxy__reserve_port + assert testgres.utils.release_port == __class__._proxy__release_port + + testgres.utils.reserve_port = __class__.sm_prev_testgres_reserve_port + testgres.utils.release_port = __class__.sm_prev_testgres_release_port + + __class__.sm_prev_testgres_reserve_port = None + __class__.sm_prev_testgres_release_port = None + + def _proxy__reserve_port(): + assert type(__class__.sm_DummyPortMaxUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortTotalUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortCurrentUsage) == int # noqa: E721 + assert __class__.sm_DummyPortTotalUsage >= 0 + assert __class__.sm_DummyPortCurrentUsage >= 0 + + assert __class__.sm_DummyPortTotalUsage <= __class__.sm_DummyPortMaxUsage + assert __class__.sm_DummyPortCurrentUsage <= __class__.sm_DummyPortTotalUsage + + assert __class__.sm_prev_testgres_reserve_port is not None + + if __class__.sm_DummyPortTotalUsage == __class__.sm_DummyPortMaxUsage: + return __class__.sm_prev_testgres_reserve_port() + + __class__.sm_DummyPortTotalUsage += 1 + __class__.sm_DummyPortCurrentUsage += 1 + return __class__.sm_DummyPortNumber + + def _proxy__release_port(dummyPortNumber): + assert type(dummyPortNumber) == int # noqa: E721 + + assert type(__class__.sm_DummyPortMaxUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortTotalUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortCurrentUsage) == int # noqa: E721 + assert __class__.sm_DummyPortTotalUsage >= 0 + assert __class__.sm_DummyPortCurrentUsage >= 0 + + assert __class__.sm_DummyPortTotalUsage <= __class__.sm_DummyPortMaxUsage + assert __class__.sm_DummyPortCurrentUsage <= __class__.sm_DummyPortTotalUsage + + assert __class__.sm_prev_testgres_release_port is not None + + if __class__.sm_DummyPortCurrentUsage > 0 and dummyPortNumber == __class__.sm_DummyPortNumber: + assert __class__.sm_DummyPortTotalUsage > 0 + __class__.sm_DummyPortCurrentUsage -= 1 + return + + return __class__.sm_prev_testgres_release_port(dummyPortNumber) + + def test_port_rereserve_during_node_start(self): + C_COUNT_OF_BAD_PORT_USAGE = 3 + + with get_new_node() as node1: + node1.init().start() + self.assertTrue(node1._should_free_port) + self.assertEqual(type(node1.port), int) # noqa: E721 + node1.safe_psql("SELECT 1;") + + with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): + assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port + with get_new_node() as node2: + self.assertTrue(node2._should_free_port) + self.assertEqual(node2.port, node1.port) + + node2.init().start() + + self.assertNotEqual(node2.port, node1.port) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 0) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortTotalUsage, C_COUNT_OF_BAD_PORT_USAGE) + + node2.safe_psql("SELECT 1;") + def test_simple_with_bin_dir(self): with get_new_node() as node: node.init().start() From 663612cb870fbeade43db3a0c6f771127fb650a2 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sat, 14 Dec 2024 11:07:35 +0300 Subject: [PATCH 6/6] PostgresNode._C_MAX_START_ATEMPTS=5 is added (+ 1 new test) Also - TestgresTests.test_the_same_port is updated - TestgresTests.test_port_rereserve_during_node_start is updated - TestgresTests.test_port_conflict is added --- testgres/node.py | 14 +++++++++-- tests/test_simple.py | 58 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 7f5aa648..554c226d 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -128,6 +128,9 @@ def __repr__(self): class PostgresNode(object): + # a max number of node start attempts + _C_MAX_START_ATEMPTS = 5 + def __init__(self, name=None, base_dir=None, port=None, conn_params: ConnectionParams = ConnectionParams(), bin_dir=None, prefix=None): """ PostgresNode constructor. @@ -774,6 +777,9 @@ def start(self, params=[], wait=True): Returns: This instance of :class:`.PostgresNode`. """ + + assert __class__._C_MAX_START_ATEMPTS > 1 + if self.is_started: return self @@ -789,13 +795,17 @@ def start(self, params=[], wait=True): nAttempt = 0 timeout = 1 while True: + assert nAttempt >= 0 + assert nAttempt < __class__._C_MAX_START_ATEMPTS nAttempt += 1 try: exit_status, out, error = execute_utility(_params, self.utils_log_file, verbose=True) if error and 'does not exist' in error: raise Exception except Exception as e: - if self._should_free_port and nAttempt < 5: + assert nAttempt > 0 + assert nAttempt <= __class__._C_MAX_START_ATEMPTS + if self._should_free_port and nAttempt < __class__._C_MAX_START_ATEMPTS: log_files1 = self._collect_log_files() if self._detect_port_conflict(log_files0, log_files1): log_files0 = log_files1 @@ -806,7 +816,7 @@ def start(self, params=[], wait=True): time.sleep(timeout) timeout = min(2 * timeout, 5) cur_port = self.port - new_port = utils.reserve_port() # throw + new_port = utils.reserve_port() # can raise try: options = {'port': str(new_port)} self.set_auto_conf(options) diff --git a/tests/test_simple.py b/tests/test_simple.py index 0a09135c..93968466 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1053,6 +1053,8 @@ def test_the_same_port(self): node.init().start() self.assertTrue(node._should_free_port) self.assertEqual(type(node.port), int) + node_port_copy = node.port + self.assertEqual(node.safe_psql("SELECT 1;"), b'1\n') with get_new_node(port=node.port) as node2: self.assertEqual(type(node2.port), int) @@ -1064,6 +1066,11 @@ def test_the_same_port(self): self.assertIn("Cannot start node", str(ctx.exception)) + # node is still working + self.assertEqual(node.port, node_port_copy) + self.assertTrue(node._should_free_port) + self.assertEqual(node.safe_psql("SELECT 3;"), b'3\n') + class tagPortManagerProxy: sm_prev_testgres_reserve_port = None sm_prev_testgres_release_port = None @@ -1159,13 +1166,16 @@ def _proxy__release_port(dummyPortNumber): return __class__.sm_prev_testgres_release_port(dummyPortNumber) def test_port_rereserve_during_node_start(self): + assert testgres.PostgresNode._C_MAX_START_ATEMPTS == 5 + C_COUNT_OF_BAD_PORT_USAGE = 3 with get_new_node() as node1: node1.init().start() self.assertTrue(node1._should_free_port) self.assertEqual(type(node1.port), int) # noqa: E721 - node1.safe_psql("SELECT 1;") + node1_port_copy = node1.port + self.assertEqual(node1.safe_psql("SELECT 1;"), b'1\n') with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port @@ -1176,10 +1186,54 @@ def test_port_rereserve_during_node_start(self): node2.init().start() self.assertNotEqual(node2.port, node1.port) + self.assertTrue(node2._should_free_port) self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 0) self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortTotalUsage, C_COUNT_OF_BAD_PORT_USAGE) + self.assertTrue(node2.is_started) + + self.assertEqual(node2.safe_psql("SELECT 2;"), b'2\n') + + # node1 is still working + self.assertEqual(node1.port, node1_port_copy) + self.assertTrue(node1._should_free_port) + self.assertEqual(node1.safe_psql("SELECT 3;"), b'3\n') + + def test_port_conflict(self): + assert testgres.PostgresNode._C_MAX_START_ATEMPTS > 1 + + C_COUNT_OF_BAD_PORT_USAGE = testgres.PostgresNode._C_MAX_START_ATEMPTS + + with get_new_node() as node1: + node1.init().start() + self.assertTrue(node1._should_free_port) + self.assertEqual(type(node1.port), int) # noqa: E721 + node1_port_copy = node1.port + self.assertEqual(node1.safe_psql("SELECT 1;"), b'1\n') + + with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): + assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port + with get_new_node() as node2: + self.assertTrue(node2._should_free_port) + self.assertEqual(node2.port, node1.port) + + with self.assertRaises(StartNodeException) as ctx: + node2.init().start() + + self.assertIn("Cannot start node", str(ctx.exception)) + + self.assertEqual(node2.port, node1.port) + self.assertTrue(node2._should_free_port) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 1) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortTotalUsage, C_COUNT_OF_BAD_PORT_USAGE) + self.assertFalse(node2.is_started) + + # node2 must release our dummyPort (node1.port) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 0) - node2.safe_psql("SELECT 1;") + # node1 is still working + self.assertEqual(node1.port, node1_port_copy) + self.assertTrue(node1._should_free_port) + self.assertEqual(node1.safe_psql("SELECT 3;"), b'3\n') def test_simple_with_bin_dir(self): with get_new_node() as node:

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!