Repository navigation
Phase 1: packet framing - #1
Merged
Merged
Conversation
Every packet in the protocol arrives inside a 3-byte little-endian payload length plus a 1-byte sequence id, so this is what everything else is parsed out of. Parsing takes a binary and returns what it understood; nothing here touches a socket. That keeps it testable against captured bytes with no server running, which the 2018 version — where the parser did its own recv — was not.
TCP hands over a byte stream, not messages, so a read can deliver less than one packet. The caller needs to know to read again rather than treat a short payload as a parse failure. No change to decode/1 — the existing clauses already cover this. The test is here so a later refactor cannot quietly turn a short read into a MatchError.
Under load the server puts several packets in one TCP segment — a column definition and a row commonly arrive together. The caller needs what is left over so it can decode the next packet without re-reading the socket. Already satisfied by the existing clause; the test fixes the contract that rest is the caller's to continue from.
The client half of framing: every command the driver sends is wrapped this way, so decode/1 needs its inverse before anything can be written to a socket. Sequence id is a parameter rather than state. Tracking it across a command belongs to whatever owns the connection, not to the codec.
Thousands of generated payloads and sequence ids, including the empty payload, for the cost of one test. What it proves is symmetry, not correctness against the spec — a pair of functions that are wrong in the same way both directions passes it happily. Checking the reading against reality needs captured server bytes, which arrive with the handshake parser.
A payload of 0xFFFFFF bytes or more does not fit the 3-byte length field. The protocol's answer is continuation: a full-length frame means another follows, and reassembly ends at the first short frame — so a payload that is an exact multiple of the maximum needs a trailing empty frame or the reader keeps waiting. Until now byte_size/1 was written straight into 24 bits, so a 16MB payload silently wrote the length modulo 2^24 and framed itself as a 3-byte packet. encode/2 now returns iodata. :gen_tcp.send/2 accepts it, and splitting a large payload should not copy it.
The reading half of splitting: a frame carrying exactly 0xFFFFFF bytes means another follows, and the message ends at the first shorter frame. Callers above this layer get one payload and never learn the message arrived in pieces. Reports the final frame's sequence id, since that is what decides the next id the connection expects.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First slice of phase 1 from ROADMAP.md: the frame that every other packet in the protocol is parsed out of. Nine tests, one property.
What is here
Mysqlex.Packet:encode/2splits anddecode_message/1rejoins, so everything above this layer deals in whole messages and never learns that a large resultset arrived in pieces.Also a decision-log entry recording why
db_connectionstays for now.How to review
Three choices here that everything above depends on:
decode/1takes a binary, not a connection. The 2018 version hadparse_header(socket)doing its ownrecv, which is why none of it was testable without a live MySQL and why a wrong field width surfaced as a context-freeMatchError.encode/2returns iodata, not a binary.:gen_tcp.send/2accepts iodata, and splitting a large payload should not copy it.Two of the example tests needed no production change —
:moreon a short read andreston trailing bytes both fell out of the existing clauses. They pin the contract rather than drive it.The continuation work did find a real bug:
byte_size/1was being written straight into 24 bits, so a payload of 16MB or more silently wrote its length modulo 2^24 and framed itself as a 3-byte packet.On the property test
stream_datais a new test-only dependency (runtime: false). The roundtrip property alone does not justify it — a few examples would do. The case for it is theint<lenenc>codec coming next, which branches at 251, 2^16 and 2^24; shrinking reports the minimal failing input, which for size-branched codecs is most of the diagnosis. Easy to drop if you would rather stay at one dependency.What the roundtrip proves is symmetry, not correctness against the spec — a pair of functions wrong the same way in both directions passes it. Checking the reading against reality needs captured server bytes, which arrive with the handshake parser.
Not here
:moreinto another read — transport, not codecint<lenenc>/string<*>codecsOK/ERR/EOFpacketsThose finish phase 1, each as its own PR.