From c7e4c9e84993a6df103d8dae7bf4aec97ad88a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Tigerstr=C3=B6m?= Date: Mon, 4 Dec 2023 01:05:20 +0100 Subject: [PATCH] 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. --- gbn/config.go | 12 ++++ gbn/timeout_manager.go | 65 ++++++++++++++++-- gbn/timeout_manager_test.go | 132 +++++++++++++++++++++++++++++++++++- mailbox/client_conn.go | 7 ++ mailbox/server_conn.go | 1 + 5 files changed, 209 insertions(+), 8 deletions(-) diff --git a/gbn/config.go b/gbn/config.go index ad18a41..dd827ea 100644 --- a/gbn/config.go +++ b/gbn/config.go @@ -65,6 +65,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.handshakeBooster.boostPercent = boostPercent + manager.resendBooster.boostPercent = 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 diff --git a/gbn/timeout_manager.go b/gbn/timeout_manager.go index 1babb69..52ecbfe 100644 --- a/gbn/timeout_manager.go +++ b/gbn/timeout_manager.go @@ -13,6 +13,7 @@ const ( defaultFinSendTimeout = 1000 * time.Millisecond defaultResendMultiplier = 5 defaultTimeoutUpdateFrequency = 100 + defaultBoostPercent = 0.5 DefaultSendTimeout = math.MaxInt64 DefaultRecvTimeout = math.MaxInt64 ) @@ -153,10 +154,21 @@ type TimeoutManager struct { // the SYN message. latestSentSYNTime time.Time - // handshakeTimeout is the time after which the server or client + // 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. - handshakeTimeout time.Duration + handshakeBooster *TimeoutBooster + + // 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 // finSendTimeout is the timeout after which the created context for // sending a FIN message will be time out. @@ -200,10 +212,26 @@ type TimeoutManager struct { func NewTimeOutManager(isServer bool, timeoutOpts ...TimeoutOptions) *TimeoutManager { + handshakeBooster := NewTimeoutBooster( + defaultHandshakeTimeout, + defaultBoostPercent, + false, + ) + + // When we are resending packets, we are likely to resend multiple + // packets at in a range. As we don't like 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. + responseBooster := NewTimeoutBooster( + defaultResendTimeout, + defaultBoostPercent, + true, + ) + m := &TimeoutManager{ isServer: isServer, resendTimeout: defaultResendTimeout, - handshakeTimeout: defaultHandshakeTimeout, useStaticTimeout: false, resendMultiplier: defaultResendMultiplier, finSendTimeout: defaultFinSendTimeout, @@ -211,6 +239,8 @@ func NewTimeOutManager(isServer bool, sendTimeout: DefaultSendTimeout, sentTimes: make(map[uint8]time.Time), timeoutUpdateFrequency: defaultTimeoutUpdateFrequency, + handshakeBooster: handshakeBooster, + resendBooster: responseBooster, } for _, opt := range timeoutOpts { @@ -245,6 +275,12 @@ func (m *TimeoutManager) Sent(msg Message, resent bool) { // the response is for the resent SYN or the original // SYN. m.latestSentSYNTime = time.Unix(0, 0) + + // 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() } else { m.latestSentSYNTime = time.Now() } @@ -259,6 +295,8 @@ func (m *TimeoutManager) Sent(msg Message, resent bool) { // update the resend timeout when we receive the // corresponding response. delete(m.sentTimes, msg.Seq) + + m.resendBooster.Boost() } else { m.sentTimes[msg.Seq] = time.Now() } @@ -331,7 +369,20 @@ func (m *TimeoutManager) updateResendTimeout(responseTime time.Duration) { m.timeoutManagerMu.Lock() defer m.timeoutManagerMu.Unlock() + 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.SetOriginalTimeout(multipliedTimeout) + m.resendBooster.Reset() + + // 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. @@ -339,7 +390,7 @@ func (m *TimeoutManager) GetResendTimeout() time.Duration { m.timeoutManagerMu.RLock() defer m.timeoutManagerMu.RUnlock() - return m.resendTimeout + return m.resendBooster.GetCurrentTimeout() } // GetHandshakeTimeout returns the handshake timeout. @@ -347,7 +398,7 @@ func (m *TimeoutManager) GetHandshakeTimeout() time.Duration { m.timeoutManagerMu.RLock() defer m.timeoutManagerMu.RUnlock() - return m.handshakeTimeout + return m.handshakeBooster.GetCurrentTimeout() } // GetFinSendTimeout returns the fin send timeout. @@ -401,6 +452,8 @@ func (m *TimeoutManager) SetStaticResendTimeout(resendTimeout time.Duration) { defer m.timeoutManagerMu.Unlock() m.resendTimeout = resendTimeout + m.resendBooster.SetOriginalTimeout(resendTimeout) + m.useStaticTimeout = true } @@ -418,7 +471,7 @@ func (m *TimeoutManager) SetHandshakeTimeout(handshakeTimeout time.Duration) { m.timeoutManagerMu.Lock() defer m.timeoutManagerMu.Unlock() - m.handshakeTimeout = handshakeTimeout + m.handshakeBooster.SetOriginalTimeout(handshakeTimeout) } // SetSendTimeout sets the send timeout. diff --git a/gbn/timeout_manager_test.go b/gbn/timeout_manager_test.go index 21dc56b..6b42284 100644 --- a/gbn/timeout_manager_test.go +++ b/gbn/timeout_manager_test.go @@ -61,11 +61,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, + ) } func TestDataPackageDynamicTimeout(t *testing.T) { @@ -118,7 +129,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.SetTimeoutUpdateFrequency(1) tm.SetResendMultiplier(100) @@ -128,6 +141,121 @@ func TestDataPackageDynamicTimeout(t *testing.T) { require.Equal(t, newResendTimeout, unchangedResendTimeout) } +func TestResendBooster(t *testing.T) { + t.Parallel() + + tm := NewTimeOutManager(false) + 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.SetTimeoutUpdateFrequency(1) + tm.SetResendMultiplier(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) { diff --git a/mailbox/client_conn.go b/mailbox/client_conn.go index a279b94..4cd26b9 100644 --- a/mailbox/client_conn.go +++ b/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. @@ -176,6 +182,7 @@ func NewClientConn(ctx context.Context, sid [64]byte, serverHost string, gbn.WithKeepalivePing( gbnClientPingTimeout, gbnPongTimeout, ), + gbn.WithBoostPercent(gbnBoostPercent), ), }, status: ClientStatusNotConnected, diff --git a/mailbox/server_conn.go b/mailbox/server_conn.go index f472c5b..75d90c2 100644 --- a/mailbox/server_conn.go +++ b/mailbox/server_conn.go @@ -89,6 +89,7 @@ func NewServerConn(ctx context.Context, serverHost string, gbn.WithKeepalivePing( gbnServerPingTimeout, gbnPongTimeout, ), + gbn.WithBoostPercent(gbnBoostPercent), ), }, status: ServerStatusNotConnected,