gbn: add boosting of resend & handshake timeouts

When we need to resend a data packet, or when we need to resend the SYN
message during the handshake, due to the other side not responding
within given timeout, we will boost the timeout by 50% for each time we
need to resend without receiving any response.
This ensures that if the original timeouts are set to a too short
duration given the current network latency, we will eventually boost the
timeouts to a long enough duration that allows the other side to be able
to respond within the timeout.

Author: ViktorT-11

gbn/config.go

@@ -66,6 +66,18 @@ func WithKeepalivePing(ping, pong time.Duration) TimeoutOptions {
 	}
 }
 
+// WithBoostPercent is used to set the boost percent that the timeout manager
+// will use to boost the resend timeout & handshake timeout every time a resend
+// is required due to not receiving a response within the current timeout.
+func WithBoostPercent(boostPercent float32) TimeoutOptions {
+	return func(manager *TimeoutManager) {
+		if boostPercent > 0 {
+			manager.resendBoostPercent = boostPercent
+			manager.handshakeBoostPercent = boostPercent
+		}
+	}
+}
+
 // config holds the configuration values for an instance of GoBackNConn.
 type config struct {
 	// n is the window size. The sender can send a maximum of n packets

gbn/timeout_manager.go

@@ -15,6 +15,7 @@ const (
 	defaultFinSendTimeout         = 1000 * time.Millisecond
 	defaultResendMultiplier       = 5
 	defaultTimeoutUpdateFrequency = 100
+	defaultBoostPercent           = 0.5
 	DefaultSendTimeout            = math.MaxInt64
 	DefaultRecvTimeout            = math.MaxInt64
 )
@@ -147,6 +148,32 @@ type TimeoutManager struct {
 	// latestSentSYNTime field.
 	latestSentSYNTimeMu sync.Mutex
 
+	// resendBooster is used to boost the resend timeout when we timeout
+	// when sending a data packet before receiving a response. The resend
+	// timeout will remain boosted until it is updated dynamically, as the
+	// timeout set during the dynamic update most accurately reflects the
+	// current response time.
+	resendBooster *TimeoutBooster
+
+	// resendBoostPercent is the percentage value the resend timeout will be
+	// boosted by, any time the Boost function is called for the
+	// resendBooster.
+	resendBoostPercent float32
+
+	// handshakeBooster is used to boost the handshake timeout if we timeout
+	// when sending the SYN message before receiving the corresponding
+	// response. The handshake timeout will remain boosted throughout the
+	// lifespan of the connection if it's boosted.
+	// The handshake timeout is the time after which the server or client
+	// will abort and restart the handshake if the expected response is
+	// not received from the peer.
+	handshakeBooster *TimeoutBooster
+
+	// handshakeBoostPercent is the percentage value the handshake timeout
+	// will be by boosted, any time the Boost function is called for the
+	// handshakeBooster.
+	handshakeBoostPercent float32
+
 	// handshakeTimeout is the time after which the server or client
 	// will abort and restart the handshake if the expected response is
 	// not received from the peer.
@@ -207,6 +234,8 @@ func NewTimeOutManager(logger btclog.Logger,
 		log:                    logger,
 		resendTimeout:          defaultResendTimeout,
 		handshakeTimeout:       defaultHandshakeTimeout,
+		resendBoostPercent:     defaultBoostPercent,
+		handshakeBoostPercent:  defaultBoostPercent,
 		useStaticTimeout:       false,
 		resendMultiplier:       defaultResendMultiplier,
 		finSendTimeout:         defaultFinSendTimeout,
@@ -220,6 +249,23 @@ func NewTimeOutManager(logger btclog.Logger,
 		opt(m)
 	}
 
+	// When we are resending packets, it's likely that we'll resend a range
+	// of packets. As we don't want every packet in that range to boost the
+	// resend timeout, we'll initialize the resend booster with a ticker,
+	// which will ensure that only the first resent packet in the range will
+	// boost the resend timeout.
+	m.resendBooster = NewTimeoutBooster(
+		m.resendTimeout,
+		m.resendBoostPercent,
+		true,
+	)
+
+	m.handshakeBooster = NewTimeoutBooster(
+		m.handshakeTimeout,
+		m.handshakeBoostPercent,
+		false,
+	)
+
 	return m
 }
 
@@ -255,6 +301,11 @@ func (m *TimeoutManager) Sent(msg Message, resent bool) {
 		// if the response is for the resent SYN or the original SYN.
 		m.latestSentSYNTime = time.Time{}
 
+		// We'll also temporarily boost the handshake timeout while
+		// we're resending the SYN message. This might occur multiple
+		// times until we receive the corresponding response.
+		m.handshakeBooster.Boost()
+
 	case *PacketData:
 		m.sentTimesMu.Lock()
 		defer m.sentTimesMu.Unlock()
@@ -266,6 +317,8 @@ func (m *TimeoutManager) Sent(msg Message, resent bool) {
 			// corresponding response.
 			delete(m.sentTimes, msg.Seq)
 
+			m.resendBooster.Boost()
+
 			return
 		}
 
@@ -356,22 +409,41 @@ func (m *TimeoutManager) updateResendTimeout(responseTime time.Duration) {
 	m.log.Tracef("Updating resendTimeout to %v", multipliedTimeout)
 
 	m.resendTimeout = multipliedTimeout
+
+	// Also update and reset the resend booster, as the new dynamic
+	// resend timeout most accurately reflects the current response
+	// time.
+	m.resendBooster.Reset(multipliedTimeout)
+
+	// As we may have received a data packet that executes this function
+	// while we are also concurrently resending the queue, we also restart
+	// the frequency timeout, to ensure that the messages we're resending
+	// won't boost the resend timeout.
+	m.resendBooster.RestartFrequencyTimeout()
 }
 
 // GetResendTimeout returns the current resend timeout.
 func (m *TimeoutManager) GetResendTimeout() time.Duration {
 	m.mu.RLock()
 	defer m.mu.RUnlock()
 
-	return m.resendTimeout
+	resendTimeout := m.resendBooster.GetCurrentTimeout()
+
+	m.log.Debugf("Returning resendTimeout %v", resendTimeout)
+
+	return resendTimeout
 }
 
 // GetHandshakeTimeout returns the handshake timeout.
 func (m *TimeoutManager) GetHandshakeTimeout() time.Duration {
 	m.mu.RLock()
 	defer m.mu.RUnlock()
 
-	return m.handshakeTimeout
+	handshake := m.handshakeBooster.GetCurrentTimeout()
+
+	m.log.Debugf("Returning handshakeTimeout %v", handshake)
+
+	return handshake
 }
 
 // GetFinSendTimeout returns the fin send timeout.

gbn/timeout_manager_test.go

@@ -64,11 +64,22 @@ func TestSYNDynamicTimeout(t *testing.T) {
 	require.Equal(t, minimumResendTimeout, newTimeout)
 
 	// Then we'll test that the resend timeout isn't dynamically set if
-	// when simulating a that the SYN message has been resent.
+	// when simulating a that the SYN message has been resent, but that the
+	// handshake timeout is boosted.
+	tm.handshakeBooster.boostPercent = 0.2
+	originalHandshakeTimeout := tm.GetHandshakeTimeout()
+
 	sendAndReceive(t, tm, synMsg, synMsg, true)
 
 	unchangedResendTimeout := tm.GetResendTimeout()
 	require.Equal(t, newTimeout, unchangedResendTimeout)
+
+	newHandshakeTimeout := tm.GetHandshakeTimeout()
+	require.Equal(
+		t,
+		time.Duration(float32(originalHandshakeTimeout)*1.2),
+		newHandshakeTimeout,
+	)
 }
 
 // TestDataPackageDynamicTimeout ensures that the resend timeout is dynamically
@@ -122,7 +133,9 @@ func TestDataPackageDynamicTimeout(t *testing.T) {
 	require.NotEqual(t, resendTimeout, newResendTimeout)
 
 	// Finally let's test that the resend timeout isn't dynamically set when
-	// simulating that the data packet has been resent.
+	// simulating that the data packet has been resent. The resend timeout
+	// shouldn't be boosted either, as the resend timeout is only boosted
+	// if we resend a packet after the duration of the previous resend time.
 	tm.timeoutUpdateFrequency = 1
 	tm.resendMultiplier = 100
 
@@ -132,6 +145,124 @@ func TestDataPackageDynamicTimeout(t *testing.T) {
 	require.Equal(t, newResendTimeout, unchangedResendTimeout)
 }
 
+// TestResendBooster tests that the resend timeout booster works as expected,
+// and that timeout manager's resendTimeout get's boosted when we need to resend
+// a packet again due to not receiving a response within the resend timeout.
+func TestResendBooster(t *testing.T) {
+	t.Parallel()
+
+	tm := NewTimeOutManager(nil)
+	setResendTimeout := time.Millisecond * 1000
+	tm.resendTimeout = setResendTimeout
+
+	initialResendTimeout := tm.GetResendTimeout()
+	msg := &PacketData{Seq: 20}
+	response := &PacketACK{Seq: 20}
+
+	// As the resend timeout won't be dynamically set when we are resending
+	// packets, we'll first test that the resend timeout didn't get
+	// dynamically updated by a resent data packet. This will however
+	// boost the resend timeout, so let's initially set the boost percent
+	// to 0 so we can test that the resend timeout wasn't set.
+	tm.timeoutUpdateFrequency = 1
+	tm.resendMultiplier = 1
+
+	tm.resendBooster.boostPercent = 0
+
+	sendAndReceiveWithDuration(
+		t, tm, time.Millisecond, msg, response, true,
+	)
+
+	unchangedResendTimeout := tm.GetResendTimeout()
+	require.Equal(t, initialResendTimeout, unchangedResendTimeout)
+
+	// Now let's change the boost percent to a non-zero value and test that
+	// the resend timeout was boosted as expected.
+	tm.resendBooster.boostPercent = 0.1
+
+	changedResendTimeout := tm.GetResendTimeout()
+
+	require.Equal(
+		t,
+		time.Duration(float32(initialResendTimeout)*1.1),
+		changedResendTimeout,
+	)
+
+	// Now let's resend another packet again, which shouldn't boost the
+	// resend timeout again, as the duration of the previous resend timeout
+	// hasn't passed.
+	sendAndReceiveWithDuration(
+		t, tm, time.Millisecond, msg, response, true,
+	)
+
+	unchangedResendTimeout = tm.GetResendTimeout()
+
+	require.Equal(
+		t,
+		time.Duration(float32(initialResendTimeout)*1.1),
+		unchangedResendTimeout,
+	)
+
+	// Now let's wait for the duration of the previous resend timeout and
+	// then resend another packet. This should boost the resend timeout
+	// once more, as the duration of the previous resend timeout has passed.
+	err := wait.Invariant(func() bool {
+		currentResendTimeout := tm.GetResendTimeout()
+
+		return unchangedResendTimeout == currentResendTimeout
+	}, setResendTimeout)
+	require.NoError(t, err)
+
+	sendAndReceiveWithDuration(
+		t, tm, time.Millisecond, msg, response, true,
+	)
+
+	changedResendTimeout = tm.GetResendTimeout()
+
+	require.Equal(
+		t,
+		time.Duration(float32(initialResendTimeout)*1.2),
+		changedResendTimeout,
+	)
+
+	// Now let's verify that in case the resend timeout is dynamically set,
+	// the boost of the resend timeout is reset. Note that we're not
+	// simulating a resend here, as that will dynamically set the resend
+	// timeout as the timeout update frequency is set to 1.
+	sendAndReceiveWithDuration(
+		t, tm, time.Second, msg, response, false,
+	)
+
+	newResendTimeout := tm.GetResendTimeout()
+
+	require.NotEqual(t, changedResendTimeout, newResendTimeout)
+	require.Equal(t, 0, tm.resendBooster.boostCount)
+
+	// Finally let's check that the resend timeout isn't boosted if we
+	// simulate a resend before the duration of the newly set resend
+	// timeout hasn't passed.
+	sendAndReceiveWithDuration(
+		t, tm, time.Millisecond, msg, response, true,
+	)
+
+	require.Equal(t, 0, tm.resendBooster.boostCount)
+
+	// But if we wait for the duration of the newly set resend timeout and
+	// then simulate a resend, then the resend timeout should be boosted.
+	err = wait.Invariant(func() bool {
+		currentResendTimeout := tm.GetResendTimeout()
+
+		return newResendTimeout == currentResendTimeout
+	}, newResendTimeout)
+	require.NoError(t, err)
+
+	sendAndReceiveWithDuration(
+		t, tm, time.Millisecond, msg, response, true,
+	)
+
+	require.Equal(t, 1, tm.resendBooster.boostCount)
+}
+
 // TestStaticTimeout ensures that the resend timeout isn't dynamically set if a
 // static timeout has been set.
 func TestStaticTimeout(t *testing.T) {

mailbox/client_conn.go

@@ -82,6 +82,12 @@ const (
 	// gbnPongTimout is the time after sending the pong message that we will
 	// timeout if we do not receive any message from our peer.
 	gbnPongTimeout = 3 * time.Second
+
+	// gbnBoostPercent is the percentage value that the resend and handshake
+	// timeout will be boosted any time we need to resend a packet due to
+	// the corresponding response not being received within the previous
+	// timeout.
+	gbnBoostPercent = 0.5
 )
 
 // ClientStatus is a description of the connection status of the client.
@@ -183,6 +189,7 @@ func NewClientConn(ctx context.Context, sid [64]byte, serverHost string,
 			gbn.WithKeepalivePing(
 				gbnClientPingTimeout, gbnPongTimeout,
 			),
+			gbn.WithBoostPercent(gbnBoostPercent),
 		),
 		gbn.WithOnFIN(func() {
 			// We force the connection to set a new status after

mailbox/server_conn.go

@@ -89,6 +89,7 @@ func NewServerConn(ctx context.Context, serverHost string,
 				gbn.WithKeepalivePing(
 					gbnServerPingTimeout, gbnPongTimeout,
 				),
+				gbn.WithBoostPercent(gbnBoostPercent),
 			),
 		},
 		status:      ServerStatusNotConnected,