From b7ed553aa98cb932e18c806f94b29437e1aca8ce Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 29 Dec 2025 21:34:08 +0300 Subject: [PATCH 1/3] PostgresNode::is_started is RO and dynamic now - We do not hold status but always get it from PG instance - PostgresNode::start always tries to start a node --- src/node.py | 302 +++++++++++++++++++--------------- tests/test_testgres_common.py | 18 +- 2 files changed, 180 insertions(+), 140 deletions(-) diff --git a/src/node.py b/src/node.py index 3c5b2b7..5f387d9 100644 --- a/src/node.py +++ b/src/node.py @@ -229,9 +229,7 @@ def __init__(self, # NOTE: for compatibility self.utils_log_name = self.utils_log_file self.pg_log_name = self.pg_log_file - - # Node state - self.is_started = False + return def __enter__(self): return self @@ -343,117 +341,28 @@ def ssh_key(self) -> typing.Optional[str]: assert isinstance(self._os_ops, OsOperations) return self._os_ops.ssh_key + @property + def is_started(self) -> bool: + x = self._get_node_state() + assert type(x) == __class__.tagInternalNodeState + return x.node_status == NodeStatus.Running + @property def pid(self): """ Return postmaster's PID if node is running, else 0. """ - self__data_dir = self.data_dir + x = self._get_node_state() + assert type(x) == __class__.tagInternalNodeState - _params = [ - self._get_bin_path('pg_ctl'), - "-D", self__data_dir, - "status" - ] # yapf: disable - - status_code, out, error = execute_utility2( - self.os_ops, - _params, - self.utils_log_file, - verbose=True, - ignore_errors=True) - - assert type(status_code) == int # noqa: E721 - assert type(out) == str # noqa: E721 - assert type(error) == str # noqa: E721 - - # ----------------- - if status_code == PG_CTL__STATUS__NODE_IS_STOPPED: + if x.pid is None: + assert x.node_status != NodeStatus.Running return 0 - # ----------------- - if status_code == PG_CTL__STATUS__BAD_DATADIR: - return 0 - - # ----------------- - if status_code != PG_CTL__STATUS__OK: - errMsg = "Getting of a node status [data_dir is {0}] failed.".format(self__data_dir) - - raise ExecUtilException( - message=errMsg, - command=_params, - exit_code=status_code, - out=out, - error=error, - ) - - # ----------------- - assert status_code == PG_CTL__STATUS__OK - - if out == "": - __class__._throw_error__pg_ctl_returns_an_empty_string( - _params - ) - - C_PID_PREFIX = "(PID: " - - i = out.find(C_PID_PREFIX) - - if i == -1: - __class__._throw_error__pg_ctl_returns_an_unexpected_string( - out, - _params - ) - - assert i > 0 - assert i < len(out) - assert len(C_PID_PREFIX) <= len(out) - assert i <= len(out) - len(C_PID_PREFIX) - - i += len(C_PID_PREFIX) - start_pid_s = i - - while True: - if i == len(out): - __class__._throw_error__pg_ctl_returns_an_unexpected_string( - out, - _params - ) - - ch = out[i] - - if ch == ")": - break - - if ch.isdigit(): - i += 1 - continue - - __class__._throw_error__pg_ctl_returns_an_unexpected_string( - out, - _params - ) - assert False - - if i == start_pid_s: - __class__._throw_error__pg_ctl_returns_an_unexpected_string( - out, - _params - ) - - # TODO: Let's verify a length of pid string. - - pid = int(out[start_pid_s:i]) - - if pid == 0: - __class__._throw_error__pg_ctl_returns_a_zero_pid( - out, - _params - ) - - assert pid != 0 - return pid + assert x.node_status == NodeStatus.Running + assert type(x.pid) == int + return x.pid @staticmethod def _throw_error__pg_ctl_returns_an_empty_string(_params): @@ -515,7 +424,19 @@ def child_processes(self): """ # get a list of postmaster's children - children = self.os_ops.get_process_children(self.pid) + x = self._get_node_state() + assert type(x) == __class__.tagInternalNodeState + + if x.pid is None: + return [] + + return self._get_child_processes(self.pid) + + def _get_child_processes(self, pid: int) -> typing.List[ProcessProxy]: + assert isinstance(self._os_ops, OsOperations) + + # get a list of postmaster's children + children = self._os_ops.get_process_children(pid) return [ProcessProxy(p) for p in children] @@ -995,28 +916,131 @@ def status(self): Returns: An instance of :class:`.NodeStatus`. """ + x = self._get_node_state() + assert type(x) == __class__.tagInternalNodeState + return x.node_status - try: - _params = [ - self._get_bin_path('pg_ctl'), - "-D", self.data_dir, - "status" - ] # yapf: disable - status_code, out, error = execute_utility2(self.os_ops, _params, self.utils_log_file, verbose=True) - if error and 'does not exist' in error: - return NodeStatus.Uninitialized - elif 'no server running' in out: - return NodeStatus.Stopped - return NodeStatus.Running + class tagInternalNodeState: + node_status: NodeStatus + pid: typing.Optional[int] - except ExecUtilException as e: - # Node is not running - if e.exit_code == 3: - return NodeStatus.Stopped + def __init__( + self, + node_status: NodeStatus, + pid: typing.Optional[int] + ): + assert type(node_status) == NodeStatus + assert pid is None or type(pid) == int + + self.node_status = node_status + self.pid = pid + return + + def _get_node_state(self) -> tagInternalNodeState: + self__data_dir = self.data_dir - # Node has no file dir - elif e.exit_code == 4: - return NodeStatus.Uninitialized + _params = [ + self._get_bin_path('pg_ctl'), + "-D", self__data_dir, + "status" + ] # yapf: disable + + status_code, out, error = execute_utility2( + self.os_ops, + _params, + self.utils_log_file, + verbose=True, + ignore_errors=True) + + assert type(status_code) == int # noqa: E721 + assert type(out) == str # noqa: E721 + assert type(error) == str # noqa: E721 + + # ----------------- + if status_code == PG_CTL__STATUS__NODE_IS_STOPPED: + return __class__.tagInternalNodeState(NodeStatus.Stopped, None) + + # ----------------- + if status_code == PG_CTL__STATUS__BAD_DATADIR: + return __class__.tagInternalNodeState(NodeStatus.Uninitialized, None) + + # ----------------- + if status_code != PG_CTL__STATUS__OK: + errMsg = "Getting of a node status [data_dir is {0}] failed.".format(self__data_dir) + + raise ExecUtilException( + message=errMsg, + command=_params, + exit_code=status_code, + out=out, + error=error, + ) + + if out == "": + __class__._throw_error__pg_ctl_returns_an_empty_string( + _params + ) + + C_PID_PREFIX = "(PID: " + + i = out.find(C_PID_PREFIX) + + if i == -1: + __class__._throw_error__pg_ctl_returns_an_unexpected_string( + out, + _params + ) + + assert i > 0 + assert i < len(out) + assert len(C_PID_PREFIX) <= len(out) + assert i <= len(out) - len(C_PID_PREFIX) + + i += len(C_PID_PREFIX) + start_pid_s = i + + while True: + if i == len(out): + __class__._throw_error__pg_ctl_returns_an_unexpected_string( + out, + _params + ) + + ch = out[i] + + if ch == ")": + break + + if ch.isdigit(): + i += 1 + continue + + __class__._throw_error__pg_ctl_returns_an_unexpected_string( + out, + _params + ) + assert False + + if i == start_pid_s: + __class__._throw_error__pg_ctl_returns_an_unexpected_string( + out, + _params + ) + + # TODO: Let's verify a length of pid string. + + pid = int(out[start_pid_s:i]) + + if pid == 0: + __class__._throw_error__pg_ctl_returns_a_zero_pid( + out, + _params + ) + + assert pid != 0 + + # ----------------- + return __class__.tagInternalNodeState(NodeStatus.Running, pid) def get_control_data(self): """ @@ -1086,9 +1110,6 @@ def start(self, params=[], wait=True, exec_env=None): assert exec_env is None or type(exec_env) == dict # noqa: E721 assert __class__._C_MAX_START_ATEMPTS > 1 - if self.is_started: - return self - if self._port is None: raise InvalidOperationException("Can't start PostgresNode. Port is not defined.") @@ -1167,7 +1188,6 @@ def LOCAL__raise_cannot_start_node__std(from_exception): continue break self._maybe_start_logger() - self.is_started = True return self def stop(self, params=[], wait=True): @@ -1181,9 +1201,6 @@ def stop(self, params=[], wait=True): Returns: This instance of :class:`.PostgresNode`. """ - if not self.is_started: - return self - _params = [ self._get_bin_path("pg_ctl"), "-D", self.data_dir, @@ -1194,7 +1211,6 @@ def stop(self, params=[], wait=True): execute_utility2(self.os_ops, _params, self.utils_log_file) self._maybe_stop_logger() - self.is_started = False return self def kill(self, someone=None): @@ -1205,13 +1221,25 @@ def kill(self, someone=None): someone: A key to the auxiliary process in the auxiliary_pids dictionary. If None, the main PostgreSQL node process will be killed. Defaults to None. """ - if self.is_started: + x = self._get_node_state() + assert type(x) == __class__.tagInternalNodeState + + if x.node_status != NodeStatus.Running: + assert x.pid is None + pass + else: + assert x.node_status == NodeStatus.Running + assert type(x.pid) == int # noqa: E721 sig = signal.SIGKILL if os.name != 'nt' else signal.SIGBREAK if someone is None: - os.kill(self.pid, sig) + self._os_ops.kill(x.pid, sig) else: - os.kill(self.auxiliary_pids[someone][0], sig) - self.is_started = False + childs = self._get_child_processes(x.pid) + for c in childs: + if c.ptype == someone: + self._os_ops.kill(c.process.pid, sig) + continue + return def restart(self, params=[]): """ diff --git a/tests/test_testgres_common.py b/tests/test_testgres_common.py index e308a95..a18e806 100644 --- a/tests/test_testgres_common.py +++ b/tests/test_testgres_common.py @@ -204,9 +204,21 @@ def test_double_start(self, node_svc: PostgresNodeService): assert isinstance(node_svc, PostgresNodeService) with __class__.helper__get_node(node_svc).init().start() as node: - # can't start node more than once - node.start() - assert (node.is_started) + assert node.is_started + + with pytest.raises(expected_exception=StartNodeException) as x: + # can't start node more than once + node.start() + + assert x is not None + assert type(x.value) == StartNodeException + assert type(x.value.message) == str + + assert x.value.message == "Cannot start node" + + assert node.is_started + + return def test_uninitialized_start(self, node_svc: PostgresNodeService): assert isinstance(node_svc, PostgresNodeService) From 64b26f223eeac9bff4517461a2aaaa20d54e823c Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 29 Dec 2025 23:26:59 +0300 Subject: [PATCH 2/3] PostgresNode::child_processes is corrected --- src/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node.py b/src/node.py index 5f387d9..8be3da8 100644 --- a/src/node.py +++ b/src/node.py @@ -430,7 +430,7 @@ def child_processes(self): if x.pid is None: return [] - return self._get_child_processes(self.pid) + return self._get_child_processes(x.pid) def _get_child_processes(self, pid: int) -> typing.List[ProcessProxy]: assert isinstance(self._os_ops, OsOperations) From 8ac8c81ef82ef2e309048e84b7690b069feaa2c1 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 30 Dec 2025 15:41:40 +0300 Subject: [PATCH 3/3] flake8 --- src/node.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/node.py b/src/node.py index 8be3da8..4af3411 100644 --- a/src/node.py +++ b/src/node.py @@ -344,7 +344,7 @@ def ssh_key(self) -> typing.Optional[str]: @property def is_started(self) -> bool: x = self._get_node_state() - assert type(x) == __class__.tagInternalNodeState + assert type(x) == __class__.tagInternalNodeState # noqa: E721 return x.node_status == NodeStatus.Running @property @@ -354,14 +354,14 @@ def pid(self): """ x = self._get_node_state() - assert type(x) == __class__.tagInternalNodeState + assert type(x) == __class__.tagInternalNodeState # noqa: E721 if x.pid is None: assert x.node_status != NodeStatus.Running return 0 assert x.node_status == NodeStatus.Running - assert type(x.pid) == int + assert type(x.pid) == int # noqa: E721 return x.pid @staticmethod @@ -425,7 +425,7 @@ def child_processes(self): # get a list of postmaster's children x = self._get_node_state() - assert type(x) == __class__.tagInternalNodeState + assert type(x) == __class__.tagInternalNodeState # noqa: E721 if x.pid is None: return [] @@ -917,7 +917,7 @@ def status(self): An instance of :class:`.NodeStatus`. """ x = self._get_node_state() - assert type(x) == __class__.tagInternalNodeState + assert type(x) == __class__.tagInternalNodeState # noqa: E721 return x.node_status class tagInternalNodeState: @@ -929,8 +929,8 @@ def __init__( node_status: NodeStatus, pid: typing.Optional[int] ): - assert type(node_status) == NodeStatus - assert pid is None or type(pid) == int + assert type(node_status) == NodeStatus # noqa: E721 + assert pid is None or type(pid) == int # noqa: E721 self.node_status = node_status self.pid = pid @@ -1222,7 +1222,7 @@ def kill(self, someone=None): If None, the main PostgreSQL node process will be killed. Defaults to None. """ x = self._get_node_state() - assert type(x) == __class__.tagInternalNodeState + assert type(x) == __class__.tagInternalNodeState # noqa: E721 if x.node_status != NodeStatus.Running: assert x.pid is None