libp2p / go-libp2p-pubsub

The PubSub implementation for go-libp2p
https://github.com/libp2p/specs/tree/master/pubsub
Other
321 stars 185 forks source link

add slack time to prune backoff clearance #374

Closed vyzo closed 4 years ago

vyzo commented 4 years ago

Fixes #368.

Implements https://github.com/libp2p/specs/pull/289.

vyzo commented 4 years ago

Well no, it's a little more complex (a real edge case). We already have some slack in that we only clear backoff every 15 ticks or so. The problem arises if we happen to remove the backoff at exactly the 60s mark, in which case it becomes eligible for graft - but the other side may be still in backoff because of variable latency. It's very unlikely that this could happen, but it is still a bug. The patch simply adds some extra slack to model the variable latency so that we don't hit the issue in the edge case.

In terms of mechanics, yeah we can add a heartbeat (or two) instead of having an extra variable; I am fine with that.

vyzo commented 4 years ago

Changed it to use 2*heartbeat for slack.

raulk commented 4 years ago

BTW -- took the liberty to submit a spec change PR, as this should be part of the agreed-upon state machine: https://github.com/libp2p/specs/pull/289

vyzo commented 4 years ago

Yeah, I saw -- good thinking, this is a tricky edge case that needs to be specified.

vyzo commented 4 years ago

gah BUG alert: the Add shouldn't go into the now variable, but rather in the backoff itself; fixing.

vyzo commented 4 years ago

ok, now it should be good.