Pgpool not showing Replication State #1845
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
to-bar
commented
Nov 12, 2020
to-bar
commented
- Correct backend_application_name in pgpool.conf
- Group tasks and change order
- Remove unneeded tasks
- Group tasks using blocks
- Improve code
- Create symlink /etc/repmgr.conf
- Shorten repmgr commands due to symlink to config file
- Ensure postgresql instance is stopped
- Update changelog
- Update documentation
|
/azp run |
plirglo
approved these changes
Nov 16, 2020
sk4zuzu
approved these changes
Nov 16, 2020
There was a problem hiding this comment.
Code in this PR looks ok to me, but I'm not impressed after reading whole that role for the first time. :( I guess my suggestions are more for refactoring the role which is out of the scope of this PR. 🤗
My recommendations for the future would be:
- Make important settings (like archive_mode that we care about) mapped one-to-one with the target config files, which would elliminate having them seeded with random values coming from the package creator in case of issues with merging.
- Add schema validation! So an user has any kind of feedback if after any upgrade current config is not disabling/enabling some important settings.
- Make all configs dict-based instead of being list-based (that will enable merging) or we should add proper merging of list-based dicts with
name:as a key. - Get rid of lineinfile/replace statements, instead use full config templates and include statements.
- Probably good idea would be keeping Postgres' data dir in a custom place (identical on all Linux distros), it would be handy with upgrades I think.
- Relying on ordering of postgresql group is naive and simply incorrect if you take into account autoscaling groups in AWS. Is there any upgrade procedure to switch from 1 master to 1 master + 1 slave and back - is it supported?
- Please don't use the notorious Software Collections ever again anywhere in the code! 🤗
| mode: u=rw,g=,o= | ||
|
|
||
| - name: Add replication user to pg_hba.conf | ||
| lineinfile: |
There was a problem hiding this comment.
I don't really like that we're using lineinfile, for me it's ugly. I think we should have full control over all configuration options. I guess changing this here now is not in scope of this PR.. I just wanted to express my dissatisfaction. 👍😇
| - is_node_already_registered.stdout == "" | ||
| # For repmgr installed from Ubuntu package additional configuration is required before repmgrd is started as daemon | ||
| - name: Set repmgr.conf file in /etc/default/repmgrd | ||
| replace: |
There was a problem hiding this comment.
Same argument as in I don't really like that we're using lineinfile, for me it's ugly.. 🤗
...rc/epicli/data/common/ansible/playbooks/roles/postgresql/tasks/replication-repmgr-Debian.yml
Show resolved
Hide resolved
rafzei
pushed a commit
to rafzei/epiphany
that referenced
this pull request
Nov 26, 2020
* Correct backend_application_name in pgpool.conf * Group tasks and change order * Remove unneeded tasks * Group tasks using blocks * Improve code * Create symlink /etc/repmgr.conf * Shorten repmgr commands due to symlink to config file * Ensure postgresql instance is stopped * Update changelog * Update documentation
toszo
pushed a commit
to toszo/epiphany
that referenced
this pull request
Dec 5, 2020
* Correct backend_application_name in pgpool.conf * Group tasks and change order * Remove unneeded tasks * Group tasks using blocks * Improve code * Create symlink /etc/repmgr.conf * Shorten repmgr commands due to symlink to config file * Ensure postgresql instance is stopped * Update changelog * Update documentation
Closed
11 tasks
This was referenced May 18, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.