Consider encoding the version of message scheme in the preamble

[freeekanayaka]

In this commit of https://github.com/canonical/raft/pull/303 we started using only the first 2 bytes to read the message type out of a 64-bit slot in the message preamble, instead of using the full 8 bytes of the slot.

That was done in preparation of using the rest of the slot to store additional information, such has a version for the scheme of a specific message type.

I think sufficient time has now passed that we could start actually encoding the schema version in that space, and be confident that the node on the other side will either read it (and take it into account when decoding), or ignore it and assume that the version is 0.

Thoughts?

[cole-miller]

Would this just be a cleanup, or does it unlock some possibilities that the current message format prevents?

[freeekanayaka]

Currently we use the length of the message in bytes to evince its schema version. For example we do that for the RequestVote RPC, see here.

This works well for adding fields, because the total size of an encoded message whose type is using a newer schema version (with added fields) will be higher than the encoded size of the same message type at a lower version schema. If the receiver does not support the new fields it will just read up to the size it supports and mark the message as using the lower version schema. If the receiver does support the new fields, it can use the message size to decide whether the schema version is the higher one or the lower, and set the version field of the decoded message accordingly, along with the other fields.

However this technique is not enough when we want to change a schema version without changing the message size (e.g. replacing or repurposing a field, or similar things).

[freeekanayaka]

In general I think we should always only add fields, that makes everything easier, so the current approach should be enough. However there might be cases were can't or don't want to do that, and it's good to have a way out for those cases. The change in the PR that I mention was a preparation to be able to support this.

[cole-miller]

In general I think we should always only add fields, that makes everything easier, so the current approach should be enough.

Yeah, this is how I feel. I think we should revisit this change when we have a use-case for it.

[freeekanayaka]

In general I think we should always only add fields, that makes everything easier, so the current approach should be enough.

Yeah, this is how I feel. I think we should revisit this change when we have a use-case for it.

Yeah, however unless I'm missing something, the problem I see is that when we'll have a use case, we will not be able to use this system because it won't be in place, so we won't be able to meet the use case. I think that was the reason to start putting it in place in the first place, to allow for future changes.

[cole-miller]

Ah, I see, because we want the support to be in place for a while before making use of it, so that we're justified in assuming that all servers in the cluster will have it?

[freeekanayaka]

Ah, I see, because we want the support to be in place for a while before making use of it, so that we're justified in assuming that all servers in the cluster will have it?

Exactly.