diff --git a/testgres/node.py b/testgres/node.py index 0faf904b..554c226d 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 @@ -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. @@ -158,7 +161,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 +474,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 +747,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. @@ -736,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 @@ -745,27 +789,46 @@ 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: + 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: - 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 + 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 + 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 self._maybe_start_logger() @@ -930,8 +993,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/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 3e8ab8ca..82d1711d 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -308,12 +308,28 @@ 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) 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 34242040..2ab41246 100644 --- a/testgres/operations/os_ops.py +++ b/testgres/operations/os_ops.py @@ -98,9 +98,18 @@ 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() + 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 00c50d93..9f88140c 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()) @@ -350,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) @@ -386,6 +460,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/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). diff --git a/tests/test_local.py b/tests/test_local.py index cb96a3bc..a8a0bde0 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,66 @@ 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) + + 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 c1a91bc6..be1a56bb 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,69 @@ 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_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. diff --git a/tests/test_simple.py b/tests/test_simple.py index fade468c..93968466 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1051,10 +1051,189 @@ 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) + node_port_copy = node.port + self.assertEqual(node.safe_psql("SELECT 1;"), b'1\n') - 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)) + + # 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 + + 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): + 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_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) + + 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) + + # 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: