Stateful host statuses (#157)

* Stateful host statuses

The client now keeps an up/down status for each of its hosts, and avoids targeting down hosts, unless their status is considered obsolete, as per the time frame indicated by `hostStatusesTimeout`.

This way, if a host is down for any reason (DNS resolution, routing problem, server down…), it will be ignored for subsequent requests, until the status timeout has expired.

* Increase host status timeout to 5 minutes

[ci skip]

Author: Clément Le Provost

Source/AbstractClient.swift

@@ -51,10 +51,38 @@ import Foundation
 }
 
 
+/// Describes what is the last known status of a given API host.
+///
+internal struct HostStatus {
+    /// Whether the host is "up" or "down".
+    /// "Up" means it answers normally, "down" means that it doesn't. This does not distinguish between the different
+    /// kinds of retriable failures: it could be DNS resolution failure, no route to host, response timeout, or server 
+    /// error. A non-retriable failure (e.g. `400 Bad Request`) is not considered for the "down" state.
+    ///
+    var up: Bool
+
+    /// When the status was last modified.
+    /// This is normally the moment when the client receives the response (or error).
+    ///
+    var lastModified: Date
+}
+
+
 /// An abstract API client.
 ///
 /// + Warning: Not meant to be used directly. See `Client` or `PlacesClient` instead.
 ///
+/// ## Stateful hosts
+///
+/// In order to avoid hitting timeouts at every request when one or more hosts are not working properly (whatever the
+/// reason: DNS failure, no route to host, server down...), the client maintains a **known status** for each host.
+/// That status can be either *up*, *down* or *unknown*. Initially, all hosts are in the *unknown* state. Then a given
+/// host's status is updated whenever a request to it returns a response or an error.
+///
+/// When a host is flagged as *down*, it will not be considered for subsequent requests. However, to avoid discarding
+/// hosts permanently, statuses are only remembered for a given timeframe, indicated by `hostStatusTimeout`. (You may
+/// adjust it as needed, although the default value `defaultHostStatusTimeout` should make sense for most applications.)
+///
 @objc public class AbstractClient : NSObject {
     // MARK: Properties
 
@@ -125,6 +153,21 @@ import Foundation
         }
     }
 
+    /// The last known statuses of hosts.
+    /// If a host is absent from this dictionary, it means its status is unknown.
+    ///
+    /// + Note: Hosts are never removed from this dictionary, which is a potential memory leak in theory, but does not
+    ///   matter in practice, because (1) the host arrays are provided at init time and seldom updated and (2) very
+    ///   short anyway.
+    ///
+    internal var hostStatuses: [String: HostStatus] = [:]
+
+    /// The timeout for host statuses.
+    @objc public var hostStatusTimeout: TimeInterval = defaultHostStatusTimeout
+
+    /// GCD queue to synchronize access to `hostStatuses`.
+    internal var hostStatusQueue = DispatchQueue(label: "AbstractClient.hostStatusQueue")
+    
     // NOTE: Not constant only for the sake of mocking during unit tests.
     var session: URLSession
 
@@ -137,6 +180,11 @@ import Foundation
     /// Dispatch queue used to run completion handlers.
     internal var completionQueue = DispatchQueue.main
 
+    // MARK: Constant
+    
+    /// The default timeout for host statuses.
+    @objc public static let defaultHostStatusTimeout: TimeInterval = 5 * 60
+    
     // MARK: Initialization
 
     internal init(appID: String?, apiKey: String?, readHosts: [String], writeHosts: [String]) {
@@ -241,7 +289,39 @@ import Foundation
     /// Create a request with this client's settings.
     func newRequest(method: HTTPMethod, path: String, body: JSONObject?, hostnames: [String], isSearchQuery: Bool = false, completion: CompletionHandler? = nil) -> Request {
         let currentTimeout = isSearchQuery ? searchTimeout : timeout
-        let request = Request(session: session, method: method, hosts: hostnames, firstHostIndex: 0, path: path, headers: headers, jsonBody: body, timeout: currentTimeout, completion:  completion)
+        let request = Request(client: self, method: method, hosts: hostnames, firstHostIndex: 0, path: path, headers: headers, jsonBody: body, timeout: currentTimeout, completion:  completion)
         return request
     }
+
+    /// Filter a list of hosts according to the currently known statuses, keeping only those that are up or unknown.
+    ///
+    /// - parameter hosts: The list of hosts to filter.
+    /// - returns: A filtered list of hosts, or the original list if the result of filtering would be empty.
+    ///
+    func upOrUnknownHosts(_ hosts: [String]) -> [String] {
+        assert(!hosts.isEmpty)
+        let now = Date()
+        let filteredHosts = hostStatusQueue.sync {
+            return hosts.filter { (host) -> Bool in
+                if let status = self.hostStatuses[host] { // known status
+                    return status.up || now.timeIntervalSince(status.lastModified) >= self.hostStatusTimeout // include if up or obsolete
+                } else { // unknown status
+                    return true // always include
+                }
+            }
+        }
+        // Avoid returning an empty list.
+        return filteredHosts.isEmpty ? hosts : filteredHosts
+    }
+
+    /// Update the status for a given host.
+    ///
+    /// - parameter host: The name of the host to update.
+    /// - parameter up: Whether the host is currently up (true) or down (false).
+    ///
+    func updateHostStatus(host: String, up: Bool) {
+        hostStatusQueue.sync {
+            self.hostStatuses[host] = HostStatus(up: up, lastModified: Date())
+        }
+    }
 }

