chore(pullsync): cancel ruid cleanup #3456
Conversation
istae
force-pushed
the
pullsync-remove-cancel
branch
2 times, most recently
from
12cf358
to
49f3eb2
Compare
istae
requested review from
a team,
vladopajic,
notanatol and
aloknerurkar
and removed request for
a team
| loggerV2.Debug("peer pulling", "peer_address", p.Address, "ruid", ru.Ruid) | ||
| ctx, cancel := context.WithCancel(ctx) | ||
| loggerV2.Debug("peer pulling", "peer_address", p.Address) | ||
|
|
There was a problem hiding this comment.
So if the peer goes away and we have active sync workers waiting to restart, but within this time, if the peer comes back and issues a fresh pull sync request, we should check here if we have the workers running, if yes we should clean them up.
There was a problem hiding this comment.
what does clean up imply here?
There was a problem hiding this comment.
What happens to the sync workers that are in progress? How does the handler take those into consideration?
If the peers try to sync again, then kademlia topo signal after the worker wakes up doesn't remove the peer, what will happen?
This ruid handling would cancel those contexts and close them. We would need to do something similar?
There was a problem hiding this comment.
If I understood the question:
If there is a hanging makeOffer call because there are not enough chunks to satisfy the request the makeOffer will timeout in 15 seconds with the worseCase timer in the falling edge detector. If the worry is there could be a leak, this is not the case.
There was a problem hiding this comment.
The cancel ruid functionality was to cancel this waiting makeOffer call but everything should be ok with this 15 second timer terminating the wait.
istae
force-pushed
the
pullsync-remove-cancel
branch
from
2175ccc
to
5167642
Compare
Checklist
Description
PR removes the cancellation ruid and all related code after the latest changes to the puller and pullsync protocols.
Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):
This change is