Add PostgreSQL HA cluster guide#8919
Open
sadohert wants to merge 17 commits into
Open
Conversation
Contributor
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdates documentation: clarifies the deployment guide excludes database-level HA/disaster recovery and adds a new PostgreSQL HA cluster admin guide (repmgr, HAProxy, Keepalived, pgchk) plus a TOC entry and a brief high-availability subsection. ChangesPostgreSQL HA documentation
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin
participant Keepalived
participant HAProxy
participant pgchk as pgchk (health service)
participant repmgr
participant Postgres1 as Primary
participant Postgres2 as StandbyA
participant Postgres3 as StandbyB
Admin->>Keepalived: configure VIP & priorities
Admin->>HAProxy: configure TCP frontends (5000 write, 5001 read)
Admin->>pgchk: deploy health check service
HAProxy->>pgchk: HTTP health check (8008)
pgchk->>repmgr: query node role/state
repmgr->>Postgres1: monitor primary status
repmgr->>Postgres2: monitor standbys
Note over repmgr,Postgres1: On primary failure
repmgr->>Postgres2: promote to primary
repmgr->>Keepalived: notify / changes reflected via health checks
Keepalived->>HAProxy: VIP moves to new primary host
HAProxy->>Postgres2: route write traffic to promoted node
Admin->>repmgr: re-register old primary as standby (post-repair)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
source/administration-guide/scale/high-availability-cluster-based-deployment.rst (1)
33-35: Clarify HA vs DR scope in this redirect sentence.This reads a bit awkwardly and mixes concepts; make the scope explicit as “database-level HA/DR design is out of scope here” before linking to the PostgreSQL HA page.
Suggested minimal wording update
-Set up and maintain a high availability cluster-based deployment on your Mattermost servers. This document doesn't cover the configuration of databases in terms of -disaster recovery. For self-hosted deployments requiring database-level HA, +Set up and maintain a high availability cluster-based deployment on your Mattermost servers. This document doesn't cover database high availability or disaster recovery design. +For self-hosted deployments requiring database-level HA, see :doc:`PostgreSQL high availability cluster </administration-guide/scale/postgres-ha-cluster>`.As per coding guidelines, "Flag awkward phrasing, punctuation mistakes, tense shifts, and terminology inconsistencies in documentation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/scale/high-availability-cluster-based-deployment.rst` around lines 33 - 35, Revise the redirect sentence that begins "For self-hosted deployments requiring database-level HA," to explicitly state that database-level HA and disaster recovery (DR) design are out of scope for this document, then follow with the existing link to the PostgreSQL page (the :doc:`PostgreSQL high availability cluster </administration-guide/scale/postgres-ha-cluster>` reference) so readers are clearly directed for DB-level HA/DR details; replace the awkward phrasing with a concise sentence such as "Database-level HA and DR design are out of scope for this document; for PostgreSQL-specific guidance, see :doc:`PostgreSQL high availability cluster </administration-guide/scale/postgres-ha-cluster>`."source/administration-guide/scale/postgres-ha-cluster.rst (1)
331-334: Tighten replication user privileges (or justify superuser explicitly).
createuser --superuser repmgris broader than necessary for many environments. Prefer least privilege by default, or add a strong warning explaining why elevated rights are required in this design.As per coding guidelines, "When reviewing ... through the lens of Veteran Vince ... flag content that is ... security-unsafe."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/scale/postgres-ha-cluster.rst` around lines 331 - 334, The current step creates a replication user with --superuser which is broader than needed; replace it with a least-privilege creation and explicit grants (create the user with REPLICATION and LOGIN, set a password, and grant only the specific DB ownership/SEARCH_PATH needed) and update the ALTER USER repmgr SET search_path line to match the non-superuser account; if you must keep --superuser for this design, add a prominent security justification/warning explaining why elevated rights are required and the risks. Refer to the existing commands (createuser --superuser repmgr and ALTER USER repmgr SET search_path TO repmgr, public) when making the change so the docs show the least-privilege variant and the optional justified-superuser warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/administration-guide/scale/postgres-ha-cluster.rst`:
- Around line 295-305: Change the documented pg_hba entries so that
``scram-sha-256`` is shown as the default production authentication method
instead of ``trust`` (update the example host lines and note text that currently
reference ``trust``), and move the ``trust`` entries into a clearly labeled
"lab/testing only" fallback section; also wrap the lab-only fallback in a
warning admonition to highlight the security risk (refer to the example host
entries for repmgr and replication and the existing note block that mentions
``.pgpass`` to update placement and wording accordingly).
- Around line 520-534: The Keepalived snippet hardcodes "interface eth0" which
will fail on systems with predictable NIC names; update the docs around creating
/etc/keepalived/keepalived.conf and the vrrp_instance VI_1 block to require
selecting the correct network interface (do not assume eth0), add a brief step
telling the user to identify the active interface (e.g., via ip link or ip addr)
and substitute that interface name into the "interface" field of the
vrrp_instance configuration before applying Keepalived, and include a short
example note instructing readers to replace <CLUSTER_VIP> and the placeholder
interface with their actual values.
- Around line 483-490: The doc currently references a non-existent repo for
pgchk.py; replace that broken external link by either embedding the full
pgchk.py script directly in this document (with a short explanation of its
purpose and usage) and instructing readers to save it to /usr/local/bin/pgchk.py
and chmod +x, or point to a verified alternate source and include a pinned
commit/tag and SHA256 checksum plus a one-line curl/wget + sha256sum
verification step; update the text around the symbol pgchk.py to include the
chosen solution and add verification instructions so admins can validate the
file before placing it at /usr/local/bin/pgchk.py.
---
Nitpick comments:
In
`@source/administration-guide/scale/high-availability-cluster-based-deployment.rst`:
- Around line 33-35: Revise the redirect sentence that begins "For self-hosted
deployments requiring database-level HA," to explicitly state that
database-level HA and disaster recovery (DR) design are out of scope for this
document, then follow with the existing link to the PostgreSQL page (the
:doc:`PostgreSQL high availability cluster
</administration-guide/scale/postgres-ha-cluster>` reference) so readers are
clearly directed for DB-level HA/DR details; replace the awkward phrasing with a
concise sentence such as "Database-level HA and DR design are out of scope for
this document; for PostgreSQL-specific guidance, see :doc:`PostgreSQL high
availability cluster </administration-guide/scale/postgres-ha-cluster>`."
In `@source/administration-guide/scale/postgres-ha-cluster.rst`:
- Around line 331-334: The current step creates a replication user with
--superuser which is broader than needed; replace it with a least-privilege
creation and explicit grants (create the user with REPLICATION and LOGIN, set a
password, and grant only the specific DB ownership/SEARCH_PATH needed) and
update the ALTER USER repmgr SET search_path line to match the non-superuser
account; if you must keep --superuser for this design, add a prominent security
justification/warning explaining why elevated rights are required and the risks.
Refer to the existing commands (createuser --superuser repmgr and ALTER USER
repmgr SET search_path TO repmgr, public) when making the change so the docs
show the least-privilege variant and the optional justified-superuser warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e4d9ba5-82a6-486e-8263-05d6d2475d02
📒 Files selected for processing (3)
source/administration-guide/scale/high-availability-cluster-based-deployment.rstsource/administration-guide/scale/postgres-ha-cluster.rstsource/administration-guide/scale/scaling-for-enterprise.rst
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sadohert
force-pushed
the
postgres-ha-cluster-guide
branch
from
0f144d7 to
1e7d060
Compare
- pg_hba.conf: default to scram-sha-256; trust moved to lab-only warning - pgchk.py: embed full script inline instead of linking to external repo - Keepalived: add ip link step to identify interface before hardcoding - repmgr createuser: add note explaining why superuser is required - high-availability-cluster-based-deployment.rst: clarify HA vs DR scope Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
@esethna @wiersgallak - Would one of you mind adding the |
esethna
added
the
preview-environment
Contributor
|
Newest code from sadohert has been published to preview environment for Git SHA faa3b49 |
Contributor
|
Done @sadohert |
Contributor
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
Contributor
|
Heya @sadohert - Is there you a dev you'd recommend we tag on this for the technical review? |
Contributor
Author
|
Hey @Combs7th - You could raise with Jesse, Alejandro, or Doug? |
wiersgallak
added
1: Dev Review
agarciamontoro
requested changes
Jun 4, 2026
Member
agarciamontoro
left a comment
agarciamontoro
left a comment
There was a problem hiding this comment.
Amazing guide! The only thing that I though "huh, weird", is the need for the Python script. Isn't there a way to automate that so that we can avoid using that mini-server? If not, it's ok, but I'm curious.
My comments are mostly nits, so treat them like that, but I wanted to make sure I understood everything :)
Comment on lines
+17
to
+18
| This guide has been validated on: **Ubuntu 24.04 LTS**, **PostgreSQL 17**, | ||
| **repmgr 5.5**, **HAProxy 2.8**, **Keepalived**. |
Member
There was a problem hiding this comment.
Are we missing the version in Keepalived?
Comment on lines
+23
to
+26
| A PostgreSQL HA cluster for Mattermost consists of three nodes running in | ||
| parallel. Each node runs the full stack: PostgreSQL, repmgr daemon (repmgrd), | ||
| HAProxy, Keepalived, and a health-check service. A Virtual IP (VIP) floats | ||
| across nodes and always points to the current primary. |
Member
There was a problem hiding this comment.
Does it need to be exactly three?
Comment on lines
+66
to
+73
| * - Keepalived | ||
| - — | ||
| - Manages the VIP using VRRP. Moves the VIP to the new primary after | ||
| failover. | ||
| * - pgchk.py | ||
| - — | ||
| - HTTP health-check endpoint (port 8008). HAProxy queries this to | ||
| determine which node is the current primary. |
Member
There was a problem hiding this comment.
There are no versions here either, not sure if intended, raising just in case.
Comment on lines
+226
to
+230
| .. code-block:: text | ||
|
|
||
| <PG1_IP> pg1 | ||
| <PG2_IP> pg2 | ||
| <PG3_IP> pg3 |
Member
There was a problem hiding this comment.
Maybe a dumb question: should each node use localhost for its own entry?
Comment on lines
+302
to
+308
| Create a ``.pgpass`` file on each node so repmgr can authenticate without | ||
| an interactive password prompt: | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| echo "*:*:repmgr:repmgr:<YOUR_REPMGR_PASSWORD>" >> ~/.pgpass | ||
| chmod 600 ~/.pgpass |
Member
There was a problem hiding this comment.
This reads a bit weird, because it seems we have already come up with a string for <YOUR_REPMGR_PASSWORD> before. However, we're deciding the password at this point, and the actual setting of the password happens later when we run ALTER USER repmgr PASSWORD '<YOUR_REPMGR_PASSWORD>'. It took me a couple of seconds to understand this, so maybe this warrants some short explanation here? Or maybe configuring this after we run the ALTER USER? Not sure.
Comment on lines
+484
to
+495
| backend pg_primary | ||
| option tcp-check | ||
| server pg1 <PG1_IP>:5432 check port 8008 | ||
| server pg2 <PG2_IP>:5432 check port 8008 backup | ||
| server pg3 <PG3_IP>:5432 check port 8008 backup | ||
|
|
||
| backend pg_replicas | ||
| balance roundrobin | ||
| option tcp-check | ||
| server pg2 <PG2_IP>:5432 check port 8008 | ||
| server pg3 <PG3_IP>:5432 check port 8008 | ||
| server pg1 <PG1_IP>:5432 check port 8008 backup |
Member
There was a problem hiding this comment.
I guess the order in each of these blocks is important. Should we make a note about that? Or would that be too much?
Comment on lines
+503
to
+581
| On each node, create ``/usr/local/bin/pgchk.py`` with the following content: | ||
|
|
||
| .. code-block:: python | ||
|
|
||
| #!/usr/bin/env python3 | ||
| import subprocess | ||
| from http.server import BaseHTTPRequestHandler, HTTPServer | ||
| import argparse | ||
|
|
||
| DEFAULT_PORT = 8008 | ||
| PG_USER = "postgres" | ||
| PG_DB = "postgres" | ||
| PG_PORT = "5432" | ||
|
|
||
| class PostgresHealthCheckHandler(BaseHTTPRequestHandler): | ||
| def safe_write(self, data): | ||
| try: | ||
| self.wfile.write(data) | ||
| except (BrokenPipeError, ConnectionResetError): | ||
| pass | ||
|
|
||
| def check_postgres_status(self): | ||
| try: | ||
| cmd = ["psql", "-U", PG_USER, "-d", PG_DB, "-p", PG_PORT, | ||
| "-t", "-c", "SELECT pg_is_in_recovery();"] | ||
| result = subprocess.run(cmd, capture_output=True, text=True, timeout=5) | ||
| if result.returncode != 0: | ||
| return None | ||
| output = result.stdout.strip() | ||
| if output == 't': | ||
| return True # Standby | ||
| elif output == 'f': | ||
| return False # Primary | ||
| return None | ||
| except Exception: | ||
| return None | ||
|
|
||
| def do_GET(self): | ||
| status = self.check_postgres_status() | ||
| if status is None: | ||
| self.send_response(503) | ||
| self.end_headers() | ||
| self.safe_write(b"PostgreSQL Unreachable\n") | ||
| return | ||
| if self.path in ('/', '/master'): | ||
| if not status: | ||
| self.send_response(200); self.end_headers() | ||
| self.safe_write(b"OK - Primary\n") | ||
| else: | ||
| self.send_response(503); self.end_headers() | ||
| self.safe_write(b"Service Unavailable - Not Primary\n") | ||
| elif self.path == '/replica': | ||
| if status: | ||
| self.send_response(200); self.end_headers() | ||
| self.safe_write(b"OK - Replica\n") | ||
| else: | ||
| self.send_response(503); self.end_headers() | ||
| self.safe_write(b"Service Unavailable - Not Replica\n") | ||
| else: | ||
| self.send_response(404); self.end_headers() | ||
| self.safe_write(b"Not Found\n") | ||
|
|
||
| def log_message(self, format, *args): | ||
| pass | ||
|
|
||
| def run(port=DEFAULT_PORT): | ||
| httpd = HTTPServer(('', port), PostgresHealthCheckHandler) | ||
| print(f"Starting PostgreSQL Health Check on port {port}...") | ||
| try: | ||
| httpd.serve_forever() | ||
| except KeyboardInterrupt: | ||
| pass | ||
| httpd.server_close() | ||
|
|
||
| if __name__ == '__main__': | ||
| parser = argparse.ArgumentParser(description='PostgreSQL Health Check for HAProxy') | ||
| parser.add_argument('--port', type=int, default=DEFAULT_PORT) | ||
| args = parser.parse_args() | ||
| run(port=args.port) |
Member
There was a problem hiding this comment.
Should we host this in a repo or in a gist so that we can track history here? Just an idea.
Comment on lines
+565
to
+566
| def log_message(self, format, *args): | ||
| pass |
Member
There was a problem hiding this comment.
This function is not used, we should remove it.
| sudo systemctl enable keepalived | ||
| sudo systemctl start keepalived | ||
|
|
||
| ✅ **Phase 4 checkpoint** — run on any node: |
Member
There was a problem hiding this comment.
every again? Maybe this is just me, sorry 😂
Suggested change
| ✅ **Phase 4 checkpoint** — run on any node: | |
| ✅ **Phase 4 checkpoint** — run on every node: |
Comment on lines
+779
to
+795
| Add a standby node | ||
| ~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| 1. Provision a new server and complete Phases 1–2 of the setup guide. | ||
| 2. Create ``/etc/repmgr.conf`` with the next available ``node_id``. | ||
| 3. On the new node: | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| sudo systemctl stop postgresql | ||
| sudo -u postgres repmgr -h <PRIMARY_IP> -U repmgr -d repmgr \ | ||
| -f /etc/repmgr.conf standby clone --delete-existing-pgdata | ||
| sudo systemctl start postgresql | ||
| sudo -u postgres repmgr -f /etc/repmgr.conf standby register | ||
|
|
||
| 4. Add the new node to ``/etc/haproxy/haproxy.cfg`` on all existing nodes and | ||
| reload HAProxy: ``sudo systemctl reload haproxy``. |
Member
There was a problem hiding this comment.
Do we need to change the /etc/hosts file of the pre-existing nodes to add the new pg4 with its IP?
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Summary
source/administration-guide/scale/postgres-ha-cluster.rst— a new guide for deploying a 3-node PostgreSQL HA cluster using repmgr, HAProxy, and Keepalivedscaling-for-enterprise.rsttoctree and adds a prose entryhigh-availability-cluster-based-deployment.rstto cross-link to the new pageWhy
The existing HA doc covers Mattermost app-layer clustering but explicitly does not cover database HA. Many self-hosted customers on bare-metal or VMs need a database-level HA guide. This fills that gap.
Page structure
The guide follows Mattermost documentation conventions:
Validation
All setup steps and checkpoint commands have been validated on Ubuntu 24.04 LTS, PostgreSQL 17, repmgr 5.5, HAProxy 2.8.
Related
🤖 Generated with Claude Code