Skip to content

fix: add replication slot resources in migration#351

Merged
jason-lynch merged 1 commit intomainfrom
fix/PLAT-537/create-replication-slot-in-migration
Apr 21, 2026
Merged

fix: add replication slot resources in migration#351
jason-lynch merged 1 commit intomainfrom
fix/PLAT-537/create-replication-slot-in-migration

Conversation

@jason-lynch
Copy link
Copy Markdown
Member

@jason-lynch jason-lynch commented Apr 17, 2026

Summary

The replication slot resource wasn't added until v0.7.0, so databases created in versions <= v0.6.2 will be missing this resource. With this change, the migration now adds replication slot resources if they're missing in addition to upgrading any existing resources.

Changes

  • Regenerated server/internal/resource/migrations/schemas/v1_0_0/database.go by running:
go run -C ./server/internal/resource/migrations/schematool . -repo $(pwd) -package v1_0_0 main server/internal/database \
    InstanceResource \
    LagTrackerCommitTimestampResource \
    PostgresDatabaseResource \
    ReplicationSlotAdvanceFromCTSResource \
    ReplicationSlotCreateResource \
    ReplicationSlotResource \
    SubscriptionResource \
    SyncEventResource \
    WaitForSyncEventResource \
    > ./server/internal/resource/migrations/schemas/v1_0_0/database.go
  • Updated the state v1.0.0 migration to incorporate the new DatabaseID field on PostgresDatabaseResource
  • Updated the state v1.0.0 migration to add replication slot resources when they don't exist

Testing

# This patch re-adds the old IP address setting to make this template work with
# v0.6.2.
git apply <<EOF
diff --git a/e2e/fixtures/roles/deploy_control_plane/templates/stack.yaml.tmpl b/e2e/fixtures/roles/deploy_control_plane/templates/stack.yaml.tmpl
index 05cb104b..47c88d75 100644
--- a/e2e/fixtures/roles/deploy_control_plane/templates/stack.yaml.tmpl
+++ b/e2e/fixtures/roles/deploy_control_plane/templates/stack.yaml.tmpl
@@ -5,6 +5,7 @@ services:
     image: {{ control_plane_image }}
     command: run
     environment:
+      - PGEDGE_IPV4_ADDRESS={{ hostvars[host].peer_ip }}
       - PGEDGE_PEER_ADDRESSES={{ hostvars[host].peer_ip }}
       - PGEDGE_CLIENT_ADDRESSES={{ hostvars[host].external_ip }}
       - PGEDGE_HOST_ID={{ host }}
EOF

# Reset and deploy v0.6.2 to the lima fixture
make reset-lima-fixture FIXTURE_CONTROL_PLANE_IMAGE=ghcr.io/pgedge/control-plane:v0.6.2

# Just to be safe, make sure the clocks are all correct on the fixture
ansible -i ./lima/inventory.yaml all -m command -a 'date'

# Use 'limactl restart <host ID>' to restart any of the machines that are out
# of sync, e.g. 'limactl restart host-1'

# Once the clocks are in sync, initialize the cluster.
cp-init

# Create a database with two nodes so that it has subscriptions. We can't
# pipe into cp-follow-task because of API changes, so copy the task ID from the
# output of this command.
cp1-req create-database <<EOF
{
  "id": "storefront",
  "spec": {
    "database_name": "storefront",
    "database_users": [
      {
        "username": "admin",
        "password": "password",
        "db_owner": true,
        "attributes": ["SUPERUSER", "LOGIN"]
      }
    ],
    "port": 0,
    "nodes": [
      { "name": "n1", "host_ids": ["host-1"] },
      { "name": "n2", "host_ids": ["host-2"] }
    ]
  }
}
EOF

# Use the task ID in this command to follow the task until it finishes
cp-follow-task -s database -e storefront -t <task ID>

# Deploy the changes from this branch. The state will update on startup.
make update-lima-fixture

# (Optional) You can find the log messages from the state update by SSH'ing to each
# server with `cpN-ssh`, e.g. `cp1-ssh`, and looking at the control plane server logs.
# Which server logs these will be random, but it should show something like:
# host-4-1  | 8:23PM INF running migration component=migration_runner migration=add_task_scope
# host-3-1  | 8:23PM INF upgraded state component=resource_service database_id=storefront to_version=1.0.0

