* Use YAML files to validate Postgres GUCs through Patroni.
Patroni used to have a static list of Postgres GUCs validators in
`patroni.postgresql.validator`.
One problem with that approach, for example, is that it would not
allow GUCs from custom Postgres builds to be validated/accepted.
The idea that we had to work around that issue was to move the
validators from the source code to an external and extendable source.
With that Patroni will start reading the current validators from that
external source plus whatever custom validators are found.
From this commit onwards Patroni will read and parse all YAML files
that are found under the `patroni/postgresql/available_parameters`
directory to build its Postgres GUCs validation rules.
All the details about how this work can be found in the docstring
of the introduced function `_load_postgres_gucs_validators`.
When using a custom Postgres distribution it may be the case that the Postgres binaries are compiled with different names other than the ones used by the community Postgres distribution.
With that in mind we implemented a new set of settings for Patroni, so the user is able to override the default binary names with custom binary names through the new section postgresql.bin_name in the local configuration.
References: PAT-17.
- abstract_main only creates Config object using the passed configfile
and instantiates the passed daemon class
- common args parser is extracted into a separate func that is called
from daemons' main funcs (specific args can be added afterwards)
Co-authored-by: Alexander Kukushkin <[email protected]>
pass reference to a last known leader object in order to avoid obtaining it from the `AbstractDCS.cluster` cache.
This change is useful for Consul, Etcd3 and Zookeeper implementations.
It didn't took into account the fact that we can get a new lease after changing a TTL. In this case we have to update the exiting leader key with the new lease.
To solve it we introduce the on transaction 'failure' callback. The whole workflow looks like (schematically):
```python
txn(
compare=(old_value == self._name)),
success=put(self.leader_path, self._name, self._lease),
failure=txn(
compare=(create_revision == '0'),
success=put(self.leader_path, self._name, self._lease)
)
)
```
The problem was introduced in d98d6d0b02c9cc67464a7b1b31b1c5570c26e12d
- Always pass etcd3 key revision as a string
- Make sure the leader key isn't unconditionally overwritten. It may happen that the leader heart-beat loop didn't run properly and the session has expired. In this case the leader may create a new session and a new leader key. But, there are chances that the other node already created a leader key and we don't want to overwrite it.
- Try to sync HA loops between nodes by adding 0.5 seconds to timeout on non-leader nodes
- fix --validate-config not to error out if bin_dir is an empty string in the yaml config
- mention bin_dir optionality in the docs
- validate bin_dir even if it is not in the yaml config (add optional
default value for Optional config params in validator)
- make rewind user optional
Make it return the new `SyncState` object in order to avoid reading the new cluster state in the Ha.process_sync_replication().
Now it is a small optimization, but it will become very handy in the quorum commit feature.
- added pyrightconfig.json with typeCheckingMode=strict
- added type hints to all files except api.py
- added type stubs for dns, etcd, consul, kazoo, pysyncobj and other modules
- added type stubs for psycopg2 and urllib3 with some little fixes
- fixes most of the issues reported by pyright
- remaining issues will be addressed later, along with enabling CI linting task
The two cases we have in mind are:
* In spite of following all best practices client-side, logical replication connections can sometimes hang the Postgres shutdown sequence. We'd like to sigterm any misbehaving logical replication connections which remain after x seconds. These will inevitably get killed anyway on master stop timeout.
* remove "role=master" label on current primary when not using k8s as DCS. Waiting until after Postgres fully stops can sometimes be too long for this.
* Pause pgbouncer connections before switchover
Close#2596
Previous to this commit the `parse_dcs` function would fail with a `PatroniCtlException` if the user ever passed an `etcd3` URL through `--dcs-url` command-line option.
As a consequence, the only way of using `etcd3` in `patronictl` was by using a configuration file passed through `-c` command-line option.
This commit fixes that issue and allows `etcd3` to be used in `--dcs-url`.
References: PAT-91
Close#2638
1. make `SyncHandler.current_state()` return `CaseInsensitiveSet` instead of `list` objects.
2. take `sync_node_count` and `sync_node_maxlag` from the `Postgresql._global_config` instead of passing them as arguments.
3. Make `AbstractDCS.write_sync_state()` accept any `Collection`-like objects.
For now it implements:
- CaseInsensitiveDict()
- CaseInsensitiveSet()
Update `patroni.postgresql.sync.parse_sync_standby_names()` to use `CaseInsensitiveSet()` instead of `CaseInsensitiveDict()`
* Ignore D401 in flake8-docstrings
* Fix newly reported flake8 issues, ignore the old W503 rule
* rely on concatenation of adjecent strings
* Format behave scripts
* Reformat ha.py according to new rules
Co-authored-by: Alexander Kukushkin <[email protected]>
despite being EOL the number of downloads from pypi for 3.6 is on the first place, hence we need to support it.
Fixed a couple of problems in tests:
1. pytest complains about `call` imported from mock, mock.call() is fine
2. one test of Citus intergration was broken
Previously it used to compare between the leader and sync_standbys, while in some cases (actually most of them) the leader should be excluded.
This commit makes `matches()` method flexible:
1. The leader will be included to comparison only if requested
2. checks will be performed as case insensitive (like PG does)
Besides that, everywhere in code start using `cluster.sync.matches()` instead of `name in cluster.sync.members`.
`patronictl edit-config` requires a pager to show the diff output back to the user. It used to be hard-coded to use either `less` or `more`.
When these tools were not available in the host that would cause `patronictl` to face an exception in `ydiff` module and to show the stack trace in the console.
This PR changes `patronictl edit-config` command to behave like this:
- If `PAGER` environment variable is set, attempt to find the corresponding executable.
- If `PAGER` is not set or is set with an invalid executable, then attempt to use either `less` or `more` as it used to do.
- If no executable is find at all then throw a `PatroniCtlException` to show an user friendly message
Unit tests in `tests/test_ctl.py` were modified accordingly.
References: PAT-21
Close#2604
It was possible to have it empty if the all cluster keys are missing in DCS. In this case the `Cluster` object was manually created with all values set to `None` or `[]` (including sync).
It already resulted in #2217, which is in fact wasn't a correct fix.
In order to solve it and reduce code duplication we introduce `Cluster.empty()` and `SyncState.empty()` methods, which will create corresponding empty objects and start using `Cluster.empty()` from all places where the empty `Cluster` object was manually created.
If communication with etcd nodes failed it is logical to start from scratch, from nodes that are listed in the config. But, it could happen that config is in fact outdated and all nodes in the real cluster were replaced.
Previously we used to track whether config file was changed, which turned out not to work in all possible cases.
The new strategy is a bit more different - if communication with all nodes failed we will continue keeping the last know topology and at the same time will try to figure out the new one by merging two lists together, the cached list and the list from the config file.
Configuration parameter is `kubernetes.retriable_http_codes` or `PATRONI_KUBERNETES_RETRIABLE_HTTP_CODES` environment variable.
These status codes are added to the default list of 500, 503, 504.
Close https://github.com/zalando/patroni/issues/2536
It could happen that Patroni is started up before PGDATA was mounted. In this case Patroni can't determine major Postgres version from PG_VERSION file. Later, when PGDATA is mounted, Patroni was trying to create the recovery.conf even if the actual Postgres major version is newver than 12.
To mitigate the problem we double check that the `Postgresql._major_version` is set before writing recovery configuration or starting postgres up.
Close https://github.com/zalando/patroni/issues/2434
Since a long time Patroni enforcing only one callback script running at a time. If the new callback is executed while the old one is still running, the old one is killed (including all child processes).
Such behavior is fine for all callbacks but on_reload, because the last one may accidentally cancel important ones, that for example updating DNS or assigning/removing Virtual IP.
To mitigate the problem we introduce a dedicated executor for on_reload callbacks, so that on_reload may only cancel another on_reload.
Ref: https://github.com/zalando/patroni/issues/2445
keep as much backward compatibility as possible.
Following changes were made:
1. All internal checks are performed as `role in ('master', 'primary')`
2. All internal variables/functions/methods are renamed
3. `GET /metrics` endpoint returns `patroni_primary` in addition to `patroni_master`.
4. Logs are changed to use leader/primary/member/remote depending on the context
5. Unit-tests are using only role = 'primary' instead of 'master' to verify that 1 works.
6. patronictl still supports old syntax, but also accepts `--leader` and `--primary`.
7. `master_(start|stop)_timeout` is automatically translated to `primary_(start|stop)_timeout` if the last one is not set.
8. updated the documentation and some examples
Future plan: in the next major release switch role name from `master` to `primary` and maybe drop `master` altogether.
The Kubernetes implementation will require more work and keep two labels in parallel. Label values should probably be configurable as described in https://github.com/zalando/patroni/issues/2495.
Citus cluster (coordinator and workers) will be stored in DCS as a fleet of Patroni logically grouped together:
```
/service/batman/
/service/batman/0/
/service/batman/0/initialize
/service/batman/0/leader
/service/batman/0/members/
/service/batman/0/members/m1
/service/batman/0/members/m2
/service/batman/
/service/batman/1/
/service/batman/1/initialize
/service/batman/1/leader
/service/batman/1/members/
/service/batman/1/members/m1
/service/batman/1/members/m2
...
```
Where 0 is a Citus group for coordinator and 1, 2, etc are worker groups.
Such hierarchy allows reading the entire Citus cluster with a single call to DCS (except Zookeeper).
The get_cluster() method will be reading the entire Citus cluster on the coordinator because it needs to discover workers. For the worker cluster it will be reading the subtree of its own group.
Besides that we introduce a new method get_citus_coordinator(). It will be used only by worker clusters.
Since there is no hierarchical structures on K8s we will use the citus group suffix on all objects that Patroni creates.
E.g.
```
batman-0-leader # the leader config map for the coordinator
batman-0-config # the config map holding initialize, config, and history "keys"
...
batman-1-leader # the leader config map for worker group 1
batman-1-config
...
```
Citus integration is enabled from patroni.yaml:
```yaml
citus:
database: citus
group: 0 # 0 is for coordinator, 1, 2, etc are for workers
```
If enabled, Patroni will create the database, citus extension in it, and INSERTs INTO `pg_dist_authinfo` information required for Citus nodes to communicate between each other, i.e. 'password', 'sslcert', 'sslkey' for superuser if they are defined in the Patroni configuration file.
When the new Citus coordinator/worker is bootstrapped, Patroni adds `synchronous_mode: on` to the `bootstrap.dcs` section.
Besides that, Patroni takes over management of some Postgres GUCs:
- `shared_preload_libraries` - Patroni ensures that the "citus" is added to the first place
- `max_prepared_transactions` - if not set or set to 0, Patroni changes the value to `max_connections*2`
- wal_level - automatically set to logical. It is used by Citus to move/split shards. Under the hood Citus is creating/removing replication slots and they are automatically added by Patroni to the `ignore_slots` configuration to avoid accidental removal.
The coordinator primary actively discovers worker primary nodes and registers/updates them in the `pg_dist_node` table using
citus_add_node() and citus_update_node() functions.
Patroni running on the coordinator provides the new REST API endpoint: `POST /citus`. It is used by workers to facilitate controlled switchovers and restarts of worker primaries.
When the worker primary needs to shut down Postgres because of restart or switchover, it calls the `POST /citus` endpoint on the coordinator and the Patroni on the coordinator starts a transaction and calls `citus_update_node(nodeid, 'host-demoted', port)` in order to pause client connections that work with the given worker.
Once the new leader is elected or postgres started back, they perform another call to the `POST/citus` endpoint, that does another `citus_update_node()` call with actual hostname and port and commits a transaction. After transaction is committed, coordinator reestablishes connections to the worker node and client connections are unblocked.
If clients don't run long transaction the operation finishes without client visible errors, but only a short latency spike.
All operations on the `pg_dist_node` are serialized by Patroni on the coordinator. It allows to have more control and ROLLBACK transaction in progress if its lifetime exceeding a certain threshold and there are other worker nodes should be updated.
When `synchronous_standby_names` GUC is changed PostgreSQL nearly immediately starts reporting corresponding walsenders as synchronous, while in fact they maybe didn't reach this state yet. To mitigate this problem we memorize current flush lsn on the primary right after change of `synchronous_standby_names` got visible and use it as an additional check for walsenders.
The walsender will be counted as truly "sync" only when write/flush/replay_lsn on it reached memorized LSN and the `application_name` is known to be a part of `synchronous_standby_names`.
The size of PR mostly related to refactoring and moving the code responsible for working with `synchronous_standby_names` and `pg_stat_replication` to the dedicated file.
And `parse_sync_standby_names()` function was mostly copied from #672.
1. Fix problem with logical slots not advancing when only the primary lost access to DCS
2. Don't let Patroni to join as a raft voting member when running failsafe behave tests. It allows to test exactly the same conditions as for other DCS
3. Speed up dcs_failsafe_mode behave tests by getting rid from long sleeps, slight reshuffling of places when we start/stop outage, and by killing Patroni/Postgres to avoid long shutdown due to the leader key removal attempts.
If enabled it will allow Patroni to cope with DCS outages.
In case of a DCS outage the leader tries to call all remaining members in the cluster via API and if all of them respond with success the leader will not be demoted.
The failsafe_mode could be enabled by running
```sh
patronictl edit-config -s failsafe_mode=true
```
or by calling the `/config` REST API endpoint.
Co-authored-by: Polina Bungina <[email protected]>
If the cluster is stable (no nodes are joining/leaving/lagging) we want to run at most one monitor query per every HA loop. So far it worker perfectly except when synchronous_mode is enabled, where we run two additional queries:
1. SHOW synchronous_mode
2. SELECT ... FROM pg_stat_replication
In order to solve it, we will include these "queries" to the common monitoring query is synchronous_mode is enabled.
In addition to that make sure that `synchronous_standby_names` is reset on replicas that used to be a primary and avoid using replicas which are not in the 'running' state.
P.S.: in the monitoring query we also extract the current value of synchronous_standby_names, because it will be useful for the quorum commit feature.
Close https://github.com/zalando/patroni/issues/2469
When doing the leader race we need to check that the former primary isn't alive anymore. For that we relied on non-inclusive terms. In order to simplify future work on getting rid from all non-inclusive words we change the check to rely on a difference in format of wal/xlog field. There is only "location" for the primary and "replayed_location" + "received_location" for standbys.
In addition to that we start supporting "wal" field as well as deprecated "xlog".
Co-authored-by: Polina Bungina <[email protected]>
There is a known [vector of attact](https://pganalyze.com/blog/5mins-postgres-security-patch-releases-pgspot-pghostile) by creating functions and/or operators in a public scheme with the same name and signature as corresponding objects in `pg_catalog`.
Since Patroni is heavily relying on superuser connections we want to mitigate it by enforcing `search_path=pg_catalog` for all connections created by Patroni (except replication connections). It is achieved by introducing a new function, that wraps psycopg.connect() and appends ` -c search_path=pg_catalog` to `options` parameter.
In addition to that, we set connection.autocommit to True before returning it.
Fix issue of patroni configuring watchdog with defaults when bootstrapping a new cluster rather than taking configuration used to bootstrap the DCS.
Also log changes to watchdog configuration based on calculated timeout value.
Close#2470
Previously such an exception was raised only from the `get_cluster()` method, and now we will to do the same from the `update_leader()` and `attempt_to_acquire_leader()` methods.
These methods influence Postgres promotion and demotion and we want to make a difference between different types of failures. Specifically, if calls have failed because DCS isn't accessible or due to a timeout.
This commit is extracted from the #2379