diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f29050a7..e2ad5765 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -154,7 +154,7 @@ jobs: with: name: behave-${{ matrix.os }}-${{ matrix.dcs }}-${{ matrix.python-version }}-logs path: | - features/output/*_failed/*postgres?.* + features/output/*_failed/*postgres-?.* features/output/*.log if-no-files-found: error retention-days: 5 diff --git a/features/permanent_slots.feature b/features/permanent_slots.feature index e89ffac1..501c48e2 100644 --- a/features/permanent_slots.feature +++ b/features/permanent_slots.feature @@ -81,4 +81,6 @@ Feature: permanent slots When I start postgres-0 Then postgres-0 role is the replica after 20 seconds And physical replication slot named postgres_1 on postgres-0 has no xmin value after 10 seconds - And physical replication slot named postgres_2 on postgres-0 has no xmin value after 10 seconds + # postgres_2 and postgres_3 slots are retained, but postgres_2 will still have xmin value :( + And postgres-0 has a physical replication slot named postgres_2 after 10 seconds + And postgres-0 has a physical replication slot named postgres_3 after 10 seconds diff --git a/features/steps/slots.py b/features/steps/slots.py index aca00866..befd1007 100644 --- a/features/steps/slots.py +++ b/features/steps/slots.py @@ -99,8 +99,8 @@ def has_physical_replication_slot(context, pg_name, slot_name, time_limit): def physical_slot_no_xmin(context, pg_name, slot_name, time_limit): time_limit *= context.timeout_multiplier max_time = time.time() + int(time_limit) - query = "SELECT xmin FROM pg_catalog.pg_replication_slots WHERE slot_type = 'physical'" - f" AND slot_name = '{slot_name}'" + query = "SELECT xmin FROM pg_catalog.pg_replication_slots WHERE slot_type = 'physical'" +\ + f" AND slot_name = '{slot_name}'" exists = False while time.time() < max_time: try: diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index 19d3c1aa..7414c1eb 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -1841,10 +1841,6 @@ class AbstractDCS(abc.ABC): """ timestamp = time.time() - # DCS is a source of truth, therefore we take missing values from there - self._last_retain_slots.update({name: timestamp for name in self._last_status['retain_slots'] - if (not slots or name not in slots) and name not in self._last_retain_slots}) - if slots: # if slots is not empty it implies we are running v11+ members: Set[str] = set() found_self = False @@ -1929,6 +1925,23 @@ class AbstractDCS(abc.ABC): :returns: ``True`` if key has been created successfully. """ + def acquire_leader_lock(self) -> bool: + """Attempt to acquire leader lock. + + .. note:: + This method wraps :meth:`~AbstractDCS.attempt_to_acquire_leader`: and is + used to reset retention time of physical replication slots that representing + members of the cluster when current node is to be promoted to the leader. + + :returns: ``True`` if the leader key has been created successfully. + """ + ret = self.attempt_to_acquire_leader() + if ret: + timestamp = time.time() + # every time we promote we need to reset retention time for slots recorded in the /status key + self._last_retain_slots = {name: timestamp for name in self._last_status['retain_slots']} + return ret + @abc.abstractmethod def set_failover_value(self, value: str, version: Optional[Any] = None) -> bool: """Create or update ``/failover`` key. diff --git a/patroni/ha.py b/patroni/ha.py index 8a198036..31ae3f76 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -345,11 +345,11 @@ class Ha(object): def acquire_lock(self) -> bool: try: - ret = self.dcs.attempt_to_acquire_leader() + ret = self.dcs.acquire_leader_lock() except DCSError: raise except Exception: - logger.exception('Unexpected exception raised from attempt_to_acquire_leader, please report it as a BUG') + logger.exception('Unexpected exception raised from acquire_leader_lock, please report it as a BUG') ret = False self.set_is_leader(ret) return ret