switch interserver protocol to use postcard#922
Conversation
|
||||||||||||||||||
|
Test Coverage: 81.26% |
There was a problem hiding this comment.
I know we discussed on the call and said that since we serialize the operations to log, and we pass logs here, the concerns raised by this PR are already the case, no? (we already have the danger around operations).
Maybe worth updating the PR description with this information?
There was a problem hiding this comment.
Maybe a better name than postcard_? (as in, the trailing _)
| let req = self | ||
| .build_request("/repl/raft/vote", rpc) | ||
| .with_timeout(option.soft_ttl()) | ||
| .with_format(Format::Msgpack); |
There was a problem hiding this comment.
Why is this one using msgpack and others postcard?
There was a problem hiding this comment.
all the things that are low-volume use msgpack (it's the default; this one just calls it out explicitly because it's also overriding the TTL)
|
We've run the experiment and learned what we wanted to learn, right? Can be closed? |
3 participants
FWIW, I don't think the proposal in svix/rfc#54 will work very well for interserver messages (since they contain the union of all modules' operation messages). Actually doing this correctly will be rather hard (because, e.g., the "version" of an AppendEntries message is going to be the union of all of the versions of the underlying requests). Right now, this will just make the cluster break extremely un-gracefully if we ever change any operation definition. It's also not meaningfully faster, since the actual network time dominates the msgpack time. Anywho, here it is as requested.