NetSim: Better cleaning of orphaned messages#1788
Conversation
…ogic for deciding whether our local client controls a message. Also, clean up messages that aren't being simulated by anyone.
There was a problem hiding this comment.
Do you plan to unify the logic so that the sender always simulates, or are there reasons to have the recipient continue to receive when there is no router, or is it just not worth the trouble to unify?
There was a problem hiding this comment.
"Who simulates" effectively means, "which client has permission to pull this message out of storage?" In a P2P situation, that must be the receiving client as the sender has no indication of whether the message has been read.
We could reverse the logic when a router is present, and have a client act as the router for all messages destined for their own node; that would allow the router to just pass-through the simulatedBy property for all messages it processes, but I'm afraid it would make other router features like limited bandwidth and router stats seem less accurate - since the router gets immediate local notification when the sender simulates, it can at least update its own logs and timings immediately, and put the network delay between the router and the recipient (matching the delay in the arrival of the router stats).
There was a problem hiding this comment.
Makes great sense, thanks for the explanation.
|
LGTM. Good tests! |
…hput NetSim: Better cleaning of orphaned messages
The change in
NetSimRouterNode.prototype.localSimulationOwnsMessageRow_sums up what's being done here.Before I had this big ugly logic block for deciding whether the local simulation could process a message, which sometimes depended on opening up the binary payload and parsing out headers to see where it was ultimately going.
Facing the possibility of duplicating this logic in
NetSimShardCleaner, I decided it was easier to add asimulatedBymetadata field to every message.Benefits:
Unfortunates:
NetSimMessage.sendnow takes six arguments: shard, fromNode, toNode, simulatedBy, payload and onComplete. You also have to understand enough to pick a simulating node any time you send a message. Fortunately, this only happens in three places, right now: Client sending a message, router forwarding a message, auto-DNS sending a response.See also updated tests, and a new block of tests for auto-DNS behavior that will break if the simulatedBy property is set incorrectly at any point in the process.