1. Enforce `_load_cluster()` after acquisition for the leader lock in ZooKeeper. Sometimes the notification from ZooKeeper was arriving too late and Patroni wasn't setting the `role=standby_leader`.
2. The `_get_node_to_follow()` method was falsely assuming that we still own the leader lock and returning the remote node instead of the new standby leader. While not a big issue per se, because the next HA loop usually fixes it, such behavior was causing flakiness of behave tests with Postgres 12 and older, where restart is required to update `primary_conninfo` GUC.
It used to work before #2652
Besides that fix a couple of more problems:
- make sure `_patch_or_create()` method isn't instantiating the `k8s_client.V1ConfigMap` object instead of `k8s_client.V1Endpoints` for non leader endpoints. The only reason it worked is that the JSON serialization for both object types is the same and doesn't include the object type name.
- `attempt_to_acquire_leader()` should immediately put the IP address of the primary to the leader endpoint. It didn't happen because of the oversight in the https://github.com/zalando/patroni/pull/1820.
* Add `etcd3` as a DCS option for behave tests in `tox.ini`
Currently behave tests run through `tox` accept only `etcd` as a DCS.
This commit adds the option of using `etcd3` too.
* Add JSON report to `tox` behave tests
This commit adds a JSON report when running behave tests through
`tox`.
That makes it easier to parse the results.
---------
Signed-off-by: Israel Barth Rubio <[email protected]>
This PR introduces a documentation page for `patronictl` application.
We adopted a top-down approach when writing this document. We start by describing the outer most parts, and then keep writing new sections that specialize the knowledge.
We basically added a section called `patronictl` to the left menu. Inside that section we created a page with this structure:
- `patronictl`: describes what it is
- `Configuraiton`: how to configure `patronictl`
- `Usage`: how to use the CLI. Inside this section, there are subsections for each of the subcommands exposed by `patronictl`, and each of them are described using the following subsubsections:
- `Synopsis`: syntax of the command and its positional and optional arguments
- `Description`: a description of what the command does
- `Parameters`: a detailed description of the arguments and how to use them
- `Examples`: one or more examples of execution of the command
References: PAT-200.
- get rid from sleeps
- reduce retry_timeout
- avoid graceful Patroni shut down while DCS is "paused", just kill
Patroni and after that gracefully stop postgres
- don't try to delete Pod when Patroni is killed. If K8s API is paused it takes ages
The run time on my laptop is reduced from 2m to 1m28s.
yaml parser interprets such lines as null and stores it as None into the
array of the parsed values, which can not be handled by write() function
and crashes the whole bootstrap process.
Even though it is not the proper value, it won't hurt if we just ignore it instead of failing completely.
We used to call `socket` module's functions from the config_generator tests
to later compare with the output produced by --generate-config. That
however sometimes ends up with the whole test module failure if gethostname()
returned None.
Also includes a little code deduplication (NO_VALUE_MSG imported directly from the config_generator module)
and removes debug maxDiff option
Patroni is changing `synchronous_standby_names` and the `/sync` key in a very specific order, first we add nodes to `synchronous_standby_names` and only after, when they are recognized as synchronous they are added to the `/sync` key. When removing nodes the order is different: they are first removed from the `/sync` key and only after that from the `synchronous_standby_names`.
As a result Patroni expects that either actual synchronous nodes will match with the nodes listed in the `/sync` key or that new candidates to synchronous nodes will not match with nodes listed in the `/sync` key. In case if `synchronous_standby_names` was removed from the `postgresql.conf`, manually, or due the the bug (#2876), the state becomes inconsistent because of the wrong order of updates.
To solve inconsistent state we introduce additional checks and will update the `/sync` key with actual names of synchronous nodes (usually empty set).
If restarted in pause Patroni was discarding `synchronous_standby_names` from `postgresql.conf` because in the internal cache this values was set to `None`. As a result synchronous replication transitioned to a broken state, with no synchronous replicas according to the `synchronous_standby_names` and Patroni not selecting/setting the new synchronous replicas (another bug).
To solve the problem of broken initial state and to avoid similar issues with other GUC's we will read GUC's value if Patroni is joining running Postgres.
Patroni doesn't watch on all changes of member keys in order to not create too much load on ZooKeeper, but only subscribes to changes (ZNodes added or deleted) in the `/member` directory. Therefore when some important fields in the value are updated we remove and recreate ZNode in order to notify the leader or other members.
The leader should remove the member key only when the `checkpoint_after_promote` value is changed and replicas when the `state` is changed to/from `running`.
We don't care about the `version` field, because Patroni version can't be changed without restart, what will case ZooKeeper `session_id` to change it anyway.
This fix hopefully will reduce failures of behave tests on GH Actions.
Despite being validated by `IntValidator` some GUC's couldn't be casted directly to `int` because they include suffix. Example: `128MB`.
Close https://github.com/zalando/patroni/issues/2879
WARNING messages are produced by `urllib3` if Patroni is quickly restarted.
Instead we will check that the node is listen on a given port. This fact is actually enough to detect names clashes, while HTTP request could raise an exception is a few other cases, what might case false negatives.
Close https://github.com/zalando/patroni/issues/2881
Create permanent physical replication slots on standby nodes and use `pg_replication_slot_advance()` function to move them forward.
The `restart_lsn` is advanced based on values stored in the `/status` key by the primary node.
When slot is created on a replica it could be ahead the same slot on the primary and therefore there is some period of time when it doesn't protect WAL files from being recycled.
- Allow pip to modify an EXTERNALLY-MANAGED Python installation by passing --break-system-packages
- Build Citus for arm64
- Don't use PG_MAJOR argument
As was reported by @ants on Slack it could happen that `received_tli` is ahead of replayed timeline, therefore we should stop using it when deciding on pg_rewind if postgres is running and use only `IDENTIFY_SYSTEM` via replication connection.
Previous to this commit `patronictl query` was working only if `-r` argument was provided to the command. Otherwise it would face issues:
* If neither `-r` nor `-m` were provided:
```
PGPASSWORD=zalando patronictl -c postgres0.yml query -U postgres -c "SHOW PORT"
2023-09-12 17:45:38 No connection to role=None is available
```
* If only `-m` was provided:
```
$ PGPASSWORD=zalando patronictl -c postgres0.yml query -U postgres -c "SHOW PORT" -m postgresql0
2023-09-12 17:46:15 No connection to member postgresql0 is available
```
This issue was a regression introduced by `4c3e0b9382820524239d2aa4d6b95379ef1291db` through PR #2687.
Through that PR we decided to move the common logic used to check mutually exclusiveness of `--role` and `--member` arguments to `get_any_member` function.
However, previous to that change `role` variable would assume the default value of `any` in `query` method, before `get_any_member` was called, which was not the case after the change.
This commit fixes that issue by adding a handler in `get_cursor` function to `role=None`. As `role` defaulting to `any` is handled in a sub-call to `get_any_member`, we are apparently safe in `get_cursor` to return the cursor if `role=None`.
Unit tests were updated accordingly.
References: PAT-204.
It represents the `/status` key in DCS and makes it easier to introduce new values stored in the `/status` key without need to refactor all DCS implementations.
- Don't set leader in failover key from patronictl failover
- Show warning and execute switchover if leader option is provided for patronictl failover command
- Be more precise in the log messages
- Allow to failover to an async candidate in sync mode
- Check if candidate is the same as the leader specified in api
- Fix and extend some tests
- Add documentation
Some special handling is required when changing either of these settings in a Postgres cluster that has standby nodes:
* `max_connections`
* `max_prepared_transactions`
* `max_locks_per_transaction`
* `max_wal_senders`
* `max_worker_processes`
If one attempts to decrease `max_connections` dynamic setting and restart all nodes at the same time (primary and standbys), Patroni will refuse to apply the new value on the standbys and require the user to restart it again later, once replication catches up.
That behavior is correct, but it is not documented.
This commit adds information to documentation about that behavior and why it's required.
References: PAT-166.
* Generate documentation of private members through sphinx docs
With this commit we make sphinx build API docs for the following
things, which were missing up to this point:
* `__init__` method of classes;
* "private" members (properties, functions, methods, attributes, etc.,
which name starts with an underscore);
* members that are missing a docstring, so we can still reference
them with links in the documentation.
The third point can be removed later, if we wish, when we reach a
point where everything has proper docstrings in the Patroni code base.
* Fix documentation problems found after enabling private methods in sphinx
* `:cvar:` is not a valid domain role. Replaced with `:attr:`.
* documentation for `consul.base.Consul.__init__` has a single backtick
quoted string which is interpreted as a reference which cannot be found.
Therefore, the docstring has been copied as a block quote.
* various list spacing problems and indentation problems.
* code blocks added where indentation is interpreted incorrectly
* literal string quoting issues.
---------
Signed-off-by: Israel Barth Rubio <[email protected]>
Co-authored-by: Matt Baker <[email protected]>
1. make _get_members_slots() method return data in the same format as _get_permanent_slots() method
2. move conflicting name handling from get_replication_slots() to _get_members_slots() method
3. enrich structure returned by get_replication_slots() with the LSN of permanent logical slots reported by primary
4. use the added information in the SlotsHandler instead of fetching it from the Cluster.slots
5. bugfix: don't try to advance logical slot that doesn't match required configuration
Due to a race condition Patroni was falsely assuming that the standby should be restarted because some recovery parameters (primary_conninfo or similar) were changed.
Close https://github.com/zalando/patroni/issues/2834
Sharing a single connection between REST API and the main thread (doing heartbeats) was working mostly fine, except when Postgres becomes so slow that REST API queries start blocking the main loop.
If the dedicated REST API connection isn't available we use the heartbeat connection as a fallback.
* Use virtualenv to install tox in behave Dockerfile
Upstream change in postgres docker image uses debian restriction on
installing system-wide non-debian python packages. Debian doesn't
provide a tox>=4, so we need to install with pip.
* Exclude all output directories generated using `tox-wrapper.sh`
The `tox-wrapper.sh` script created by `features/Dockerfile` creates
directories like features/output-tox-pg14-docker-behave-etcd-lin-973719674/
* Reduce footprint of tox behave docker image
When running with the leader lock Patroni was just setting the `role` label to `master` and effectively `kubernetes.standby_leader_label_value` feature never worked.
Now it is fixed, but in order to not introduce breaking changes we just update default value of the `standby_leader_label_value` to the `master`.
`patronictl` is implemented using `click` module, and that module uses the functions' docstrings for creating a helper text.
As a consequence the docstring for `ctl` function was being shown to the user, which doesn't make sense.
This PR fixes that issue by adding a user-friendly description to be shown on `patronictl --help`. We use a `\f` to tell `click` when to stop capturing text to show in the helper.
Note that `patronictl` commands are implemented using `@ctl.command` decorator, and we always provide them with `help` argument. That said, none of the subcommands are affected by the aforementioned issue, only the entry point of the CLI.
References: PAT-201.
1.0.3 removed support of `top_level` configuration parameter and builds
now are failing.
Besides that remove redundant pyyaml from requirements.docs.txt
Make it hold connection kwargs for local connections and all `NamedConnection` objects use them automatically.
Also get rid of redundant `ConfigHandler.local_connect_kwargs`.
On top of that we will introduce a dedicated connection for the REST API thread.
The same (almost) logic was used in three different places:
1. `Patroni` class
2. `Member` class
3. `_MemberStatus` class
Now they all inherit newly intoduced `Tags` class.
* Add failsafe_mode_is_active to /patroni and /metrics
* Add patroni_primary to /metrics
* Add examples showing that failsafe_mode_is_active and cluster_unlocked
are only shown for /patroni when the value is "true"
* Update /patroni and /config examples
Cluster.get_replication_slots() didn't take into account that there can not be logical replication slots in a standby cluster replicas. It was only skipping logical slots for the standby_leader, but replicas were expecting that they will have to copy them over.
Also on replicas in a standby cluster these logical slots were falsely added to the `_replication_slots` dict.