Source/Request.swift

@@ -30,7 +30,8 @@ import Foundation
 /// calls into a high-level operation. This operation can be cancelled by the user.
 ///
 internal class Request: AsyncOperationWithCompletion {
-    let session: URLSession
+    /// The client to which this request is related.
+    let client: AbstractClient
 
     /// Request method.
     let method: HTTPMethod
@@ -64,10 +65,10 @@ internal class Request: AsyncOperationWithCompletion {
 
     // MARK: - Initialization
 
-    init(session: URLSession, method: HTTPMethod, hosts: [String], firstHostIndex: Int, path: String, headers: [String: String]?, jsonBody: JSONObject?, timeout: TimeInterval, completion: CompletionHandler?) {
-        self.session = session
+    init(client: AbstractClient, method: HTTPMethod, hosts: [String], firstHostIndex: Int, path: String, headers: [String: String]?, jsonBody: JSONObject?, timeout: TimeInterval, completion: CompletionHandler?) {
+        self.client = client
         self.method = method
-        self.hosts = hosts
+        self.hosts = client.upOrUnknownHosts(hosts)
         assert(!hosts.isEmpty)
         self.firstHostIndex = firstHostIndex
         self.nextHostIndex = firstHostIndex
@@ -127,9 +128,10 @@ internal class Request: AsyncOperationWithCompletion {
         if _cancelled {
             return
         }
-        let request = createRequest(nextHostIndex)
+        let currentHostIndex = nextHostIndex
+        let request = createRequest(currentHostIndex)
         nextHostIndex = (nextHostIndex + 1) % hosts.count
-        task = session.dataTask(with: request) {
+        task = client.session.dataTask(with: request) {
             (data: Data?, response: URLResponse?, error: Error?) in
             var json: JSONObject?
             var finalError: Error? = error
@@ -162,6 +164,10 @@ internal class Request: AsyncOperationWithCompletion {
             }
             assert(json != nil || finalError != nil)
 
+            // Update host status.
+            let down = finalError != nil && finalError!.isTransient()
+            self.client.updateHostStatus(host: self.hosts[currentHostIndex], up: !down)
+            
             // Success: call completion block.
             if finalError == nil {
                 self.callCompletion(content: json, error: nil)

Tests/ClientTests.swift

@@ -405,4 +405,55 @@ class ClientTests: OnlineTestCase {
         let memorizedIndex = client.indices.object(forKey: indexName as NSString)
         XCTAssertNil(memorizedIndex)
     }
+
+    /// Test that the status of down hosts is correctly remembered.
+    func testHostStatus() {
+        let expectation = self.expectation(description: #function)
+        
+        client.readHosts[0] = uniqueAlgoliaBizHost()
+        let maxIterations = 10
+        let requestTimeout = client.searchTimeout
+        client.hostStatusTimeout = requestTimeout * (Double(maxIterations) * 2) // ensure that the status will be kept long enough
+        
+        let startTime = Date()
+        client.listIndexes(completionHandler: {
+            (content, error) -> Void in
+            if let error = error {
+                XCTFail("\(error)")
+                expectation.fulfill()
+                return
+            }
+            // Check that the timeout has been hit.
+            let stopTime = Date()
+            let duration = stopTime.timeIntervalSince(startTime)
+            XCTAssert(duration >= requestTimeout)
+
+            // Check that the failing host's status has been remembered.
+            guard let status = self.client.hostStatuses[self.client.readHosts[0]] else { XCTFail(); expectation.fulfill(); return }
+            XCTAssertFalse(status.up)
+            
+            // Check that further iterations do not hit the timeout.
+            func doTest(iteration: Int) {
+                self.client.listIndexes(completionHandler: {
+                    (content, error) -> Void in
+                    if let error = error {
+                        XCTFail("\(error)")
+                        expectation.fulfill()
+                        return
+                    }
+                    if iteration + 1 < maxIterations {
+                        doTest(iteration: iteration + 1)
+                    } else {
+                        // Check that the timeout has not been hit for all requests.
+                        let stopTime = Date()
+                        let duration = stopTime.timeIntervalSince(startTime)
+                        XCTAssert(duration < requestTimeout * Double(maxIterations + 1))
+                        expectation.fulfill()
+                    }
+                })
+            }
+            doTest(iteration: 0)
+        })
+        self.waitForExpectations(timeout: expectationTimeout, handler: nil)
+    }
 }

Tests/Helpers.swift

@@ -42,3 +42,11 @@ func safeIndexName(_ name: String) -> String {
 func average(values: [Double]) -> Double {
     return values.reduce(0, +) / Double(values.count)
 }
+
+/// Generate a new host name in the `algolia.biz` domain.
+/// The DNS lookup for any host in the `algolia.biz` domain will time-out.
+/// Generating a new host name every time avoids any system-level or network-level caching side effect.
+///
+func uniqueAlgoliaBizHost() -> String {
+    return "swift-\(UInt32(NSDate().timeIntervalSince1970)).algolia.biz"
+}

Tests/IndexTests.swift

@@ -1039,9 +1039,7 @@ class IndexTests: OnlineTestCase {
     func testDNSTimeout() {
         let expectation = self.expectation(description: #function)
 
-        // The DNS lookup for any host in the `algolia.biz` domain will time-out.
-        // We generate a new host name every time to avoid any cache effect.
-        client.readHosts[0] = "swift-\(UInt32(NSDate().timeIntervalSince1970)).algolia.biz"
+        client.readHosts[0] = uniqueAlgoliaBizHost()
 
         client.listIndexes(completionHandler: {
             (content, error) -> Void in

Tests/NetworkTests.swift

@@ -202,4 +202,58 @@ class NetworkTests: XCTestCase {
         }
         self.waitForExpectations(timeout: expectationTimeout, handler: nil)
     }
+
+    /// Test that the status of down hosts is correctly remembered.
+    func testHostStatus() {
+        let expectation = self.expectation(description: #function)
+        
+        client.hostStatusTimeout = 3
+
+        // First host is down, second is up.
+        session.responses["https://\(client.readHosts[0])/1/indexes"] = MockResponse(error: TIMEOUT_ERROR)
+        session.responses["https://\(client.readHosts[1])/1/indexes"] = MockResponse(statusCode: 200, jsonBody: ["from": "2nd host"])
+        
+        // Do a query: the 1st host will fail, and the client should transparently fallback to the 2nd host.
+        client.listIndexes(completionHandler: {
+            (content, error) -> Void in
+            // Check that the response is successful and comes from the 2nd host.
+            guard error == nil else { XCTFail(); expectation.fulfill(); return }
+            XCTAssertEqual(content?["from"] as? String, "2nd host")
+            
+            // Check that the failing host's status has been remembered.
+            guard let status = self.client.hostStatuses[self.client.readHosts[0]] else { XCTFail(); expectation.fulfill(); return }
+            XCTAssertFalse(status.up)
+            let statusTimestamp = status.lastModified
+
+            // First host is up again.
+            self.session.responses["https://\(self.client.readHosts[0])/1/indexes"] = MockResponse(statusCode: 200, jsonBody: ["from": "1st host"])
+            
+            // Do the same query again: the client should ignore the 1st host and target directly the 2nd host.
+            self.client.listIndexes(completionHandler: {
+                (content, error) -> Void in
+                // Check that the response is successful and still comes from the 2nd host.
+                guard error == nil else { XCTFail(); expectation.fulfill(); return }
+                XCTAssertEqual(content?["from"] as? String, "2nd host")
+                
+                // Wait for the down host to be forgotten.
+                Thread.sleep(forTimeInterval: self.client.hostStatusTimeout)
+                
+                // Do the same query again: the client should target the 1st host again.
+                self.client.listIndexes(completionHandler: {
+                    (content, error) -> Void in
+                    // Check that the response is successful and comes from the 1st host.
+                    guard error == nil else { XCTFail(); expectation.fulfill(); return }
+                    XCTAssertEqual(content?["from"] as? String, "1st host")
+                    
+                    // Check that the host's status has been updated.
+                    guard let status = self.client.hostStatuses[self.client.readHosts[0]] else { XCTFail(); expectation.fulfill(); return }
+                    XCTAssertTrue(status.up)
+                    XCTAssert(status.lastModified.compare(statusTimestamp) == .orderedDescending)
+                    
+                    expectation.fulfill()
+                })
+            })
+        })
+        self.waitForExpectations(timeout: expectationTimeout, handler: nil)
+    }
 }

Tests/ObjectiveCBridging.m

@@ -147,6 +147,7 @@ - (void)testClient {
     client.readHosts = [client.readHosts arrayByAddingObject:@"nowhere.net"];
     client.writeHosts = [client.writeHosts arrayByAddingObject:@"nobody.com"];
     [client setHosts:@[ @"nowhere.net", @"nobody.com", @"never.org" ]];
+    client.hostStatusTimeout = [Client defaultHostStatusTimeout];
 
     // Operations
     // ----------