Skip to content

Conversation

osaxma
Copy link
Contributor

@osaxma osaxma commented Sep 9, 2022

This PR adds support for Streaming Replication Protocol.

The following repository contains an example usage: postgresql-dart-replication-example. While the protocol is intended for creating database replicas, the Logical Replication can be used to listen to changes in the database (e.g. inserts, updates, and deletes).

A summary of additions and changes:

  • Added VS Code IDE files to .gitignore
  • Raised the min sdk to 2.17.0 to use enhanced enums
    • I assume this could be treated as the only possible breaking change.
  • Added querySimple method
    • Similar to execute, but it'll return whatever data the server gives.
    • This is necessary since in streaming replication mode, the extended query protocol cannot be used. On the other hand, execute does not return any data. querySimple was introduced to avoid breaking the existing API and to allow querying the database in Replication Mode which is necessary (i.e. executing IDENTIFY_SYSTEM; and retrieving the system info data)
  • Added addMessage method to PostgreSQLConnection
    • I assumed since the socket is already publicly exposed, there's no harm of adding this method.
  • Added Stream<ServerMessage> messages to PostgreSQLConnection
    • Since socket is a single listener stream, there's no way to listen to server messages after opening the connection.
    • The implementation is similar to notifications except that it'll pass all messages to any listeners.
    • In combination with addMessage, the user can take over the communication with the server.
    • This is necessary to respond to server messages in replication mode as the connection will always be in a busy state (i.e. after running START_REPLICATION)
  • Added new messages types
    • New: shared_messages.dart and logical_replication_messages.dart
    • Also added few new messages to client_messages.dart and server_messages.dart
    • I feel all these messages files could be grouped in messages folder but I left that for review.
  • Modified MessageFramer.addBytes to handle new message types and special cases
    • tests were added as well
  • Fixed a parsing issue in CommandComplete
    • When executing IDENTIFY_SYSTEM;, the SYSTEM was parsed as the affected row integer
    • Replaced the RegExp for better handling (only looks for digits at the end of the string)

I believe these were the major additions with few changes.


About testing:
I added a test for logicl_replication_test.dart -- the test might need to be disabled for CI since the test restarts the docker container (necessary afterALTER SYSTEM) which could make any concurrent tests fail. Though I wanted to see how the CI reacts first.

I also had difficulty running the tests locally (before adding the test mentioned above). Many tests throw ConnectionError when ran concurrently but when I ran them individually, they pass (had to start docker container manually for those that don't call usePostgresDocker). Also, all the SSL tests failed.

I'm not sure if there's something that I'm missing on how I should run the tests.

This change introduces new messages types to support
the streaming protocol. The change also modifies
the framer to handle the new types of messages

Introduce `executeSimple` for simple queries

Unlike execute, this method returns data if there's
any; otherwise it returns affected rows.

Fix affected rows parsing in CommandCompleteMessage

When using non-standard command such as 'IDENTIFY_SYSTEM',
the parsing had an issue where it'll parse `SYSTEM`
as the integer. A new reliable regex was added to
parse the affected rows correctly if there's any
@isoos
Copy link
Owner

isoos commented Sep 9, 2022

@osaxma: thank you for working on this, I think this feature is a good thing to have here. However, the PR is huge, and I'd be more comfortable reviewing it smaller chunks. Do you have any suggestion how to split it up or restructure?

@osaxma
Copy link
Contributor Author

osaxma commented Sep 9, 2022

Sure -- I'll split them into multiple smaller PRs...

Tho adding all the new message types will be a huge PR nevertheless but I guess you'll notice that as the majority will be in new files.

@osaxma
Copy link
Contributor Author

osaxma commented Sep 9, 2022

@isoos
Copy link
Owner

isoos commented Sep 9, 2022

Thank you so much for the smaller-sized PRs, they are easy to follow and review so far, much less frightening than this one :)

@isoos
Copy link
Owner

isoos commented Sep 10, 2022

I guess that was about it? Let me know if there is anything else, otherwise I shall release the 2.5.0 with this great feature! :)

@osaxma
Copy link
Contributor Author

osaxma commented Sep 10, 2022

I guess that was about it? Let me know if there is anything else, otherwise I shall release the 2.5.0 with this great feature! :)

I guess it's time to close this one for sure.

Thanks a lot for your time in reviewing all my PRs in such a short period and maintaing this package.

It's funny because this whole thing was not planned at all -- I was just tinkering with Logical Replication until I found myself writing the entire functionality 😅

Cheers!

@osaxma osaxma closed this Sep 10, 2022
@osaxma osaxma deleted the replication branch September 10, 2022 14:42
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