# Add a node to the database to verify that the state is valid and follow the task until it completes
cp1-req update-database storefront <<EOF | cp-follow-task
{
  "id": "storefront",
  "spec": {
    "database_name": "storefront",
    "database_users": [
      {
        "username": "admin",
        "password": "password",
        "db_owner": true,
        "attributes": ["SUPERUSER", "LOGIN"]
      }
    ],
    "port": 0,
    "nodes": [
      { "name": "n1", "host_ids": ["host-1"] },
      { "name": "n2", "host_ids": ["host-2"] },
      { "name": "n3", "host_ids": ["host-3"] }
    ]
  }
}
EOF

Repeat the same test with v0.7.0 in place of v0.6.2.

PLAT-537

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 17, 2026

📝 Walkthrough

Walkthrough

The migration logic is extended to persist database_id in PostgreSQL database resources, add a helper to create replication slot resources from subscriptions, and update the subscription migration to conditionally create missing replication slot resources. Schema definitions are updated to include DatabaseID fields and new post-initialization workflow blocks.

Changes

Cohort / File(s) Summary
Migration Logic Enhancement
server/internal/resource/migrations/1_0_0.go
Added databaseID to migration parameters, created newReplicationSlotResource helper for building replication slot resources, and updated migrateSubscriptionResources to conditionally create missing replication slot resources during subscription migration.
Migration Test Coverage
server/internal/resource/migrations/1_0_0_test.go
Added new test case three nodes without slots to verify migration behavior when replication slot resources are absent, expanding dependency planning coverage.
Schema Updates
server/internal/resource/migrations/schemas/v1_0_0/database.go
Extended InstanceResource.Spec with AllHostIDs field and new optional PostInit block; added required DatabaseID field to PostgresDatabaseResource and new optional PostDatabaseCreate block with initialization metadata.
Golden Test Fixtures
server/internal/resource/migrations/golden_test/TestVersion_1_0_0/populate_n3_with_n1_source.json, single_node_with_replicas.json, three_nodes.json, three_nodes_without_slots.json, with_restore_config.json
Updated test fixtures to include database_id attribute in database.postgres_database resources; added comprehensive golden fixture for three-node scenario without pre-existing replication slots.

Poem

🐰 With databases now have IDs so clear,
And slots that auto-create without fear,
The schema grows with post-init grace,
While fixtures confirm each edge case's place!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately summarizes the main change: adding replication slot resources during migration, which is clear and specific.
Description check ✅ Passed The PR description includes a clear summary, detailed changes, comprehensive testing steps, and links to the related issue (PLAT-537). However, the checklist items are not explicitly filled out.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/PLAT-537/create-replication-slot-in-migration

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 17, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 6 duplication

Metric Results
Duplication 6

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/internal/resource/migrations/1_0_0.go`:
- Around line 380-389: The existence checks use Identifier.String() but the
slots map is keyed by Identifier.ID, so replace the .String() calls on
v0_0_0.ReplicationSlotResourceIdentifier(...) and
v1_0_0.ReplicationSlotResourceIdentifier(...) with their .ID values; i.e., look
up slots[...] using the Identifier.ID returned by those constructors instead of
.String() so the map lookups correctly detect existing replication slots and
avoid synthesizing duplicates.
🪄 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: 969af7f1-bbb4-42fa-9308-7243ea574de6

📥 Commits

Reviewing files that changed from the base of the PR and between 107121a and 57e93cc.

📒 Files selected for processing (8)
  • server/internal/resource/migrations/1_0_0.go
  • server/internal/resource/migrations/1_0_0_test.go
  • server/internal/resource/migrations/golden_test/TestVersion_1_0_0/populate_n3_with_n1_source.json
  • server/internal/resource/migrations/golden_test/TestVersion_1_0_0/single_node_with_replicas.json
  • server/internal/resource/migrations/golden_test/TestVersion_1_0_0/three_nodes.json
  • server/internal/resource/migrations/golden_test/TestVersion_1_0_0/three_nodes_without_slots.json
  • server/internal/resource/migrations/golden_test/TestVersion_1_0_0/with_restore_config.json
  • server/internal/resource/migrations/schemas/v1_0_0/database.go

Comment thread server/internal/resource/migrations/1_0_0.go
The replication slot resource wasn't added until v0.7.0, so databases
created in versions <= v0.6.2 will be missing this resource. With this
change, the migration now adds replication slot resources if they're
missing in addition to upgrading any existing resources.

PLAT-537
Copy link
Copy Markdown
Member

@mmols mmols left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested starting from both versions - worked for me.

@jason-lynch jason-lynch merged commit 2459ade into main Apr 21, 2026
3 checks passed
@jason-lynch jason-lynch deleted the fix/PLAT-537/create-replication-slot-in-migration branch April 21, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants