Postgres Disaster Recovery (DR) docs; remove remote-replica#912
Open
tamalsaha wants to merge 8 commits into
Open
Postgres Disaster Recovery (DR) docs; remove remote-replica#912tamalsaha wants to merge 8 commits into
tamalsaha wants to merge 8 commits into
Conversation
Document the distributed Postgres DC-DR feature under guides/postgres/distributed/dc-dr: the architecture (intra-DC raft per data center, the primary-DC Lease as the single cross-DC authority, the fail-closed local fence, leader-to-leader streaming), data center roles (Member/Arbiter/ Witness), prerequisites and operator flags, deploying a DC-DR Postgres via a PlacementPolicy failover policy, reading status.disasterRecovery, unplanned failover, zero-RPO planned switchover, per-DC horizontal scaling, and day-2 ops. Signed-off-by: Tamal Saha <tamal@appscode.com>
![kodiak-appscode[bot]](https://avatars.githubusercontent.com/in/92481?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds new DC-DR Postgres documentation pages, updates Postgres guide navigation and path references, and removes the Remote Replica guide content and manifests. ChangesDC-DR Postgres Documentation
Remote Replica removal
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guides/postgres/distributed/dc-dr/index.md`:
- Around line 129-130: The wording in the distributed DC/DR guide is
contradictory because it says a Witness rule can bear data, which conflicts with
the definition of Witness elsewhere in the doc. Update the sentence in the
section describing rules with replicaIndices so it refers only to Member rules
as bearing Postgres data, and keep Arbiter and Witness as not carrying data; use
the surrounding terms like replicaIndices, Member, Arbiter, and Witness to
locate and revise the line.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0abaacfa-3540-46ff-bcef-c2583ebefa3e
📒 Files selected for processing (1)
docs/guides/postgres/distributed/dc-dr/index.md
Turn guides/postgres/distributed/dc-dr into a section (overview + user guide + runbook): - overview: the architecture and a quick start (moved from the section root). - guide: a detailed user guide covering every aspect of running in DC-DR mode - components, the DC-name contract, operator flags, deployment, connecting, monitoring (status.disasterRecovery, the marker, lag annotation), replication and RPO, the timing invariant and tuning (fence TTL vs Lease duration), quorum and arbiters, planned switchover and failback, per-DC horizontal scaling, day-2 ops, backup, deletion, and limitations. - runbook: scenario-by-scenario procedures (active DC lost, partition, switchover, failback, standby DC lost, coordination plane down, stuck failover/switchover, growing lag, fence tripped, re-adding a DC, scaling, upgrade, suspected split-brain) with symptoms, automatic behavior, verification, and actions. Signed-off-by: Tamal Saha <tamal@appscode.com>
tamalsaha
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (6)
docs/guides/postgres/distributed/dc-dr/runbook/index.md (1)
15-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHyphenate compound modifier.
"cross data center" should be "cross-data-center" when used as a compound modifier before "disaster recovery".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/distributed/dc-dr/runbook/index.md` around lines 15 - 16, Update the prose in the DC-DR runbook intro so the compound modifier is hyphenated: change the wording in the opening sentence to use “cross-data-center” before “disaster recovery,” keeping the rest of the description unchanged.docs/guides/postgres/distributed/dc-dr/guide/index.md (5)
15-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHyphenate compound modifier.
"cross data center" should be "cross-data-center" when used as a compound modifier before "disaster recovery".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/distributed/dc-dr/guide/index.md` around lines 15 - 19, Hyphenate the compound modifier in the guide intro: update the phrasing in the opening sentence so the text uses cross-data-center when it modifies disaster recovery, and keep the change localized to the introductory description in the guide content.
66-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify language for configuration block.
Add
bashorshellto the fenced code block.
<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/guides/postgres/distributed/dc-dr/guide/index.mdaround lines 66 - 70,
The fenced configuration block in the dc-dr guide is missing a language tag, so
update the Markdown fence around the options list to use a shell-like language
such as bash or shell. Locate the code block containing the dc-dr flags and
adjust the opening fence accordingly so the snippet is properly highlighted and
rendered.</details> <!-- cr-comment:v1:0b4062fe53256414db7056f1 --> --- `25-25`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **Use descriptive link text.** Replace "here" with descriptive text for accessibility. ```diff - > **New to KubeDB?** Please start [here](/docs/README.md). + > **New to KubeDB?** Please start with the [KubeDB quick start guide](/docs/README.md).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/distributed/dc-dr/guide/index.md` at line 25, The link text in the opening callout is too generic and should be replaced with descriptive text for accessibility. Update the markdown in this guide so the link currently labeled with the ambiguous “here” uses meaningful text that describes the destination, keeping the same target and preserving the existing callout wording otherwise.
40-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify language for fenced code block.
Add
textlanguage identifier to the pseudocode block for markdownlint compliance and consistent rendering.
ConfigMap primary-dc (namespace: dc-failover, on each spoke)<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/guides/postgres/distributed/dc-dr/guide/index.mdaround lines 40 - 45,
Specify the fenced pseudocode block in the distributed DC-DR guide so it uses an
explicit language tag. Update the markdown in the section showing the ConfigMap
example by adding the text identifier to the fenced block around the content
near the primary-dc example, keeping the existing content unchanged.</details> <!-- cr-comment:v1:c7953fce20ca4fdb02ec92ea --> --- `259-259`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _💤 Low value_ **Prefer "deliberately" for formality.** "deliberately" is more formal than "on purpose" in technical documentation. ```diff - Move the active DC on purpose by annotating the Postgres: + Move the active DC deliberately by annotating the Postgres:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/distributed/dc-dr/guide/index.md` at line 259, The phrasing in the Postgres DC move instruction is too informal; update the sentence in the guide content to use the more formal term “deliberately” instead of “on purpose.” Keep the surrounding instruction intact and adjust only the wording in that documented step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/guides/postgres/distributed/dc-dr/guide/index.md`:
- Around line 15-19: Hyphenate the compound modifier in the guide intro: update
the phrasing in the opening sentence so the text uses cross-data-center when it
modifies disaster recovery, and keep the change localized to the introductory
description in the guide content.
- Around line 66-70: The fenced configuration block in the dc-dr guide is
missing a language tag, so update the Markdown fence around the options list to
use a shell-like language such as bash or shell. Locate the code block
containing the dc-dr flags and adjust the opening fence accordingly so the
snippet is properly highlighted and rendered.
- Line 25: The link text in the opening callout is too generic and should be
replaced with descriptive text for accessibility. Update the markdown in this
guide so the link currently labeled with the ambiguous “here” uses meaningful
text that describes the destination, keeping the same target and preserving the
existing callout wording otherwise.
- Around line 40-45: Specify the fenced pseudocode block in the distributed
DC-DR guide so it uses an explicit language tag. Update the markdown in the
section showing the ConfigMap example by adding the text identifier to the
fenced block around the content near the primary-dc example, keeping the
existing content unchanged.
- Line 259: The phrasing in the Postgres DC move instruction is too informal;
update the sentence in the guide content to use the more formal term
“deliberately” instead of “on purpose.” Keep the surrounding instruction intact
and adjust only the wording in that documented step.
In `@docs/guides/postgres/distributed/dc-dr/runbook/index.md`:
- Around line 15-16: Update the prose in the DC-DR runbook intro so the compound
modifier is hyphenated: change the wording in the opening sentence to use
“cross-data-center” before “disaster recovery,” keeping the rest of the
description unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 753c70f9-803d-463b-9f2c-449362dee0e8
📒 Files selected for processing (4)
docs/guides/postgres/distributed/dc-dr/_index.mddocs/guides/postgres/distributed/dc-dr/guide/index.mddocs/guides/postgres/distributed/dc-dr/overview/index.mddocs/guides/postgres/distributed/dc-dr/runbook/index.md
✅ Files skipped from review due to trivial changes (2)
- docs/guides/postgres/distributed/dc-dr/_index.md
- docs/guides/postgres/distributed/dc-dr/overview/index.md
Remove the Postgres remote-replica guide (guides/postgres/remote-replica); the distributed Postgres cross-DC disaster recovery (DC-DR) docs are the supported multi-data-center story. - Delete guides/postgres/remote-replica (the page and its yamls). - README: replace the "Remote Replica Support" entry with a "Cross-DC Disaster Recovery (DC-DR)" pointer to the new guide. - pitr/archiver.md: fix the stale yaml-folder note to point at the tutorial's own pitr/yamls (it never used the remote-replica yamls). Signed-off-by: Tamal Saha <tamal@appscode.com>
tamalsaha
changed the title
Pull the cross-DC disaster recovery docs out from under "Postgres Distributed" into their own top-level "DR" section: guides/postgres/distributed/dc-dr -> guides/postgres/dr (overview, user guide, runbook). Re-parent the section to the Postgres guides root, rename the menu entry to "DR", and update the menu identifiers, internal cross-links, and the README pointer. The pages still build on Distributed Postgres (that link is preserved). Signed-off-by: Tamal Saha <tamal@appscode.com>
tamalsaha
changed the title
Add a "Deployment topologies" section to the DR overview clarifying that data lives in >=2 Member DCs while the failover decision needs a majority of three quorum sites, and showing the three layouts: - Two Member DCs + a Witness (mode TwoDC): the recommended three-site setup; put the vote-only, data-less Witness in a third public cloud or region so any single site loss still elects a majority and fails over automatically. - Three Member DCs (mode ThreeDC): a full data copy and primary capability in all three locations. - Two sites only: co-locate the third quorum vote in one of the two DCs, with reduced, asymmetric resiliency (losing the two-vote DC needs manual recovery). Includes example PlacementPolicies and an at-a-glance comparison, and a pointer from the user guide's quorum section. Signed-off-by: Tamal Saha <tamal@appscode.com>
Match the API rename of the per-DC identifier from name to clusterName: the status.disasterRecovery example and field table, the horizontalScaling.dataCenters examples, and the status jsonpath queries in the runbook. Signed-off-by: Tamal Saha <tamal@appscode.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/guides/postgres/dr/runbook/index.md (2)
112-266: 🎯 Functional Correctness | 🟠 MajorMissing scenarios 8 and 10 from the runbook.
The document references Scenario 8 ("Planned switchover stuck") at line 112 and contains content appropriate for Scenario 10 ("Unexpectedly read-only / fence tripped") per the PR objectives, but the actual scenario sections are absent. The file jumps from
## 7.(line 169) directly to## 9.(line 221), and from## 9.(line 264) directly to## 11.(line 266). Verify the intended content for scenarios 8 and 10 was not lost during the documentation reorganization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/dr/runbook/index.md` around lines 112 - 266, The runbook is missing the actual sections for Scenario 8 and Scenario 10, even though Scenario 8 is referenced and the content for the fence-tripped case is present without a matching heading. Restore the missing numbered sections in the Postgres DR runbook so the sequence runs from Scenario 7 to 11 without gaps, placing the “Planned switchover stuck” guidance under the Scenario 8 heading and the “A DC is unexpectedly read-only (fence tripped)” guidance under the Scenario 10 heading, with the section titles and diagnosis/action subsections aligned to the existing runbook structure.
195-221: 🎯 Functional Correctness | 🟠 MajorMissing Scenario 8: "Planned switchover stuck."
The document jumps from Scenario 7 (lines 169–219) directly to Scenario 9 (line 221). Scenario 8 ("Planned switchover stuck"), which is referenced in Scenario 3 (line 112), appears to be absent from the runbook. The line-range change details describe Scenario 8 content for lines 195–219, but those lines in the annotated code are the tail of Scenario 7.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/dr/runbook/index.md` around lines 195 - 221, Add the missing “Planned switchover stuck (target not catching up)” scenario as its own Scenario 8 section in the runbook, since Scenario 3 references it and the current flow skips from Scenario 7 to Scenario 9. Move or insert the switchover troubleshooting content under a dedicated Scenario 8 heading in the markdown structure, and keep Scenario 9 immediately after it so the numbering and cross-references from the runbook stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guides/postgres/README.md`:
- Around line 98-100: The markdown in the Cross-DC Disaster Recovery section
uses non-descriptive link text with Follow [here], which hurts accessibility and
clarity. Update the link in the PostgreSQL guide README to use descriptive text
that names the destination content, and keep the existing target pointing to the
DR overview path. Use the surrounding section title and the link itself as the
location cue when editing.
---
Outside diff comments:
In `@docs/guides/postgres/dr/runbook/index.md`:
- Around line 112-266: The runbook is missing the actual sections for Scenario 8
and Scenario 10, even though Scenario 8 is referenced and the content for the
fence-tripped case is present without a matching heading. Restore the missing
numbered sections in the Postgres DR runbook so the sequence runs from Scenario
7 to 11 without gaps, placing the “Planned switchover stuck” guidance under the
Scenario 8 heading and the “A DC is unexpectedly read-only (fence tripped)”
guidance under the Scenario 10 heading, with the section titles and
diagnosis/action subsections aligned to the existing runbook structure.
- Around line 195-221: Add the missing “Planned switchover stuck (target not
catching up)” scenario as its own Scenario 8 section in the runbook, since
Scenario 3 references it and the current flow skips from Scenario 7 to Scenario
9. Move or insert the switchover troubleshooting content under a dedicated
Scenario 8 heading in the markdown structure, and keep Scenario 9 immediately
after it so the numbering and cross-references from the runbook stay consistent.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b10c82f5-fd55-4966-b5be-8d3f0c596e52
📒 Files selected for processing (14)
docs/guides/postgres/README.mddocs/guides/postgres/dr/_index.mddocs/guides/postgres/dr/guide/index.mddocs/guides/postgres/dr/overview/index.mddocs/guides/postgres/dr/runbook/index.mddocs/guides/postgres/pitr/archiver.mddocs/guides/postgres/remote-replica/_index.mddocs/guides/postgres/remote-replica/remotereplica.mddocs/guides/postgres/remote-replica/yamls/pg-ingres.yamldocs/guides/postgres/remote-replica/yamls/pg-issuer.yamldocs/guides/postgres/remote-replica/yamls/pg-london-auth.yamldocs/guides/postgres/remote-replica/yamls/pg-london.yamldocs/guides/postgres/remote-replica/yamls/pg-singapore-auth.yamldocs/guides/postgres/remote-replica/yamls/pg-singapore.yaml
💤 Files with no reviewable changes (8)
- docs/guides/postgres/remote-replica/yamls/pg-london.yaml
- docs/guides/postgres/remote-replica/yamls/pg-singapore-auth.yaml
- docs/guides/postgres/remote-replica/yamls/pg-issuer.yaml
- docs/guides/postgres/remote-replica/yamls/pg-singapore.yaml
- docs/guides/postgres/remote-replica/_index.md
- docs/guides/postgres/remote-replica/remotereplica.md
- docs/guides/postgres/remote-replica/yamls/pg-ingres.yaml
- docs/guides/postgres/remote-replica/yamls/pg-london-auth.yaml
✅ Files skipped from review due to trivial changes (2)
- docs/guides/postgres/dr/_index.md
- docs/guides/postgres/pitr/archiver.md
Comment on lines
+98
to
+100
| ### Cross-DC Disaster Recovery (DC-DR) | ||
|
|
||
| Do you want to have a backup data center where you want to run your postgresql database to recover from a data center failure as soon as possible? | ||
|
|
||
| Follow [here](/docs/guides/postgres/remote-replica/remotereplica.md). | ||
| Do you want to run your PostgreSQL database across multiple data centers and recover from a full data center failure with a single, automatically failing-over endpoint? Follow [here](/docs/guides/postgres/dr/overview/index.md). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use descriptive link text.
"Follow [here]" is non-descriptive link text that hurts accessibility and navigation clarity. Replace with something that describes the destination.
-Do you want to run your PostgreSQL database across multiple data centers and recover from a full data center failure with a single, automatically failing-over endpoint? Follow [here](/docs/guides/postgres/dr/overview/index.md).
+Do you want to run your PostgreSQL database across multiple data centers and recover from a full data center failure with a single, automatically failing-over endpoint? Follow the [Cross-DC Disaster Recovery guide](/docs/guides/postgres/dr/overview/index.md).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### Cross-DC Disaster Recovery (DC-DR) | |
| Do you want to have a backup data center where you want to run your postgresql database to recover from a data center failure as soon as possible? | |
| Follow [here](/docs/guides/postgres/remote-replica/remotereplica.md). | |
| Do you want to run your PostgreSQL database across multiple data centers and recover from a full data center failure with a single, automatically failing-over endpoint? Follow [here](/docs/guides/postgres/dr/overview/index.md). | |
| ### Cross-DC Disaster Recovery (DC-DR) | |
| Do you want to run your PostgreSQL database across multiple data centers and recover from a full data center failure with a single, automatically failing-over endpoint? Follow the [Cross-DC Disaster Recovery guide](/docs/guides/postgres/dr/overview/index.md). |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 100-100: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/guides/postgres/README.md` around lines 98 - 100, The markdown in the
Cross-DC Disaster Recovery section uses non-descriptive link text with Follow
[here], which hurts accessibility and clarity. Update the link in the PostgreSQL
guide README to use descriptive text that names the destination content, and
keep the existing target pointing to the DR overview path. Use the surrounding
section title and the link itself as the location cue when editing.
Source: Linters/SAST tools
The design specifies that for Postgres the engine-free tie-breaker (witness) data center is vote-only and is declared with role: Arbiter and empty replicaIndices. The petset Witness role is reserved for engines whose witness must carry data (e.g. MongoDB) and is not used by Postgres. Correct the role in every example and the roles table (Arbiter = vote-only, the Postgres witness DC; Witness = data-bearing, not used by Postgres), and resolve a self-contradictory bullet that described Witness as both data-bearing and index-less. The operator is role-agnostic (it keys off empty replicaIndices), so no code change is needed. Signed-off-by: Tamal Saha <tamal@appscode.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/guides/postgres/dr/overview/index.md (2)
65-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCapitalize "PetSet" for consistency.
The documentation uses "PetSet" elsewhere (e.g., guide lines 148-149) but "petset" here. Standardize to the proper capitalized form.
- and empty `replicaIndices`. The petset `Witness` role is reserved for engines whose + and empty `replicaIndices`. The PetSet `Witness` role is reserved for engines whose🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/dr/overview/index.md` around lines 65 - 83, Standardize the terminology in the Postgres DR overview by changing the lowercase “petset” reference in the Data center roles section to the capitalized “PetSet” used elsewhere. Update the explanatory paragraph near the `PlacementPolicy` / `distributionRule.role` description so the `Witness` role sentence matches the existing `PetSet` naming convention.
107-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCapitalize "PetSet" for consistency.
The documentation uses "PetSet" elsewhere (guide lines 148-149, overview line 265) but "petset" here. Standardize to the proper capitalized form.
- (vote only, no Postgres) carries none. (The petset `Witness` role, a data-bearing + (vote only, no Postgres) carries none. (The PetSet `Witness` role, a data-bearing🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/dr/overview/index.md` around lines 107 - 110, The docs snippet uses the lowercase “petset” while the rest of the guide uses “PetSet”; update the wording in the overview content to use the same capitalized form for consistency. Make the change in the text associated with the postgres DR overview section so the terminology matches the existing “PetSet” references elsewhere in the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/guides/postgres/dr/overview/index.md`:
- Around line 65-83: Standardize the terminology in the Postgres DR overview by
changing the lowercase “petset” reference in the Data center roles section to
the capitalized “PetSet” used elsewhere. Update the explanatory paragraph near
the `PlacementPolicy` / `distributionRule.role` description so the `Witness`
role sentence matches the existing `PetSet` naming convention.
- Around line 107-110: The docs snippet uses the lowercase “petset” while the
rest of the guide uses “PetSet”; update the wording in the overview content to
use the same capitalized form for consistency. Make the change in the text
associated with the postgres DR overview section so the terminology matches the
existing “PetSet” references elsewhere in the document.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e018b383-5610-4286-acf7-762473161085
📒 Files selected for processing (2)
docs/guides/postgres/dr/guide/index.mddocs/guides/postgres/dr/overview/index.md
Match the docs to the DC-DR code on the dc-dr branches: - dataCenters[].role is the PlacementPolicy role (Member/Arbiter/Witness), set from the distribution rule in observeDataCenters, not primary/standby. Fix the status example and the field table; the writable field is what marks the active DC. Add the Arbiter DC to the example. - lagBytes is computed hub-side from the active primary's pg_stat_replication (one read of the writable endpoint), not by the coordinator with the hub never opening SQL. Note that a standby DC coordinator also stamps kubedb.com/dc-lag-bytes on its leader pod. - healthy comes from the per-DC dr-controlplane health Lease (dc-health-<dc>) freshness, not a ready-pod check. - runbook: dataCenters[].name -> dataCenters[].clusterName (the real field). Signed-off-by: Tamal Saha <tamal@appscode.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/guides/postgres/dr/guide/index.md (2)
53-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the array field path here.
distributionRule.clusterNamedoesn’t match the schema shown below (distributionRules[]), so this contract text points readers at a non-existent field path.♻️ Suggested fix
- the `PlacementPolicy` `distributionRule.clusterName` + the `PlacementPolicy` `distributionRules[].clusterName`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/dr/guide/index.md` around lines 53 - 61, The contract text in the guide uses an incorrect PlacementPolicy field path, since the schema references distributionRules[] rather than distributionRule.clusterName. Update the wording in this section to point readers to the array field path used by the actual PlacementPolicy schema, and keep the rest of the identifier list unchanged; this should be fixed in the same guide content that mentions PlacementPolicy and the data center identity fields.
364-365: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestate the full fence invariant here.
This short form drops the clock-skew term, so it weakens the actual safety rule and can mislead operators into choosing an unsafe TTL.
♻️ Suggested fix
- do not set a fence TTL that meets or exceeds the Lease duration. + keep `fence TTL + cross-DC clock skew < primary-DC Lease duration`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/postgres/dr/guide/index.md` around lines 364 - 365, The fence TTL warning in the DR guide is too abbreviated and omits the clock-skew constraint, so restate the full fence invariant in the surrounding text near the lease/fence explanation. Update the prose around the fence TTL guidance to include the complete safety rule from this section, using the same terminology as the lease invariant and fence logic, so readers understand the TTL must stay below the full threshold including clock skew.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/guides/postgres/dr/guide/index.md`:
- Around line 53-61: The contract text in the guide uses an incorrect
PlacementPolicy field path, since the schema references distributionRules[]
rather than distributionRule.clusterName. Update the wording in this section to
point readers to the array field path used by the actual PlacementPolicy schema,
and keep the rest of the identifier list unchanged; this should be fixed in the
same guide content that mentions PlacementPolicy and the data center identity
fields.
- Around line 364-365: The fence TTL warning in the DR guide is too abbreviated
and omits the clock-skew constraint, so restate the full fence invariant in the
surrounding text near the lease/fence explanation. Update the prose around the
fence TTL guidance to include the complete safety rule from this section, using
the same terminology as the lease invariant and fence logic, so readers
understand the TTL must stay below the full threshold including clock skew.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b546d8e6-6962-4f13-9b95-412b1020232a
📒 Files selected for processing (3)
docs/guides/postgres/dr/guide/index.mddocs/guides/postgres/dr/overview/index.mddocs/guides/postgres/dr/runbook/index.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/guides/postgres/dr/runbook/index.md
- docs/guides/postgres/dr/overview/index.md
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
1 participant
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.
Adds a top-level DR section for Postgres at
guides/postgres/dr, documenting the cross data center disaster recovery feature end to end, and removes the superseded remote-replica docs.Adds
guides/postgres/dr(a top-level "DR" section)dr/overview) — architecture and quick start: the core rule (Postgres raft never stretches across DCs), the primary-DC Lease as the single cross-DC authority, the fail-closed local fence, leader-to-leader streaming, DC roles (Member/Arbiter/Witness), deploy/observe/switchover/scale.dr/guide) — every aspect of running in DR mode: components and the marker contract, the DC-name contract, operator--dc-dr-*flags, deployment, connecting, monitoring (status.disasterRecovery, the marker, thedc-lag-bytesannotation, role labels), replication/lag/RPO, the timing invariant (fence TTL + skew < Lease duration) and tuning, quorum/arbiters, planned switchover (zero-RPO), failback, per-DC horizontal scaling, day-2 ops, backup, deletion, and limitations.dr/runbook) — scenario-by-scenario procedures (active DC lost, partition, switchover/failback, standby DC lost, coordination plane down, stuck failover/switchover, growing lag, fence tripped, re-adding a DC, scaling, upgrade, suspected split-brain), each with symptoms → automatic behavior → verify → action, plus a quick reference and escalation checklist.The DR docs build on Distributed Postgres (that link is preserved); they live in their own top-level section rather than nested under "Distributed".
Removes the Postgres remote-replica docs
The DR docs are the supported multi-data-center story, so this removes
guides/postgres/remote-replica(the page and its yamls), repoints the README's DR entry to the new DR overview, and fixes a stale yaml-folder note inpitr/archiver.md(it referenced the remote-replica yamls but actually usespitr/yamls).YAML examples were checked against the real
PlacementPolicy/Postgres/PostgresOpsRequestshapes; all front-matter and code blocks parse, and the menu hierarchy is consistent. Documents the feature implemented across the apimachinery, petset, dr-controlplane, pg-coordinator, and postgres operator PRs.Summary by CodeRabbit