feat(resharding): use binary protocol for replication#1016
Conversation
meskill
force-pushed
the
feat/replication-binary
branch
from
36b0194 to
a025a14
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
levkk
left a comment
levkk
left a comment
There was a problem hiding this comment.
Cool! It should be a bit faster but maybe the cost of decoding the WAL is too high to matter.
| let query = Query::new(format!( | ||
| r#"START_REPLICATION SLOT "{}" LOGICAL {} ("proto_version" '4', origin 'any', "publication_names" '"{}"')"#, | ||
| self.name, self.lsn, self.publication | ||
| r#"START_REPLICATION SLOT "{}" LOGICAL {} ("proto_version" '4', origin 'any', "publication_names" '"{}"', "binary" '{}')"#, |
There was a problem hiding this comment.
I know we're not passing user input, but seeing format! instead of prepared statements still makes me wince 😅
There was a problem hiding this comment.
The logical replication protocol doesn't support prepared statements.
There was a problem hiding this comment.
Looking at it again I don't even know why my brain parsed this as a SQL query.
| .collect::<Vec<_>>(); | ||
| Bind::new_params(name, ¶ms) | ||
| .unzip(); | ||
| Bind::new_params_codes(name, ¶ms, &codes) |
There was a problem hiding this comment.
Not entirely relevant to this PR since you didn't add this function, but this smells like we should use a builder
|
Speaking of sync streaming ...we should definitely work on that - push queries without waiting for replies. There isn't any downside afaik, and we can always error-out if we eventually receive an error. Basically have two tokio task loops run side by side:
I bet this would make replication super quick. |
3 participants
Use binary protocol for replication slot if
resharding_copy_formatset to binary.Performance benefits are minimal, guess mostly because of the current sequential streaming handling in replication (send -> wait for reply for every event).
I managed to get some improvements on benchmarks only with toxiproxy in case of limited bandwidth
otherwise no difference