Skip to content

fix: sync event refresh blocking updates when Spock node is not configured#273

Merged
tsivaprasad merged 2 commits intomainfrom
PLAT-420-refresh-lifecycle-methods-in-sync-event-wait-for-sync-event-resources-can-block-updates
Feb 21, 2026
Merged

fix: sync event refresh blocking updates when Spock node is not configured#273
tsivaprasad merged 2 commits intomainfrom
PLAT-420-refresh-lifecycle-methods-in-sync-event-wait-for-sync-event-resources-can-block-updates

Conversation

@tsivaprasad
Copy link
Contributor

@tsivaprasad tsivaprasad commented Feb 20, 2026

Summary

This PR fixes hard errors returned by SyncEventResource and WaitForSyncEventResource refresh methods when the database is not configured as a Spock node, which previously blocked follow-up update operations.

Changes

  • Return resource.ErrNotFound from SyncEventResource.Refresh when spock.sync_event() fails
  • Return resource.ErrNotFound from WaitForSyncEventResource.Refresh when spock.sub_show_status() or spock.wait_for_sync_event() fails
  • Treat an empty SyncEventLsn as ErrNotFound instead of a hard error
  • Add postgres.IsSpockNodeNotConfigured helper to detect object_not_in_prerequisite_state
  • Update NOTICE.txt for the new github.com/jackc/pgerrcode dependency

Testing

Verification:

  1. Created a 3 node cluster
  2. Created a database with single node
  3. Connected to host and db and dropped spock node
select spock.node_drop('n1');
 node_drop 
-----------
 t
(1 row)
select * from spock.node;
 node_id | node_name | location | country | info 
---------+-----------+----------+---------+------
(0 rows)
  1. Updated a database with new node n2
"nodes": [
      {"name": "n1", "host_ids": ["host-1"]},
      {"name": "n2", "host_ids": ["host-2"]}
    ]
  1. Verified that node n2 is added to the database & spock.nodes table has both n1 and n2
 select * from spock.node;
 node_id | node_name | location | country | info 
---------+-----------+----------+---------+------
   49708 | n1        |          |         | 
   26863 | n2        |          |         | 
(2 rows)
  1. List databases and verify that n2 is added to the database
{
  databases: [
    {
      created_at: "2026-02-19T16:39:17Z"
      id: "storefront"
      instances: [
        {
          host_id: "host-1"
          id: "storefront-n1-689qacsi"
          node_name: "n1"
          state: "available"
        }
        {
          host_id: "host-2"
          id: "storefront-n2-9ptayhma"
          node_name: "n2"
          state: "available"
        }
      ]
      state: "available"
      updated_at: "2026-02-20T01:59:41Z"
    }
  ]
}

Checklist

PLAT-420

…gured

Add license entries for github.com/jackc/pgerrcode which was added
as a direct dependency in go.mod. The package has two licenses:
MIT (for the Go code) and PostgreSQL (for the embedded error code data).

Generated via: docker run --platform linux/amd64 golang:1.25rc2 with GOTOOLCHAIN=auto

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 20, 2026

📝 Walkthrough

Walkthrough

Adds third-party license notices, introduces a postgres error helper to detect unconfigured Spock nodes with tests, updates database resource error handling to return standardized not-found errors, adds a changelog entry, and updates go.mod with an indirect dependency.

Changes

Cohort / File(s) Summary
License Notices
NOTICE.txt
Appends extensive third-party license and attribution texts for multiple dependencies (MIT, Apache-2.0, BSD-3-Clause, MPL-2.0, etc.).
Dependencies
go.mod
Adds indirect dependency github.com/jackc/pgerrcode v0.0.0-20250907135507-afb5586c32a6 and consolidates require lines into a grouped block.
Changelog
changes/unreleased/Fixed-20260220-191533.yaml
Adds an unreleased entry documenting a fix for sync_event and wait_for_sync_event blocking updates when the DB is not a Spock node.
Postgres Error Handling
server/internal/postgres/errors.go, server/internal/postgres/errors_test.go
Adds exported IsSpockNodeNotConfigured(error) bool to detect SQLSTATE 55000 and a table-driven test covering wrapped and non-wrapped PgError cases. Attention: new exported function and test coverage.
Database Resources
server/internal/database/sync_event_resource.go, server/internal/database/wait_for_sync_event_resource.go
Modifies error handling to return resource.ErrNotFound when Spock node is not configured; removes LSN-specific error formatting in one path. Review call sites for behavior expectations.

Poem

