Skip to content

Commit 3dcb4e0

Browse files
committed
Add tests
1 parent 13e4d1a commit 3dcb4e0

File tree

3 files changed

+591
-72
lines changed

3 files changed

+591
-72
lines changed

Diff for: Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift

+144-72
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
import NIOCore
16+
import XCTest
1617

1718
extension HTTPConnectionPool {
1819
/// Represents the state of a single HTTP/1.1 connection
@@ -219,7 +220,7 @@ extension HTTPConnectionPool {
219220

220221
var startingGeneralPurposeConnections: Int {
221222
var connecting = 0
222-
for connectionState in self.connections {
223+
for connectionState in self.connections[0..<self.overflowIndex] {
223224
if connectionState.isConnecting || connectionState.isBackingOff {
224225
connecting += 1
225226
}
@@ -236,7 +237,18 @@ extension HTTPConnectionPool {
236237
}
237238
}
238239

239-
// MARK: - Internal functions -
240+
// MARK: - Mutations -
241+
242+
enum NextConnectionAction<Request> {
243+
/// Directly lease the connection again
244+
case lease(Connection, Request)
245+
/// Start an idle timeout timer and make connection available to new requests
246+
case park(Connection.ID)
247+
/// Close the connection and remove
248+
case close(Connection)
249+
}
250+
251+
// MARK: Connection creation
240252

241253
mutating func createNewConnection(on eventLoop: EventLoop) -> Connection.ID {
242254
let connection = HTTP1ConnectionState(connectionID: self.generator.next(), eventLoop: eventLoop)
@@ -251,13 +263,16 @@ extension HTTPConnectionPool {
251263
return connection.connectionID
252264
}
253265

254-
mutating func newHTTP1ConnectionCreated(_ connection: Connection) -> Connection.ID {
266+
mutating func newHTTP1ConnectionCreated<RequestType>(
267+
_ connection: Connection,
268+
_ closure: (IdleConnectionContext) -> (IdleConnectionAction<RequestType>)
269+
) -> NextConnectionAction<RequestType> {
255270
guard let index = self.connections.firstIndex(where: { $0.connectionID == connection.id }) else {
256271
preconditionFailure("There is a new connection that we didn't request!")
257272
}
258-
273+
assert(connection.eventLoop === self.connections[index].eventLoop, "Expected the new connection to be on EL")
259274
self.connections[index].started(connection)
260-
return connection.id
275+
return self.withIdleConnection(index: index, closure: closure)
261276
}
262277

263278
mutating func backoffNextConnectionAttempt(_ connectionID: Connection.ID) -> EventLoop {
@@ -269,19 +284,114 @@ extension HTTPConnectionPool {
269284
return self.connections[index].eventLoop
270285
}
271286

272-
/// Information around the failed/closed connection.
273-
struct FailedConnectionContext {
274-
/// The failed connection's use. Did it serve in the pool or was it specialized for an `EventLoop`?
275-
enum PreviousUse {
276-
case generalPurpose
277-
case eventLoop
287+
// MARK: Leasing and releasing
288+
289+
mutating func leaseConnection(onPreferred preferredEL: EventLoop) -> Connection? {
290+
guard let index = self.findAvailableConnection(onPreferred: preferredEL) else {
291+
return nil
292+
}
293+
294+
return self.connections[index].lease()
295+
}
296+
297+
mutating func leaseConnection(onRequired preferredEL: EventLoop) -> Connection? {
298+
guard let index = self.findAvailableConnection(onRequired: preferredEL) else {
299+
return nil
300+
}
301+
302+
return self.connections[index].lease()
303+
}
304+
305+
mutating func releaseConnection<RequestType>(
306+
_ connectionID: Connection.ID,
307+
_ closure: (IdleConnectionContext) -> (IdleConnectionAction<RequestType>)
308+
) -> NextConnectionAction<RequestType> {
309+
guard let index = self.connections.firstIndex(where: { $0.connectionID == connectionID }) else {
310+
preconditionFailure("A connection that we don't know was released? Something is very wrong...")
311+
}
312+
313+
self.connections[index].release()
314+
return self.withIdleConnection(index: index, closure: closure)
315+
}
316+
317+
struct IdleConnectionContext {
318+
/// The `EventLoop` the connection runs on.
319+
var eventLoop: EventLoop
320+
/// The connection's use. Either general purpose or for requests with `EventLoop`
321+
/// requirements.
322+
var use: ConnectionUse
323+
}
324+
325+
enum IdleConnectionAction<R> {
326+
/// Directly lease the connection again
327+
case lease(for: R)
328+
/// Start an idle timeout timer and make connection available to new requests
329+
case park
330+
/// Close the connection and remove
331+
case close
332+
}
333+
334+
private mutating func withIdleConnection<RequestType>(
335+
index: Int,
336+
closure: (IdleConnectionContext) -> (IdleConnectionAction<RequestType>)
337+
) -> NextConnectionAction<RequestType> {
338+
let eventLoop = self.connections[index].eventLoop
339+
let use: ConnectionUse
340+
if index < self.overflowIndex {
341+
use = .generalPurpose
342+
} else {
343+
use = .eventLoop(eventLoop)
344+
}
345+
346+
let context = IdleConnectionContext(eventLoop: eventLoop, use: use)
347+
switch closure(context) {
348+
case .close:
349+
if index < self.overflowIndex {
350+
self.overflowIndex -= 1
351+
}
352+
var connectionState = self.connections.remove(at: index)
353+
return .close(connectionState.close())
354+
355+
case .lease(for: let request):
356+
return .lease(self.connections[index].lease(), request)
357+
358+
case .park:
359+
return .park(self.connections[index].connectionID)
360+
}
361+
}
362+
363+
// MARK: Connection close/removal
364+
365+
mutating func closeConnectionIfIdle(_ connectionID: Connection.ID) -> Connection? {
366+
guard let index = self.connections.firstIndex(where: { $0.connectionID == connectionID }) else {
367+
// because of a race this connection (connection close runs against trigger of timeout)
368+
// was already removed from the state machine.
369+
return .none
370+
}
371+
372+
guard self.connections[index].isAvailable else {
373+
// connection is not available anymore, we may have just leased it for a request
374+
return nil
278375
}
279376

377+
return self.closeConnection(at: index)
378+
}
379+
380+
// MARK: Connection failure
381+
382+
/// A connection's use. Did it serve in the pool or was it specialized for an `EventLoop`?
383+
enum ConnectionUse {
384+
case generalPurpose
385+
case eventLoop(EventLoop)
386+
}
387+
388+
/// Information around the failed/closed connection.
389+
struct FailedConnectionContext {
280390
/// The eventLoop the connection ran on.
281391
var eventLoop: EventLoop
282392
/// The failed connection's use.
283-
var use: PreviousUse
284-
/// Connections that we start up for this usecase
393+
var use: ConnectionUse
394+
/// Connections that we start up for this use-case
285395
var connectionsStartingForUseCase: Int
286396
}
287397

@@ -292,6 +402,15 @@ extension HTTPConnectionPool {
292402
case remove
293403
}
294404

405+
/// Remove or replace a connection.
406+
///
407+
/// Call this function after the connection start backoff is done, or a live connection is closed.
408+
///
409+
/// - Parameters:
410+
/// - connectionID: The failed connection's ID
411+
/// - closure: A closure in which a decision is made how to progress with this connection spot
412+
///
413+
/// - Returns: A new connectionID and eventLoop to create a new connection, if the action is `.replace`
295414
mutating func failConnection(
296415
_ connectionID: Connection.ID,
297416
_ closure: (FailedConnectionContext) -> (FailedConnectionAction)
@@ -300,15 +419,17 @@ extension HTTPConnectionPool {
300419
preconditionFailure("We tried to create a new connection that we know nothing about?")
301420
}
302421

303-
let use: FailedConnectionContext.PreviousUse
422+
let use: ConnectionUse
304423
let eventLoop = self.connections[index].eventLoop
305424
let starting: Int
306425
if index < self.overflowIndex {
307426
use = .generalPurpose
308-
starting = self.startingGeneralPurposeConnections
427+
// we need to subtract 1, since we dont want this connection to count
428+
starting = self.startingGeneralPurposeConnections - 1
309429
} else {
310-
use = .eventLoop
311-
starting = self.startingEventLoopConnections(on: eventLoop)
430+
use = .eventLoop(eventLoop)
431+
// we need to subtract 1, since we dont want this connection to count
432+
starting = self.startingEventLoopConnections(on: eventLoop) - 1
312433
}
313434

314435
let context = FailedConnectionContext(
@@ -335,67 +456,18 @@ extension HTTPConnectionPool {
335456
}
336457
}
337458

338-
mutating func leaseConnection(onPreferred preferredEL: EventLoop) -> Connection? {
339-
guard let index = self.findAvailableConnection(onPreferred: preferredEL) else {
340-
return nil
341-
}
342-
343-
return self.connections[index].lease()
344-
}
345-
346-
mutating func leaseConnection(onRequired preferredEL: EventLoop) -> Connection? {
347-
guard let index = self.findAvailableConnection(onRequired: preferredEL) else {
348-
return nil
349-
}
350-
351-
return self.connections[index].lease()
352-
}
353-
354-
mutating func releaseConnection(_ connectionID: Connection.ID) {
355-
guard let index = self.connections.firstIndex(where: { $0.connectionID == connectionID }) else {
356-
preconditionFailure("A connection that we don't know was released? Something is very wrong...")
357-
}
358-
359-
self.connections[index].release()
360-
}
361-
362-
/// Remove deletes an already closed connection from the connection pool.
363-
mutating func removeConnection(_ connectionID: Connection.ID) {
364-
guard let index = self.connections.firstIndex(where: { $0.connectionID == connectionID }) else {
365-
preconditionFailure("We tried to create a new connection, that we know nothing about?")
366-
}
367-
self.connections.remove(at: index)
368-
self.overflowIndex = self.connections.index(before: self.overflowIndex)
369-
}
370-
371-
mutating func closeConnectionIfIdle(_ connectionID: Connection.ID) -> Connection? {
372-
guard let index = self.connections.firstIndex(where: { $0.connectionID == connectionID }) else {
373-
// because of a race this connection (connection close runs against trigger of timeout)
374-
// was already removed from the state machine.
375-
return .none
376-
}
377-
378-
guard self.connections[index].isAvailable else {
379-
// connection is not available anymore, we may have just leased it for a request
380-
return nil
381-
}
382-
383-
return self.closeConnection(at: index)
384-
}
385-
386-
mutating func closeConnection(_ connectionID: Connection.ID) -> Connection {
387-
guard let index = self.connections.firstIndex(where: { $0.connectionID == connectionID }) else {
388-
preconditionFailure("We tried to create a new connection, that we know nothing about?")
389-
}
390-
return self.closeConnection(at: index)
391-
}
459+
// MARK: Shutdown
392460

393461
mutating func shutdown() -> CleanupContext {
394462
var cleanupContext = CleanupContext()
463+
let initialOverflowIndex = self.overflowIndex
395464

396-
self.connections = self.connections.compactMap { connectionState in
465+
self.connections = self.connections.enumerated().compactMap { index, connectionState in
397466
var connectionState = connectionState
398467
if connectionState.cleanup(&cleanupContext) {
468+
if index < initialOverflowIndex {
469+
self.overflowIndex -= 1
470+
}
399471
return nil
400472
}
401473
return connectionState

0 commit comments

Comments
 (0)