Fix mixnode HOL blocking #530
Conversation
| if let Entry::Vacant(entry) = self.forwarders.entry(packet.target) { | ||
| entry.insert(Forwarder::new(packet.target, self.config)); | ||
| } | ||
|
|
||
| let forwarder = self.forwarders.get_mut(&packet.target).unwrap(); |
There was a problem hiding this comment.
nit: self.forwarders.entry(...).or_insert_with(|| ...).schedule(packet.body)
| } | ||
| } | ||
|
|
||
| fn handle_retry(&self, mut pkt: Packet) { |
There was a problem hiding this comment.
why did we remove retries?
There was a problem hiding this comment.
Ah, I forgot to mention about this in the PR. Actually, we're not removing retry. I changed the retry mechanism little bit in this case.
Previously, we keep a retry_count for each packet, and if it still fails after max_retries, we drop the packet.
But, in this PR, I simplifed the retry mechanism from per-packet to per-connection, since a mixnode must always maintain connections with the next mix layer. If we fail to send a packet to a certain connection, we push the packet back to the queue and try to re-establish the connection. So, it's reconnect precisely (rather than retry).
(Actually, our previous retry implementation had something wrong. It didn't re-establish the connection when the write call is failed with IO error. Then, the retry won't help.)
Also, I removed the max_retries parameter in this PR because the mixnode has to try its best to re-establish connections with the next layer. This will make the message queue (UnboundedChannel) very big if the retry keeps failing, but I think it's the common case that can happen in other parts that use queues. Maybe, we can revive the max_retries parameter, and drop/block all messages for the dead connection if the reconnect keeps failing.
p.s.
I found that I removed the exponential backoff. I'll revive it.
There was a problem hiding this comment.
Makes sense, after all all packets in a connection will likely suffer the same fate
|
@zeegomo Please don't review this PR. I'm closing this because I found that there is a similar part in the mixclient, which can be refactored similarily using the connection pool that I implemented in this PR. I'm gonna create a common crate |
Closes #526
TODO: #529 and #531