🐰 I dug through logs and license trails,
Found Spock nodes hiding in postgres vales,
I wrapped errors neat and chased the fright,
Pulled licenses close and set them right,
Hop — the code now sleeps soundly through the night.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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 PR title clearly and concisely summarizes the main change: fixing sync event refresh behavior when a Spock node is not configured, which is directly supported by the changeset.
Description check ✅ Passed The PR description comprehensively covers all required sections: Summary, Changes, Testing with detailed verification steps, Checklist with completed items, and issue linking.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch PLAT-420-refresh-lifecycle-methods-in-sync-event-wait-for-sync-event-resources-can-block-updates

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.

Copy link

@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: 3

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

Inline comments:
In `@go.mod`:
- Line 49: The go.mod entry for github.com/jackc/pgerrcode is marked "//
indirect" but it's actually imported directly in
server/internal/postgres/errors.go (the import of github.com/jackc/pgerrcode),
so remove the "// indirect" annotation and place that require line into the
direct-dependencies section of go.mod; after editing go.mod run "go mod tidy" to
update the module graph and ensure the dependency is recorded as direct.

In `@NOTICE.txt`:
- Around line 3976-3986: The second github.com/jackc/pgerrcode entry erroneously
labels the block as “[PostgreSQL]” while the content is the MIT license; locate
the duplicated github.com/jackc/pgerrcode block and either correct the SPDX
label to reflect MIT (change “[PostgreSQL]” to “[MIT]”) or, if you cannot alter
the tool output, insert a single-line explanatory HTML comment immediately above
the second block (e.g., <!-- The single LICENSE file covers both licenses; full
text repeated per tool output -->) so auditors understand the repetition is
intentional.

In `@server/internal/postgres/errors_test.go`:
- Around line 44-51: The test case titled "returns true when wrapped with
fmt.Errorf" doesn't actually wrap the PgError; update the err function to return
a wrapped error using fmt.Errorf with %w (e.g. create pgErr :=
&pgconn.PgError{Code: pgerrcode.ObjectNotInPrerequisiteState} then return
fmt.Errorf("wrapped: %w", pgErr)) so the test exercises errors.As chain
traversal used by IsSpockNodeNotConfigured; keep expected: true.

Comment on lines +3976 to +3986
## github.com/jackc/pgerrcode

* Name: github.com/jackc/pgerrcode
* Version: v0.0.0-20250907135507-afb5586c32a6
* License: [PostgreSQL](https://github.com/jackc/pgerrcode/blob/afb5586c32a6/LICENSE)

```
Copyright (c) 2019 Jack Christensen

MIT License

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Second pgerrcode entry is labeled [PostgreSQL] but its content block opens with MIT license text.

Lines 3976–4030 duplicate the first entry (lines 3920–3974) byte-for-byte — the content block starts with "MIT License" / "Copyright (c) 2019 Jack Christensen" under a header that advertises the PostgreSQL license. Anyone auditing the [PostgreSQL] section for compliance would immediately encounter the wrong license header.

This looks like a tool artifact where the generator repeats the full LICENSE file content under every detected SPDX identifier. If the tooling cannot be changed, a brief explanatory comment above the second block (e.g., <!-- The single LICENSE file covers both licenses; full text repeated per tool output -->) would reduce confusion during legal review.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@NOTICE.txt` around lines 3976 - 3986, The second github.com/jackc/pgerrcode
entry erroneously labels the block as “[PostgreSQL]” while the content is the
MIT license; locate the duplicated github.com/jackc/pgerrcode block and either
correct the SPDX label to reflect MIT (change “[PostgreSQL]” to “[MIT]”) or, if
you cannot alter the tool output, insert a single-line explanatory HTML comment
immediately above the second block (e.g., <!-- The single LICENSE file covers
both licenses; full text repeated per tool output -->) so auditors understand
the repetition is intentional.

Copy link
Member

@jason-lynch jason-lynch left a comment

Choose a reason for hiding this comment

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

Nice! I'd ignore the NOTICE.txt comment, but the other two code rabbit comments in errors_test.go and go.mod are valid. They're definitely minor issues, but could you please fix them up before you merge?

Copy link

@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.

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

Duplicate comments:
In `@go.mod`:
- Around line 47-50: The go.mod currently has github.com/gohugoio/hashstructure
listed as indirect while github.com/jackc/pgerrcode remains in the wrong block;
run `go mod tidy` to let Go reorder dependencies so pgerrcode moves into the
primary require block and hashstructure is marked as indirect, or manually move
github.com/jackc/pgerrcode into the main direct-deps require block and restore
github.com/gohugoio/hashstructure to an indirect entry (// indirect) so the file
follows standard direct/indirect grouping.

@tsivaprasad tsivaprasad merged commit a7e8a31 into main Feb 21, 2026
3 checks passed
@tsivaprasad tsivaprasad deleted the PLAT-420-refresh-lifecycle-methods-in-sync-event-wait-for-sync-event-resources-can-block-updates branch February 21, 2026 06:53
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