Implement incremental backoff and jitter for channel attach retries

Part of #1431.

Co-authored-by: Lawrence Forooghian <[email protected]>

Source/ARTAttachRetryState.m

@@ -31,14 +31,22 @@ - (instancetype)initWithRetryDelayCalculator:(id<ARTRetryDelayCalculator>)retryD
 }
 
 - (ARTRetryAttempt *)addRetryAttempt {
-    // Note: we currently reset the retry sequence every time we wish to perform a retry (defeating the point of using it in the first place, but it's OK since the delays are all constant). As part of implementing #1431 we will reuse any existing retry sequence, resetting it only in response to certain state changes.
-    self.retrySequence = [[ARTRetrySequence alloc] initWithDelayCalculator:self.retryDelayCalculator];
-    ARTLogDebug(self.logger, @"%@Created attach retry sequence %@", self.logMessagePrefix, self.retrySequence);
+    if (!self.retrySequence) {
+        self.retrySequence = [[ARTRetrySequence alloc] initWithDelayCalculator:self.retryDelayCalculator];
+        ARTLogDebug(self.logger, @"%@Created attach retry sequence %@", self.logMessagePrefix, self.retrySequence);
+    }
 
     ARTRetryAttempt *const retryAttempt = [self.retrySequence addRetryAttempt];
     ARTLogDebug(self.logger, @"%@Adding attach retry attempt to %@ gave %@", self.logMessagePrefix, self.retrySequence.id, retryAttempt);
 
     return retryAttempt;
 }
 
+- (void)channelWillTransitionToState:(ARTRealtimeChannelState)state {
+    // The client library specification doesn’t specify when to reset the retry count; have taken the logic from ably-js: https://github.com/ably/ably-js/blob/404c4128316cc5f735e3bf95a25e654e3fedd166/src/common/lib/client/realtimechannel.ts#L804-L806 (see discussion https://github.com/ably/ably-js/pull/1008/files#r925898316)
+    if (state != ARTRealtimeChannelAttaching && state != ARTRealtimeChannelSuspended) {
+        self.retrySequence = nil;
+    }
+}
+
 @end

Source/ARTRealtimeChannel.m

@@ -30,7 +30,7 @@
 #import "ARTChannelStateChangeMetadata.h"
 #import "ARTAttachRequestMetadata.h"
 #import "ARTRetrySequence.h"
-#import "ARTConstantRetryDelayCalculator.h"
+#import "ARTBackoffRetryDelayCalculator.h"
 #import "ARTInternalLog.h"
 #import "ARTAttachRetryState.h"
 #if TARGET_OS_IPHONE
