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,