Opened 3 weeks ago

Last modified 5 days ago

#2660 new enhancement

PeerState.java: remove _consecutiveFailedSends

Reported by: jogger Owned by: zzz
Priority: minor Milestone: undecided
Component: router/transport Version: 0.9.43
Keywords: Cc:
Parent Tickets: Sensitive: no

Description

This is only used by send(OutNetMessage? msg), where it controls a way too long timeout. There it can strike after 50 sec earliest, while the timeout occurs no later than 60 sec anyway, so there is no practical difference.

Subtickets

Change History (2)

comment:1 Changed 7 days ago by zzz

Status: newinfoneeded_new

This is shown on the /peers page, and not hidden behind the advanced flag (although perhaps/probably it should be). Seems useful for debugging.

Not sure about 'controls a way too long timeout'.

It is used in UDPTransport.failed() to decide whether to disconnect the peer, which seems important, and it does happen (342 udp.dropPeerConsecutiveFailure events in 48 hours on a medium-speed router). But it could also be triggered by the lastSendFully() test. So OP may be incorrect? Not sure.

So I propose to keep it but hide the /peers indication behind the advanced setting.

comment:2 Changed 5 days ago by jogger

Status: infoneeded_newnew

oops actually it is the use in UDPTransport.failed() I was referring to.

            if ( (_context.clock().now() - msg.getPeer().getLastSendFullyTime() <= 60*1000) || (consecutive < MAX_CONSECUTIVE_FAILED) ) {

the first test fires after 60 sec, so having a second that *might* fire after 50+ is no benefit.

Note: See TracTickets for help on using tickets.