CBL-1910: maxAttempts, maxAttemptWaitTime, heartBeat (#2840)

* update the swift unit test for maxAttempt * update maxAttemptWaitTime and heartbeat props as well * remove the default values from platform side, since it will be handled in lite core * make maxAttempts unsigned
couchbase · May 26, 2021 · 0d3463d · 0d3463d
1 parent 02af6e2
commit 0d3463d
Show file tree

Hide file tree

Showing 9 changed files with 179 additions and 240 deletions.
diff --git a/Objective-C/CBLReplicatorConfiguration.h b/Objective-C/CBLReplicatorConfiguration.h
@@ -124,29 +124,32 @@ typedef BOOL (^CBLReplicationFilter) (CBLDocument* document, CBLDocumentFlags fl
 /**
  The heartbeat interval in second.
 
- The interval when the replicator sends the ping message to check whether the other peer is still alive.
- Note: Setting the heartbeat to zero or negative value will result in InvalidArgumentException being thrown.
+ The interval when the replicator sends the ping message to check whether the other peer is still alive. Set the value to zero(by default)
+ means using the default heartbeat of 300 seconds.
+ 
+ Note: Setting the heartbeat to negative value will result in InvalidArgumentException being thrown.
  */
 @property (nonatomic) NSTimeInterval heartbeat;
 
 /**
  The maximum attempts to perform retry. The retry attempt will be reset when the replicator is able to connect and replicate with
  the remote server again.
  
- Without setting the maxRetries value, the default maxRetries of 9 times for single shot replicators and infinite times for
- continuous replicators will be applied and present to users. Settings the value to 0 will result in no retry attempt.
- 
- Setting a negative number will result in InvalidArgumentException being thrown.
+ Setting the maxAttempts to zero(by default), the default maxAttempts of 10 times for single shot replicators and max-int times for
+ continuous replicators will be applied and present to users. Settings the value to 1, will perform an initial request and
+ if there is a transient error occurs, will stop will retrying.
  */
-@property (nonatomic) NSInteger maxRetries;
+@property (nonatomic) NSUInteger maxAttempts;
 
 /**
- Max wait time for the next retry.
+ Max wait time for the next attempt(retry).
+ 
+ The exponential backoff for calculating the wait time will be used by default and cannot be customized. Set the value to zero(by default)
+ means using the default max attempts of 300 seconds.
  
- The exponential backoff for calculating the wait time will be used by default and cannot be customized. Set the maxRetryWaitTime to zero
- or negative value will result in InvalidArgumentException being thrown.
+ Set the maxAttemptWaitTime to negative value will result in InvalidArgumentException being thrown.
  */
-@property (nonatomic) NSTimeInterval maxRetryWaitTime;
+@property (nonatomic) NSTimeInterval maxAttemptWaitTime;
 
 /** Not available */
 - (instancetype) init NS_UNAVAILABLE;

diff --git a/Objective-C/CBLReplicatorConfiguration.m b/Objective-C/CBLReplicatorConfiguration.m
@@ -29,11 +29,6 @@
 #import "CBLReplicatorConfiguration+ServerCert.h"
 #endif
 
-static int kDefaultHeartBeat = 300;
-static int kDefaultSingleShotMaxRetries = 9;
-static NSInteger kDefaultContinousMaxRetries = NSIntegerMax;
-static int kDefaultMaxRetryWaitTime = 300;
-
 @implementation CBLReplicatorConfiguration {
     BOOL _readonly;
 }
@@ -47,7 +42,7 @@ @implementation CBLReplicatorConfiguration {
 @synthesize pushFilter=_pushFilter, pullFilter=_pullFilter;
 @synthesize checkpointInterval=_checkpointInterval, heartbeat=_heartbeat;
 @synthesize conflictResolver=_conflictResolver;
-@synthesize maxRetries=_maxRetries, maxRetryWaitTime=_maxRetryWaitTime;
+@synthesize maxAttempts=_maxAttempts, maxAttemptWaitTime=_maxAttemptWaitTime;
 
 #ifdef COUCHBASE_ENTERPRISE
 @synthesize acceptOnlySelfSignedServerCertificate=_acceptOnlySelfSignedServerCertificate;
@@ -71,9 +66,9 @@ - (instancetype) initWithDatabase: (CBLDatabase*)database
 #ifdef COUCHBASE_ENTERPRISE
         _acceptOnlySelfSignedServerCertificate = NO;
 #endif
-        _heartbeat = kDefaultHeartBeat;
-        _maxRetries = -1;
-        _maxRetryWaitTime = kDefaultMaxRetryWaitTime;
+        _heartbeat = 0;
+        _maxAttempts = 0;
+        _maxAttemptWaitTime = 0;
     }
     return self;
 }
@@ -145,35 +140,27 @@ - (void) setAllowReplicatingInBackground: (BOOL)allowReplicatingInBackground {
 - (void) setHeartbeat: (NSTimeInterval)heartbeat {
     [self checkReadonly];
 
-    if (heartbeat <= 0)
+    if (heartbeat < 0)
         [NSException raise: NSInvalidArgumentException
-                    format: @"Attempt to store zero or negative value in heartbeat"];
+                    format: @"Attempt to store negative value in heartbeat"];
 
     _heartbeat = heartbeat;
 }
 
-- (void) setMaxRetries: (NSInteger)maxRetries {
+- (void) setMaxAttempts: (NSUInteger)maxAttempts {
     [self checkReadonly];
 
-    if (maxRetries < 0)
-        [NSException raise: NSInvalidArgumentException
-                    format: @"Attempt to store negative value in maxRetries"];
-
-    _maxRetries = maxRetries;
-}
-
-- (NSInteger) maxRetries {
-    return _maxRetries >= 0 ? _maxRetries : _continuous ? kDefaultContinousMaxRetries : kDefaultSingleShotMaxRetries;
+    _maxAttempts = maxAttempts;
 }
 
-- (void) setMaxRetryWaitTime: (NSTimeInterval)maxRetryWaitTime {
+- (void) setMaxAttemptWaitTime: (NSTimeInterval)maxAttemptWaitTime {
     [self checkReadonly];
 
-    if (maxRetryWaitTime <= 0)
+    if (maxAttemptWaitTime < 0)
         [NSException raise: NSInvalidArgumentException
-                    format: @"Attempt to store zero or negative value in maxRetryWaitTime"];
+                    format: @"Attempt to store negative value in maxAttemptWaitTime"];
 
-    _maxRetryWaitTime = maxRetryWaitTime;
+    _maxAttemptWaitTime = maxAttemptWaitTime;
 }
 
 #pragma mark - Internal
@@ -201,8 +188,8 @@ - (instancetype) initWithConfig: (CBLReplicatorConfiguration*)config
         _heartbeat = config.heartbeat;
         _checkpointInterval = config.checkpointInterval;
         _conflictResolver = config.conflictResolver;
-        _maxRetries = config.maxRetries;
-        _maxRetryWaitTime = config.maxRetryWaitTime;
+        _maxAttempts = config.maxAttempts;
+        _maxAttemptWaitTime = config.maxAttemptWaitTime;
 #if TARGET_OS_IPHONE
         _allowReplicatingInBackground = config.allowReplicatingInBackground;
 #endif
@@ -247,10 +234,11 @@ - (NSDictionary*) effectiveOptions {
     if (_heartbeat > 0)
         options[@kC4ReplicatorHeartbeatInterval] = @(_heartbeat);
 
-    if (_maxRetryWaitTime > 0)
-        options[@kC4ReplicatorOptionMaxRetryInterval] = @(_maxRetryWaitTime);
+    if (_maxAttemptWaitTime > 0)
+        options[@kC4ReplicatorOptionMaxRetryInterval] = @(_maxAttemptWaitTime);
 
-    options[@kC4ReplicatorOptionMaxRetries] = @([self maxRetries]);
+    if (_maxAttempts > 0)
+        options[@kC4ReplicatorOptionMaxRetries] = @(_maxAttempts - 1);
 
 #ifdef COUCHBASE_ENTERPRISE
     NSString* uniqueID = $castIf(CBLMessageEndpoint, _target).uid;

diff --git a/Objective-C/Tests/ReplicatorTest+Main.m b/Objective-C/Tests/ReplicatorTest+Main.m
@@ -1714,7 +1714,7 @@ - (void) testReplicationConfigSetterMethods {
     CBLReplicatorConfiguration* temp = [self configWithTarget: target
                                                          type: kCBLReplicatorTypePush
                                                    continuous: YES];
-    AssertEqual(temp.heartbeat, 300);
+    AssertEqual(temp.heartbeat, 0);
 
     [temp setContinuous: YES];
     [temp setAuthenticator: basic];
@@ -1741,7 +1741,7 @@ - (void) testReplicationConfigSetterMethods {
     AssertEqualObjects(headers, config.headers);
     AssertEqualObjects(docIds, config.documentIDs);
     AssertEqualObjects(channels, config.channels);
-    AssertEqual(config.heartbeat, 300);
+    AssertEqual(config.heartbeat, 0);
 }
 
 #pragma mark - HeartBeat
@@ -1751,10 +1751,6 @@ - (void) testHeartbeatWithInvalidValue {
                                                            type: kCBLReplicatorTypePush
                                                      continuous: YES];
 
-    [self expectException: @"NSInvalidArgumentException" in:^{
-        config.heartbeat = 0;
-    }];
-
     [self expectException: @"NSInvalidArgumentException" in:^{
         config.heartbeat = -1;
     }];
@@ -1778,56 +1774,45 @@ - (void) testCustomHeartbeat {
     repl = nil;
 }
 
-#pragma mark - Max Retry Count
+#pragma mark - Max Attempt Count
 
-- (void) testMaxRetryCount {
+- (void) testMaxAttemptCount {
     // continuous
     CBLReplicatorConfiguration* config = [self configWithTarget: kDummyTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: YES];
-    AssertEqual(config.maxRetries, NSIntegerMax);
+    AssertEqual(config.maxAttempts, 0);
 
     // single shot
     config = [self configWithTarget: kDummyTarget
                                type: kCBLReplicatorTypePush
                          continuous: NO];
-    AssertEqual(config.maxRetries, 9);
+    AssertEqual(config.maxAttempts, 0);
 }
 
-- (void) testCustomMaxRetryCount {
+- (void) testCustomMaxAttemptCount {
     // continuous
     CBLReplicatorConfiguration* config = [self configWithTarget: kDummyTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: YES];
-    config.maxRetries = 22;
-    AssertEqual(config.maxRetries, 22);
+    config.maxAttempts = 22;
+    AssertEqual(config.maxAttempts, 22);
 
     // continous
     config = [self configWithTarget: kDummyTarget
                                type: kCBLReplicatorTypePush
                          continuous: NO];
-    config.maxRetries = 11;
-    AssertEqual(config.maxRetries, 11);
+    config.maxAttempts = 11;
+    AssertEqual(config.maxAttempts, 11);
 }
 
-- (void) testInvalidMaxRetry {
-    CBLReplicatorConfiguration* config = [self configWithTarget: kDummyTarget
-                                                           type: kCBLReplicatorTypePush
-                                                     continuous: NO];
-    [self expectException: @"NSInvalidArgumentException" in:^{
-        config.maxRetries = -1;
-    }];
-}
-
-// set retry negative value for testing the default values
-- (void) testMaxRetry: (int) retry count: (int)count continuous: (BOOL)continuous {
+- (void) testMaxAttempt: (int) attempt count: (int)count continuous: (BOOL)continuous {
     XCTestExpectation* exp = [self expectationWithDescription: @"replicator finish"];
     CBLReplicatorConfiguration* config = [self configWithTarget: kConnRefusedTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: continuous];
     __block int offlineCount = 0;
-    if (retry >= 0)
-        config.maxRetries = retry;
+    config.maxAttempts = attempt;
 
     repl = [[CBLReplicator alloc] initWithConfig: config];
     [repl addChangeListener: ^(CBLReplicatorChange * c) {
@@ -1842,73 +1827,75 @@ - (void) testMaxRetry: (int) retry count: (int)count continuous: (BOOL)continuou
     AssertEqual(offlineCount, count);
 }
 
-- (void) testMaxRetry {
-    [self testMaxRetry: 0 count: 0 continuous: NO];
-    [self testMaxRetry: 0 count: 0 continuous: YES];
-
-    [self testMaxRetry: 1 count: 1 continuous: NO];
-    [self testMaxRetry: 1 count: 1 continuous: YES];
+- (void) testMaxAttempt {
+    // replicator with no retry; only initial request
+    [self testMaxAttempt: 1 count: 0 continuous: NO];
+    [self testMaxAttempt: 1 count: 0 continuous: YES];
+
+    // replicator with one retry; initial + one retry(offline)
+    [self testMaxAttempt: 2 count: 1 continuous: NO];
+    [self testMaxAttempt: 2 count: 1 continuous: YES];
 }
 
-// disbale the test, since this might take ~13mints
-- (void) _testMaxRetryForSingleShot {
-    [self testMaxRetry: -1 count: 9 continuous: NO];
+// disbale the test, since this test will take 13mints to finish
+- (void) _testMaxAttemptForSingleShot {
+    [self testMaxAttempt: 0 count: 9 continuous: NO];
 }
 
-#pragma mark - Max Retry Wait Time
+#pragma mark - Max Attempt Wait Time
 
-- (void) testMaxRetryWaitTime {
+- (void) testMaxAttemptWaitTime {
     // single shot
     CBLReplicatorConfiguration* config = [self configWithTarget: kDummyTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: NO];
-    AssertEqual(config.maxRetryWaitTime, 300);
+    AssertEqual(config.maxAttemptWaitTime, 0);
     repl = [[CBLReplicator alloc] initWithConfig: config];
-    AssertEqual(repl.config.maxRetryWaitTime, 300);
+    AssertEqual(repl.config.maxAttemptWaitTime, 0);
 
     // continuous
     config = [self configWithTarget: kDummyTarget
                                type: kCBLReplicatorTypePush
                          continuous: YES];
-    AssertEqual(config.maxRetryWaitTime, 300);
+    AssertEqual(config.maxAttemptWaitTime, 0);
     repl = [[CBLReplicator alloc] initWithConfig: config];
-    AssertEqual(repl.config.maxRetryWaitTime, 300);
+    AssertEqual(repl.config.maxAttemptWaitTime, 0);
 
     repl = nil;
 }
 
-- (void) testCustomMaxRetryWaitTime {
+- (void) testCustomMaxAttemptWaitTime {
     // single shot
     CBLReplicatorConfiguration* config = [self configWithTarget: kDummyTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: NO];
-    config.maxRetryWaitTime = 444;
-    AssertEqual(config.maxRetryWaitTime, 444);
+    config.maxAttemptWaitTime = 444;
+    AssertEqual(config.maxAttemptWaitTime, 444);
 
     // continuous
     config = [self configWithTarget: kDummyTarget
                                type: kCBLReplicatorTypePush
                          continuous: YES];
-    config.maxRetryWaitTime = 444;
-    AssertEqual(config.maxRetryWaitTime, 444);
+    config.maxAttemptWaitTime = 444;
+    AssertEqual(config.maxAttemptWaitTime, 444);
 }
 
-- (void) testInvalidMaxRetryWaitTime {
+- (void) testInvalidMaxAttemptWaitTime {
     CBLReplicatorConfiguration* config = [self configWithTarget: kDummyTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: NO];
     [self expectException: @"NSInvalidArgumentException" in:^{
-        config.maxRetryWaitTime = -1;
+        config.maxAttemptWaitTime = -1;
     }];
 }
 
-- (void) testMaxRetryWaitTimeOfReplicator {
+- (void) testMaxAttemptWaitTimeOfReplicator {
     XCTestExpectation* exp = [self expectationWithDescription: @"replicator finish"];
     CBLReplicatorConfiguration* config = [self configWithTarget: kConnRefusedTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: NO];
-    config.maxRetryWaitTime = 2;
-    config.maxRetries = 4;
+    config.maxAttemptWaitTime = 2;
+    config.maxAttempts = 3;
     repl = [[CBLReplicator alloc] initWithConfig: config];
     __block NSDate* begin = [NSDate date];
     __block NSTimeInterval diff;
@@ -1922,11 +1909,9 @@ - (void) testMaxRetryWaitTimeOfReplicator {
     }];
     [repl start];
     [self waitForExpectations: @[exp] timeout: timeout];
-    Assert(ABS(diff - config.maxRetryWaitTime) < 1.0);
+    Assert(ABS(diff - config.maxAttemptWaitTime) < 1.0);
 }
 
-#pragma mark - Max Retry Wait Time
-
 # pragma mark - CBLDocumentReplication
 
 - (void) testCreateDocumentReplicator {
@@ -1979,8 +1964,8 @@ - (void) testListenerAddRemoveAfterReplicatorStart {
     CBLReplicatorConfiguration* config = [self configWithTarget: kConnRefusedTarget
                                                            type: kCBLReplicatorTypePush
                                                      continuous: NO];
-    config.maxRetryWaitTime = 2;
-    config.maxRetries = 4;
+    config.maxAttemptWaitTime = 2;
+    config.maxAttempts = 4;
     repl = [[CBLReplicator alloc] initWithConfig: config];
     id token1 = [repl addChangeListener: ^(CBLReplicatorChange * c) {
         if (c.status.activity == kCBLReplicatorStopped) {

diff --git a/Objective-C/Tests/ReplicatorTest.h b/Objective-C/Tests/ReplicatorTest.h
@@ -83,7 +83,7 @@ NS_ASSUME_NONNULL_BEGIN
                                       continuous: (BOOL)continuous
                                    authenticator: (nullable CBLAuthenticator*)authenticator
                                       serverCert: (nullable SecCertificateRef)serverCert
-                                      maxRetries: (NSInteger)maxRetries;
+                                     maxAttempts: (NSInteger)maxAttempts;
 
 #ifdef COUCHBASE_ENTERPRISE
 - (CBLReplicatorConfiguration*) configWithTarget: (id<CBLEndpoint>)target
@@ -124,7 +124,7 @@ onReplicatorReady: (nullable void (^)(CBLReplicator*))onReplicatorReady;
             continuous: (BOOL)continuous
          authenticator: (nullable CBLAuthenticator*)authenticator
             serverCert: (nullable SecCertificateRef)serverCert
-            maxRetries: (NSInteger)maxRetries
+           maxAttempts: (NSInteger)maxAttempts
              errorCode: (NSInteger)errorCode
            errorDomain: (nullable NSString*)errorDomain;