fix(snownet): replace Allocation if credentials to relay change
#3590
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Terraform Cloud Plan Output |
| if existing.is_some() { | ||
| tracing::info!(address = %server, "Replaced existing allocation because credentials to TURN server changed"); | ||
| } else { | ||
| tracing::info!(address = %server, "Added new TURN server"); | ||
| } |
There was a problem hiding this comment.
I started a discussion with the str0m maintainers on whether we should call invalidate_candidate here on all agents and perhaps trigger an ICE restart. it could be that we are just removing the nominated candidate pair and thus need to trigger an ICE restart but at present, invalidate_candidate doesn't tell me.
Discussion is here: https://str0m.zulipchat.com/#narrow/stream/377845-general/topic/invalidating-candidates
There was a problem hiding this comment.
We don't have to block this PR on that though. If a relay restarts, the connections won't work anymore anyway so this is just an optimisation to perhaps rescue some of the connections through an ICE restart instead of having to fail the connection via the tunnel.
Performance Test Results: direct-perfTCP
UDP
|
Performance Test Results: relayed-perfTCP
UDP
|
thomaseizinger
force-pushed
the
fix/snownet/replace-relay-if-credentials-differ
branch
from
4bc3909
to
d187bbb
Compare
thomaseizinger
deleted the
fix/snownet/replace-relay-if-credentials-differ
branch
When a relay restarts, its credentials change but the socket we use to connect to it might not. Because we upsert
Allocations within asnownet::Nodebased on the socket, such a change is currently not picked up.Instead, we now check whether an existing allocation uses the same credentials and if it doesn't we throw the old one away and use the new one instead.