-
Notifications
You must be signed in to change notification settings - Fork 126
Cleanup: Connection cancel -> shutdown #404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,89 +18,102 @@ enum HTTPConnectionPool { | |
struct Connection: Hashable { | ||
typealias ID = Int | ||
|
||
// PLEASE NOTE: | ||
// The HTTP/1.1 connection code here is commented out, for a sad and simple reason: We | ||
// don't have a HTTP1Connection yet. As soon as the HTTP1Connection has landed | ||
// (https://github.com/swift-server/async-http-client/pull/400) we will enable | ||
// HTTP1Connections here. Landing the connection box now enables us to already review the | ||
// ConnectionPool StateMachines. | ||
|
||
private enum Reference { | ||
// case http1_1(HTTP1Connection) | ||
|
||
case http1_1(HTTP1Connection) | ||
case http2(HTTP2Connection) | ||
case __testOnly_connection(ID, EventLoop) | ||
} | ||
|
||
private let _ref: Reference | ||
|
||
// fileprivate static func http1_1(_ conn: HTTP1Connection) -> Self { | ||
// Connection(_ref: .http1_1(conn)) | ||
// } | ||
fileprivate static func http1_1(_ conn: HTTP1Connection) -> Self { | ||
Connection(_ref: .http1_1(conn)) | ||
} | ||
|
||
fileprivate static func http2(_ conn: HTTP2Connection) -> Self { | ||
Connection(_ref: .http2(conn)) | ||
} | ||
|
||
static func __testOnly_connection(id: ID, eventLoop: EventLoop) -> Self { | ||
Connection(_ref: .__testOnly_connection(id, eventLoop)) | ||
} | ||
|
||
var id: ID { | ||
switch self._ref { | ||
// case .http1_1(let connection): | ||
// return connection.id | ||
case .http1_1(let connection): | ||
return connection.id | ||
case .http2(let connection): | ||
return connection.id | ||
case .__testOnly_connection(let id, _): | ||
return id | ||
} | ||
} | ||
|
||
var eventLoop: EventLoop { | ||
switch self._ref { | ||
// case .http1_1(let connection): | ||
// return connection.channel.eventLoop | ||
case .http1_1(let connection): | ||
return connection.channel.eventLoop | ||
case .http2(let connection): | ||
return connection.channel.eventLoop | ||
case .__testOnly_connection(_, let eventLoop): | ||
return eventLoop | ||
} | ||
} | ||
|
||
@discardableResult | ||
fileprivate func close() -> EventLoopFuture<Void> { | ||
fileprivate func execute(request: HTTPExecutableRequest) { | ||
switch self._ref { | ||
// case .http1_1(let connection): | ||
// return connection.close() | ||
|
||
case .__testOnly_connection(_, let eventLoop): | ||
return eventLoop.makeSucceededFuture(()) | ||
case .http1_1(let connection): | ||
return connection.execute(request: request) | ||
case .http2(let connection): | ||
return connection.executeRequest(request) | ||
case .__testOnly_connection: | ||
break | ||
} | ||
} | ||
|
||
fileprivate func execute(request: HTTPExecutableRequest) { | ||
/// Shutdown cancels any running requests on the connection and then closes the connection | ||
fileprivate func shutdown() { | ||
switch self._ref { | ||
// case .http1_1(let connection): | ||
// return connection.execute(request: request) | ||
case .http1_1(let connection): | ||
return connection.shutdown() | ||
case .http2(let connection): | ||
return connection.shutdown() | ||
case .__testOnly_connection: | ||
break | ||
} | ||
} | ||
|
||
fileprivate func cancel() { | ||
/// Closes the connection without cancelling running requests. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens to the requests if we don't cancel them? Why would do use this over shutdown? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the pool state machine, we know if a connection is idle or not. For this reason, we can use the direct way to close, if we are sure nothing is running on the connection. |
||
fileprivate func close() -> EventLoopFuture<Void> { | ||
switch self._ref { | ||
// case .http1_1(let connection): | ||
// return connection.cancel() | ||
case .__testOnly_connection: | ||
break | ||
case .http1_1(let connection): | ||
return connection.close() | ||
case .http2(let connection): | ||
return connection.close() | ||
case .__testOnly_connection(_, let eventLoop): | ||
return eventLoop.makeSucceededFuture(()) | ||
} | ||
} | ||
|
||
static func == (lhs: HTTPConnectionPool.Connection, rhs: HTTPConnectionPool.Connection) -> Bool { | ||
switch (lhs._ref, rhs._ref) { | ||
// case (.http1_1(let lhsConn), .http1_1(let rhsConn)): | ||
// return lhsConn === rhsConn | ||
case (.http1_1(let lhsConn), .http1_1(let rhsConn)): | ||
return lhsConn.id == rhsConn.id | ||
case (.http2(let lhsConn), .http2(let rhsConn)): | ||
return lhsConn.id == rhsConn.id | ||
case (.__testOnly_connection(let lhsID, let lhsEventLoop), .__testOnly_connection(let rhsID, let rhsEventLoop)): | ||
return lhsID == rhsID && lhsEventLoop === rhsEventLoop | ||
// default: | ||
// return false | ||
default: | ||
return false | ||
} | ||
} | ||
|
||
func hash(into hasher: inout Hasher) { | ||
switch self._ref { | ||
case .http1_1(let conn): | ||
hasher.combine(conn.id) | ||
case .http2(let conn): | ||
hasher.combine(conn.id) | ||
case .__testOnly_connection(let id, let eventLoop): | ||
hasher.combine(id) | ||
hasher.combine(eventLoop.id) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's some mildly irritating asymmetry :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.