Skip to content

multiple metadata messages in a single UDP packet#99

Draft
janscheres wants to merge 1 commit intoXTXMarkets:mainfrom
janscheres:main
Draft

multiple metadata messages in a single UDP packet#99
janscheres wants to merge 1 commit intoXTXMarkets:mainfrom
janscheres:main

Conversation

@janscheres
Copy link
Copy Markdown

Issue #27

I am not completely sure about the intended behaviour with regards to MTU but I have implemented what I understood from the issue.

So far I have just modified processRequests()

Still todo:

  • Modify cpp/shard/Shard.cpp to handle several messages in one packet
  • Modify cpp/cdc/CDC.cpp ^^
  • Modify parseResponse() and processRawResponse() ^^
  • Bump protocol

I will try get round to these when I have some time :)

Please let me know if you have any suggestions or if i made any mistakes.

@mcrnic
Copy link
Copy Markdown
Collaborator

mcrnic commented Feb 5, 2026

Ignore the CI checks, there is some issues with running CI on external contributions which we didn't address as there are not many.
I'll take a look at code tomorrow but your understanding from description seems correct

@janscheres
Copy link
Copy Markdown
Author

Am I understanding correctly that there is no way to know what the length of req.req.Pack would be (for example from the kind) ?

What I'm currently thinking is that I will have to add a length (I think uint16) to the header so a packet would now look like:
protocol + n*[requestid kind length(data) data]

So we can properly decode packets.

@mcrnic
Copy link
Copy Markdown
Collaborator

mcrnic commented Feb 6, 2026

Am I understanding correctly that there is no way to know what the length of req.req.Pack would be (for example from the kind) ?

Some protocol messages are fixed size, some are variable size so we don't know length before decoding.

What I'm currently thinking is that I will have to add a length (I think uint16) to the header so a packet would now look like: protocol + n*[requestid kind length(data) data]

So we can properly decode packets.

length is not needed if we keep decoding messages until we reach the end of UDP packet. Each individual message will not decode more than needed.

Non related to this specific question. When approaching such protocol changes you have to take into account this has to be deployed to a live system without downtime.
You should first check if it can be done in backward compatible way without bumping the protocl.
I can tell you that it can but the order of changes and deployment matters.

  1. As a first step all of the decoders (golang, c++ and c (kernel module)) need to be changed to support reading multiple messages from same udp packet.
  2. Only after this is rolled out encoders should be changed to actually start sending multiple messages.

For easier testing you can have a branch with 2 commits. First commit switches decoders, second one switches encoders but both commits have to pass CI to ensure it is safe to roll out to production.

@janscheres
Copy link
Copy Markdown
Author

janscheres commented Feb 8, 2026

Thanks for the advice. I've not had to think about rolling out changes to prod without downtime much yet in my life, but what you said makes a lot of sense and I'll keep it in mind in future :)

I've made changes to the go and cpp decoders now (I dont know c well....)

(force pushed to maintain cleanliness of commit history, I have locally stashed the changes I made to the go encoder for later)

@mcrnic
Copy link
Copy Markdown
Collaborator

mcrnic commented Feb 13, 2026

if you check how ProtocolMessage and SignedProtocolMessage unpack works you will see it enforces the buffer to be fully consumed.
It's an easy change as it's bool template param. It has to be changed in Protocol.hpp defines

@janscheres
Copy link
Copy Markdown
Author

from what i understand, all of those needed updating right? Ive done that now :)

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