Add support send/recv chunk for checkin protocol #89
Conversation
|
💚 CLA has been signed |
blakerouse
force-pushed
the
chunk-checkin
branch
from
c1a308b
to
f3373d8
Compare
There was a problem hiding this comment.
there are 2 small blocker, small things just to avoid confusion.
Another thing I think would be good is to add at least a test for the case where a new chinked message arrives to ensure it discard the old and keeps the new
Another test I see is missing is for when the 1st message fits more than 1 unit
Updated the tests in adcba18 to do that.
Add tests in adcba18 that test this as well. |
| if first.UnitsTimestamp.AsTime().After(msg.UnitsTimestamp.AsTime()) { | ||
| // not newer so we ignore the message | ||
| continue |
There was a problem hiding this comment.
Doing this makes sense but we probably I think we want a way to tell that it happened. If this begins to cause missed checkins we won't be able to know why to debug it.
| } | ||
| } | ||
|
|
||
| func shallowCopyCheckinExpected(msg *proto.CheckinExpected) *proto.CheckinExpected { |
There was a problem hiding this comment.
proto.Clone is a deep copy but has the advantage that we don't have to keep this function up to date if we add a top level message field.
Is there a test we can write that would fail if you forgot to update this function (and the observed version of it)?
Overview
Adds the ability for observed and expected checkin messages to be larger that the maximum message size, where the messages are chunked across the streaming protocol and merged back together.
Requirements
How it works
It divides messages
unitsacross multiple messages with aunits_timestampto ensure that the messages are merged back together are from the same original message. In the case that they are not the sameunits_timestampthen only the newer message is used and taken as a new start to units. This allows the chunk of messages to be cancelled and a new chunk to be sent even before all the previous chunks have been sent.