Merged
Conversation
|
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)
📝 WalkthroughWalkthroughAdded a changelog entry for v1.9.0 and modified CDC replication handling: ensure the DB pool is closed on return and explicitly handle Changes
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/CHANGELOG.md (1)
21-23: Consider varying sentence structure for better readability.Three consecutive security update entries begin with "Upgraded", which slightly reduces readability.
📝 Optional style improvement
-- Upgraded `moby/buildkit` v0.27.1 → v0.28.1 (CVE-2026-33747, CVE-2026-33748). -- Upgraded `go.opentelemetry.io/otel/sdk` + exporters to v1.43.0 (CVE-2026-39882, CVE-2026-39883). -- Upgraded `google.golang.org/grpc` v1.79.1 → v1.80.0 (CVE-2026-33186). +- `moby/buildkit` upgraded v0.27.1 → v0.28.1 (CVE-2026-33747, CVE-2026-33748). +- `go.opentelemetry.io/otel/sdk` + exporters upgraded to v1.43.0 (CVE-2026-39882, CVE-2026-39883). +- `google.golang.org/grpc` upgraded v1.79.1 → v1.80.0 (CVE-2026-33186).Or use alternative verbs like "Bumped" or "Updated" for variation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/CHANGELOG.md` around lines 21 - 23, Change the three consecutive changelog bullets that all start with "Upgraded" to vary sentence starters for readability; for example, update the first bullet for `moby/buildkit` to "Bumped", the second for `go.opentelemetry.io/otel/sdk` to "Updated", and keep or change the third for `google.golang.org/grpc` to a different verb like "Upgraded" or "Bumped" so each package line (`moby/buildkit`, `go.opentelemetry.io/otel/sdk`, `google.golang.org/grpc`) uses a distinct leading verb.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/CHANGELOG.md`:
- Around line 21-23: Change the three consecutive changelog bullets that all
start with "Upgraded" to vary sentence starters for readability; for example,
update the first bullet for `moby/buildkit` to "Bumped", the second for
`go.opentelemetry.io/otel/sdk` to "Updated", and keep or change the third for
`google.golang.org/grpc` to a different verb like "Upgraded" or "Bumped" so each
package line (`moby/buildkit`, `go.opentelemetry.io/otel/sdk`,
`google.golang.org/grpc`) uses a distinct leading verb.
processReplicationStream fell through to the default case on *pgproto3.ErrorResponse, logging only the Go type and continuing the receive loop. The next ReceiveMessage would then time out and the code would falsely report "Replication stream drained" — so UpdateFromCDC returned nil, callers assumed success, and metadata was updated with an LSN that hadn't actually advanced. Add an explicit case for *pgproto3.ErrorResponse that logs every server diagnostic field (severity, code, message, detail, hint, where, routine) and treats the event as fatal: sets processingErr, stops streaming, and returns the SQLSTATE + message to the caller as a real error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…leak processReplicationStream acquired a pgxpool via auth.GetClusterNodeConnection on entry and never closed it — no defer, no cleanup on any return path. Every call to UpdateFromCDC or ListenForChanges leaked one pool, and each leaked pool retains a backgroundHealthCheck goroutine plus its connections. Under repeated invocations (e.g. the CDC regression tests with -count=10) leaked connections accumulated until Postgres refused new clients with "sorry, too many clients already" (SQLSTATE 53300). Add the defer immediately after the pool acquisition so all return paths release it, including the long-running continuous ListenForChanges case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
No description provided.