Replaced JSON & strings with bincode

[Dherse]

Update to message and packet content

This PR changes the way messages are sent between nodes, it replaces the usage of JSON and String-based payload with bincode based payloads. The ultimate goal of this change is to allow bastion to exchange serialized messages instead of text between cluster members.

Impact

This fairly simple change has a few positive and negative impacts:

The pros

• the new serialization format is more efficient and should lead to smaller packet sizes (~60% according to these benchmarks and this article)
• allows sending packets that aren't text-based
• using bincode, it should be faster than JSON (some x3.5 faster according to this article and x2.5 faster according to these benchmarks))
• because it's still using serde, switching the (de)serialization backend should be very easy and could be done with features

The cons

• this loses observability, the content of the packet is now a binary data
• the single choice of bincode may not be to the taste of everybody, letting users select the format may be of interest

Tests

Currently, I have not written additional tests for this, however, with the additional work I have done on bastion and the showcase repos (forks on my account), it works as expected. And existing tests seem to run fine.

User facing changes

The breaking changes that appear are the following:

• ArtilleryMemberEvent::Payload now contains a Vec<u8> instead of a String
• Cluster::send_payload now enforces a trait bound of Serialize instead of AsRef<str>

This changes do have consequences for people using these features as they completely change the responsability of the user. Instead of having to "prepare"/serialize the data externally, it is now done internally. I belive this change is logical as serde is a very widely used crate. However, this could be changed by adding a Cluster::send_raw_payload that takes a Vec<u8> instead of a message and sends that instead.

Checklist

• [x] tests are passing with cargo test.
• [ ] tests and/or benchmarks are included
• [x] documentation is changed or added
• [x] commit message is clear

[o0Ignition0o]

This is looking good! Is there a chance we can use Bytes instead of Vec ?

[Dherse]

This is looking good! Is there a chance we can use Bytes instead of Vec ?

I think this should be doable. Ideally, we wouldn't need to allocate that Vec or any other underlying data structure but I am unsure how one would go about implementing that. I will have a look at it when I get back home :)

My understanding is that Bytes (I am assuming from the bytes crate) should implement Read & Write. Worst case scenario, a simple shim can be written on top of it to implement that.

[Dherse]

First and foremost, sorry for taking so long, I only had time to look at it today in the end.

I have done a couple of commits changing the implementation and here are a few takeaways:

• I don't think this will improve performance/memory allocation behaviour: since we can't use the "Arc-iness" of the Bytes struct we don't really benefit from the re-used allocations it can provide but we're still punished by its overhead (reference counting). Taking advantage of this would be a lot of work.
• Serde doesn't take advantage of bytes: since serde can't use the "Arc-iness" of Bytes, we still need to allocate new Bytes struct when deserializing. Therefore, spending time implementing the use of slices instead of Bytes wherever possible (or potentially deserializing the limited number of inter-cluster messages manually to use it may be worth it).
• bytes doesn't offer any feature that can be immediately used in the code. I don't doubt that in the future this can be done but not in the immediate.

Therefore, the only way of making bytes worth it would be to do some manual (de)serializing and using bytes through there allowing more re-use.

Tell me what you think, this can be things that are worked on but I'm fairly limited on time lately :(

[o0Ignition0o]

Hey, no worries at all really, this is OSS and we do it when we have time, no pressure at all :)

Let's keep it as is, i m ready to review as soon as it looks good to you :)

[vertexclique]

I am interested in this. FYI. Looking for a gated bincode impl.

[Dherse]

I am interested in this. FYI. Looking for a gated bincode impl.

Hey, I am sorry for not coming back to this sooner, I had been busy and I should have finished this instead of letting this inactive for months. For this I apologize. I am currently on holiday and I will try and do two things:

• Update it to the latest artillery commits
• I believe there is some unsoundness in my initial implementation and I would like to go back over it
• Finally, I couldn't hastily implement support for bytes in a satisfactory way, but I believe there is a way of doing it properly, and I will be looking it all over again next week.

Again, sorry for the delay, I hope to finish this in a timely matter to both improve artillery and finish the work I started.