@@ -269,7 +269,8 @@ - (instancetype)initWithRealtime:(ARTRealtimeInternal *)realtime andName:(NSStri
         _attachedEventEmitter = [[ARTInternalEventEmitter alloc] initWithQueue:_queue];
         _detachedEventEmitter = [[ARTInternalEventEmitter alloc] initWithQueue:_queue];
         _internalEventEmitter = [[ARTInternalEventEmitter alloc] initWithQueue:_queue];
-        const id<ARTRetryDelayCalculator> attachRetryDelayCalculator = [[ARTConstantRetryDelayCalculator alloc] initWithConstantDelay:realtime.options.channelRetryTimeout];
+        const id<ARTRetryDelayCalculator> attachRetryDelayCalculator = [[ARTBackoffRetryDelayCalculator alloc] initWithInitialRetryTimeout:realtime.options.channelRetryTimeout
+                                                                                                                jitterCoefficientGenerator:realtime.options.testOptions.jitterCoefficientGenerator];
         _attachRetryState = [[ARTAttachRetryState alloc] initWithRetryDelayCalculator:attachRetryDelayCalculator
                                                                                logger:logger
                                                                      logMessagePrefix:[NSString stringWithFormat:@"RT: %p C:%p ", _realtime, self]];
@@ -611,6 +612,8 @@ - (void)transition:(ARTRealtimeChannelState)state withMetadata:(ARTChannelStateC
         _errorReason = metadata.errorInfo;
     }
 
+    [self.attachRetryState channelWillTransitionToState:state];
+
     ARTEventListener *channelRetryListener = nil;
     switch (state) {
         case ARTRealtimeChannelAttached:

Source/PrivateHeaders/Ably/ARTAttachRetryState.h

@@ -1,4 +1,5 @@
 @import Foundation;
+#import "ARTTypes.h"
 
 @class ARTRetryAttempt;
 @class ARTInternalLog;
@@ -22,6 +23,11 @@ NS_SWIFT_NAME(AttachRetryState)
  */
 - (ARTRetryAttempt *)addRetryAttempt;
 
+/**
+ Resets the retry sequence when the channel leaves the sequence of `SUSPENDED` <-> `ATTACHING` state changes.
+ */
+- (void)channelWillTransitionToState:(ARTRealtimeChannelState)state;
+
 @end
 
 NS_ASSUME_NONNULL_END

Test/Tests/AttachRetryStateTests.swift

@@ -21,4 +21,41 @@ class AttachRetryStateTests: XCTestCase {
         XCTAssertEqual(secondRetryAttempt.delay, delays[1])
         XCTAssertEqual(thirdRetryAttempt.delay, delays[2])
     }
+
+    func test_transitionToNonAttachedOrSuspendedStateResetsRetrySequence() {
+        // Given: an AttachRetryState initialized with a delay calculator that returns arbitrary values from its `delay(forRetryNumber:)` method...
+        let delays: [TimeInterval] = [0.1, 0.3, 0.9]
+        let calculator = MockRetryDelayCalculator(delays: delays)
+
+        let retryState = AttachRetryState(retryDelayCalculator: calculator,
+                                          logger: .init(core: MockInternalLogCore()),
+                                          logMessagePrefix: "")
+
+        // When: addRetryAttempt is called an arbitrarily-chosen number n ( = 3) times on the attach retry state, and then channelWillTransition(to:) is called on the attach retry state with a channel state that is not SUSPENDED or ATTACHING (arbitrarily chosen ATTACHED here), and addRetryAttempt is then called again on the attach retry state...
+        (0..<3).forEach { _ in let _ = retryState.addRetryAttempt() }
+        retryState.channelWillTransition(to: .attached)
+        let retryAttempt = retryState.addRetryAttempt()
+
+        // Then: the `delay` property of the post-channelWillTransition(to:) returned RetryAttempt object matches that returned by the calculator’s `delay(forRetryNumber: 1)` method.
+        XCTAssertEqual(retryAttempt.delay, delays[0])
+    }
+
+    func test_transitionToAttachedOrSuspendedStateDoesNotResetRetrySequence() {
+        // Given: an AttachRetryState initialized with a delay calculator that returns arbitrary values from its `delay(forRetryNumber:)` method...
+        let delays: [TimeInterval] = [0.1, 0.3, 0.9, 0.7]
+        let calculator = MockRetryDelayCalculator(delays: delays)
+
+        let retryState = AttachRetryState(retryDelayCalculator: calculator,
+                                          logger: .init(core: MockInternalLogCore()),
+                                          logMessagePrefix: "")
+
+        // When: addRetryAttempt is called an arbitrarily-chosen number n ( = 3) times on the attach retry state, and then channelWillTransition(to:) is called on the attach retry state, once with channel state SUSPENDED and once with channel state ATTACHING, and addRetryAttempt is then called again on the attach retry state...
+        (0..<3).forEach { _ in let _ = retryState.addRetryAttempt() }
+        retryState.channelWillTransition(to: .suspended)
+        retryState.channelWillTransition(to: .attaching)
+        let retryAttempt = retryState.addRetryAttempt()
+
+        // Then: the `delay` property of the post-channelWillTransition(to:) returned RetryAttempt object matches that returned by the calculator’s `delay(forRetryNumber: n + 1)` method.
+        XCTAssertEqual(retryAttempt.delay, delays[3])
+    }
 }

Test/Tests/RealtimeClientChannelTests.swift

@@ -4015,13 +4015,18 @@ class RealtimeClientChannelTests: XCTestCase {
 
            ## Motivation for chosen channelRetryTimeout value
 
-           We expect the retries in this sequence to be spaced apart by channelRetryTimeout seconds. The default value is 15 seconds, so as above, in order to avoid a very long test execution time we reduce it to 1 second.
+           We expect the retries in this sequence to be spaced apart by values in the range of [channelRequestTimeout seconds, 2 * channelRequestTimeout seconds]. The default value of channelRequestTimeout is 15 seconds, so as above, in order to avoid a very long test execution time we reduce it to 1 second.
          */
         let options = try AblyTests.commonAppSetup(for: test)
         options.channelRetryTimeout = 1.0
         options.autoConnect = false
         options.testOptions.realtimeRequestTimeout = 1.0
         options.testOptions.transportFactory = TestProxyTransportFactory()
+
+        let jitterCoefficients = StaticJitterCoefficients()
+        let mockJitterCoefficientGenerator = MockJitterCoefficientGenerator(coefficients: jitterCoefficients)
+        options.testOptions.jitterCoefficientGenerator = mockJitterCoefficientGenerator
+
         let client = ARTRealtime(options: options)
         client.connect()
         defer { client.dispose(); client.close() }
@@ -4078,11 +4083,17 @@ class RealtimeClientChannelTests: XCTestCase {
 
         // Then...
 
-        let timeout = Double(numberOfRetriesToWaitFor) * (
+        let expectedRetryDelays = Array(
+            AblyTests.expectedRetryDelays(
+                forTimeout: options.channelRetryTimeout,
+                jitterCoefficients: jitterCoefficients
+            ).prefix(numberOfRetriesToWaitFor)
+        )
+        let timeout = expectedRetryDelays.map { retryDelay in
             options.testOptions.realtimeRequestTimeout // waiting for attach to time out
-            + options.channelRetryTimeout // waiting for retry to occur
+            + retryDelay // waiting for retry to occur
             + 0.2 // some extra tolerance, arbitrarily chosen
-        )
+        }.reduce(0, +)
         let observedStateChanges = try retrySequenceDataGatherer.waitForData(timeout: timeout)
 
         let expectedNumberOfObservedStateChanges = 1 + 2 * numberOfRetriesToWaitFor
@@ -4103,9 +4114,9 @@ class RealtimeClientChannelTests: XCTestCase {
         for retryNumber in 1 ... numberOfRetriesToWaitFor {
             let observedStateChangesStartIndexForThisRetry = 1 + 2 * (retryNumber - 1)
 
-            let expectedRetryDelay = options.channelRetryTimeout
+            let expectedRetryDelay = expectedRetryDelays[retryNumber - 1]
 
-            // after channelRetryTimeout seconds (as described by the retry metadata attached to the channel state change, and as approximately measured), the channel emits a state change to the ATTACHING state...
+            // after a delay (as described by the retry metadata attached to the channel state change, and as approximately measured) matching that defined by RTB1 (with initial retry timeout of channelRetryTimeout), the channel emits a state change to the ATTACHING state...
             let firstObservedStateChange = observedStateChanges[observedStateChangesStartIndexForThisRetry]
             XCTAssertEqual(firstObservedStateChange.stateChange.previous, .suspended)
             XCTAssertEqual(firstObservedStateChange.stateChange.current, .attaching)