Rate limit #778
Rate limit #778
Conversation
| // in a routine cause we don't want to block the processing of the DKG | ||
| // as well - that's ok since we are only expecting to send 3 packets out | ||
| // at the very least. | ||
| go b.dispatcher.broadcastDirect(proto) |
There was a problem hiding this comment.
having these off in go-routines seems potentially like it could end up with some weird memory leak / build-up of pending writes. I'd prefer a channel and single sending thread for these as well in parallel to the non-bypassing ones.
There was a problem hiding this comment.
I'm not saying I dont want to do it but I want to highlight that DKG only outputs 1 deal packet, 1 resp packet and up to 1 justification packet.
There was a problem hiding this comment.
my worry is cases where the sending blocks - e.g. because dns resolution has decided on a bad time to time out for a minute.
In the mean-time, everything can get torn down and a new reshare could start, and then some random packet from this previous thread could still get sent out.
Having it in the same place makes it easier to trust it's going to get torn down properly and not get left dangling.
There was a problem hiding this comment.
Mhhh but in this specific failure scenario , this can already happen with the current code right ?(Because we're using channels, and we simply close them, but the "pending" messages in the channel are still present and will be sent I believe.)
I think what we need to prevent against that is using context, so when we stop the broadcaster, we stop all context and thus no further messages get sent. Does that seem right ?
And if so, can I or you do that in a subsequent PR ?
|
can we disable this large test in CI since it seems to fail the limits there? |
|
sorry for spamming with circle failures when i hit the ci a few times before believing it's a legit failure. it looks like what's going on is the The right answer is to have a context there, so it blocks until the context cancels and we can have a concept of timeout. |
|
I simply saw that it wasn't worth block-ing on the thread so now it simply returns an error and the test pass. |
No description provided.