-
Notifications
You must be signed in to change notification settings - Fork 873
Implement the PostgreSQL Streaming Replication Protocol #3281
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
Merged
Merged
Changes from all commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
bc8c54e
Add NpgsqlLogSequenceNumber and PgLsnHandler to handle the pg_lsn type
Brar 73cdc62
Convert timestamp and timestamp with time zone directly to DateTime
Brar 8cdacfa
Implement the PostgreSQL Streaming Replication Protocol
Brar 53e75ff
Disable feedback timers when cancelling
Brar 8e49da7
Remove [PublicAPI]
roji 8eb4de6
Update test/Npgsql.Tests/Replication/SafeReplicationTestBase.cs
roji f44caa7
Update src/Npgsql/Replication/NpgsqlReplicationConnection.cs
roji 72f04b4
Small fixes and nits
roji 62e25cf
Simplify some test names
roji d31df95
Tweak name setup in tests to use [CallMemberName] etc.
roji dbf3e36
DRY some PG version checks in tests
roji 71d4e55
Add some missing cancellation token checks
roji baf6c85
Update src/Npgsql/Replication/Logical/Internal/NpgsqlLogicalReplicati…
roji 99a3e4f
Remove some needed cancellation token checks
roji 4aae8c4
Cleanup of LSN
roji 781ee73
Make cancellation token mandatory for StartReplication
roji c760050
Ignore ReplicationSurvivesPausesLongerThanWalSenderTimeout except on CI
roji 1147c74
Add some more PG config checks for skipping tests that would otherwis…
roji 2399af0
Correct mess done in last commit
roji 1ae6d3b
Refactor StartReplication methods
roji 6848312
Read from IAsyncEnumerable directly in tests
roji 6cf84ef
Clean up state management mechanism
Brar 9a3a37e
Fix truncate PG version checks in tests being a bit too DRY
Brar bb60011
Fix TimelineHistoryNonExisting test for PG10 physical replication
Brar c64cbf7
Clean up some leftover stuff
Brar b0b2329
Tiny assertion tweak
roji ebed714
Make null-terminated string reading more robust
roji a39b6a6
Make some methods local
roji 4825c12
And another local method
roji 1b14b5f
Refactor namespaces and type names
roji a3e965a
Recycle logical replication messages
roji 03d6a95
Slight simplification in TestDecoding
roji 461ea05
Recycle XLogData too
roji 15aec30
Handle all feedback locking inside SendFeedback
roji 6b09f21
Refactor command creation
roji b0360b3
Recycle test_decoding message as well
roji 89f32f5
Fix tiny namespace confusion
roji 873fc42
Fixup
roji 1908979
Remove Npgsql prefix everywhere
roji f53b581
Add constructors with connection strings to connections
roji 4c1da9f
Fixup
roji 416d414
Fix StringBuilder.AppendJoin() missing in .NET Standard 2.0
Brar 3be4e28
And remove another unused thing
roji a3ed79e
Revert "And remove another unused thing"
roji c781f9d
Timer concurrency logic
roji 110ec52
Disable cancel-on-dispose for now
roji 38d2238
Implement proper cancel-on-dispose
roji 6dd1fc3
Fix up some silliness
roji 324fc8b
Provide more information when cancellation fails in tests
roji 744e7b9
Work on logical plugin options
roji cba3374
Rename OpenAsync to Open
roji 405f0e2
Actually rename OpenAsync to Open
roji 3226d14
Use the regular user action mechanism for state management
roji faf49e6
Make physical replication tests explicit for now
roji 6afa695
Skip empty transactions in tests
roji 43fbc68
Add @Brar as replication code owner
roji File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| * @roji @YohDeadfall | ||
| * @roji @YohDeadfall | ||
| /src/Npgsql/Replication/ @Brar |
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
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
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.
Maybe a bit late, but I think this code will fail if the split is in the middle of a unicode character that uses more than 1 byte to represent. Better
Ensurethe max length of a string is already present in the buffer (as it was previously), or create a temporary byte buffer with all the data which you then convert to a string in one go using theencoding.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.
You're technically right... However, note that this is for reading null-terminated strings, which AFAIK are only used for certain short ASCII control strings, rather than actual user data - so I doubt there's a real bug here. But you can open a new issue to at least have it tracked.
BTW if we do want to preemptively fix this, then rather than entirely buffering the strings them in a temporary buffer, we can use System.Text.Decoder to incrementally load more chunks a decode. Note that we haven't even done this optimization for actual user strings (which are probably much more important, see TextHandler.ReadLong).
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.
At least in the replication protocol they are used for user data as well, for example table names, which can contain non-ascii characters.
I don't see why System.Text.Decoder would be anything better. It gives you some
chars on every iteration. These chars are temporary anyway and must then be made into a string, after which the previously buffered chars are discarded. But generally a char[] takes up more memory than the corresponding byte[] in UTF8, so I think it's better to just buffer everything as bytes. The encoder can then convert these to a string in-place, with no extra temporary space.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.
You're right... IIRC that's why I didn't do it in TextHandler either.
Open an issue for us to fix this?