-
Notifications
You must be signed in to change notification settings - Fork 39
feat: Add Logical Replication Tests for v3 #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #121 +/- ##
==========================================
- Coverage 87.42% 87.28% -0.15%
==========================================
Files 29 29
Lines 2498 2500 +2
==========================================
- Hits 2184 2182 -2
- Misses 314 318 +4
☔ View full report in Codecov by Sentry. |
@osaxma: could you please rebase this PR now? |
@isoos The tests that I added aren't passing locally (by the way, this PR only add these tests and nothing else) I'll try to fix it today. |
a620c80
to
f20bac1
Compare
case CopyBothResponseMessage(): | ||
// This message indicates a successful start for Streaming Replication. | ||
// Hence, in this context, the query is complete. And from here on, | ||
// the server will be streaming replication messages. | ||
// But if the connection was used after this point to execute further | ||
// queries, the server messages will be blocked. | ||
// TODO(osaxma): Prevent executing queries when Streaming Replication | ||
// is ongoing | ||
await _completeQuery(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a temporary special case for Streaming Replication since CopyBothResponseMessage
is only used for that. Ideally, we should stop any queries from being executed after this point to prevent blocking the server messages.
I will try updating the example repository for Steaming Replication to investigate what could go wrong and how to better handle this.
CopyBothResponseMessage
case in message handler