Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Examples/v2/echo/Subcommands/Collect.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct Collect: AsyncParsableCommand {
print("collect ← \(message.text)")
}

client.close()
client.beginGracefulShutdown()
}
}
}
2 changes: 1 addition & 1 deletion Examples/v2/echo/Subcommands/Expand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct Expand: AsyncParsableCommand {
}
}

client.close()
client.beginGracefulShutdown()
}
}
}
2 changes: 1 addition & 1 deletion Examples/v2/echo/Subcommands/Get.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct Get: AsyncParsableCommand {
print("get ← \(response.text)")
}

client.close()
client.beginGracefulShutdown()
}
}
}
2 changes: 1 addition & 1 deletion Examples/v2/echo/Subcommands/Serve.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct Serve: AsyncParsableCommand {
)

try await withThrowingDiscardingTaskGroup { group in
group.addTask { try await server.run() }
group.addTask { try await server.serve() }
if let address = try await server.listeningAddress {
print("Echo listening on \(address)")
}
Expand Down
2 changes: 1 addition & 1 deletion Examples/v2/echo/Subcommands/Update.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ struct Update: AsyncParsableCommand {
}
}

client.close()
client.beginGracefulShutdown()
}
}
}
2 changes: 1 addition & 1 deletion Examples/v2/hello-world/Subcommands/Greet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct Greet: AsyncParsableCommand {
}

defer {
client.close()
client.beginGracefulShutdown()
}

let greeter = Helloworld_GreeterClient(wrapping: client)
Expand Down
2 changes: 1 addition & 1 deletion Examples/v2/hello-world/Subcommands/Serve.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ struct Serve: AsyncParsableCommand {
)

try await withThrowingDiscardingTaskGroup { group in
group.addTask { try await server.run() }
group.addTask { try await server.serve() }
if let address = try await server.listeningAddress {
print("Greeter listening on \(address)")
}
Expand Down
2 changes: 1 addition & 1 deletion Examples/v2/route-guide/Subcommands/GetFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ struct GetFeature: AsyncParsableCommand {
print("Found '\(feature.name)' at (\(self.latitude), \(self.longitude))")
}

client.close()
client.beginGracefulShutdown()
}
}
}
2 changes: 1 addition & 1 deletion Examples/v2/route-guide/Subcommands/ListFeatures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ struct ListFeatures: AsyncParsableCommand {
}
}

client.close()
client.beginGracefulShutdown()
}

}
Expand Down
2 changes: 1 addition & 1 deletion Examples/v2/route-guide/Subcommands/RecordRoute.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ struct RecordRoute: AsyncParsableCommand {
"""
print(text)

client.close()
client.beginGracefulShutdown()
}
}
}
2 changes: 1 addition & 1 deletion Examples/v2/route-guide/Subcommands/RouteChat.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ struct RouteChat: AsyncParsableCommand {
}
}

client.close()
client.beginGracefulShutdown()
}
}
}
2 changes: 1 addition & 1 deletion Examples/v2/route-guide/Subcommands/Serve.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ struct Serve: AsyncParsableCommand {

let server = GRPCServer(transport: transport, services: [RouteGuideService(features: features)])
try await withThrowingDiscardingTaskGroup { group in
group.addTask { try await server.run() }
group.addTask { try await server.serve() }
let address = try await transport.listeningAddress
print("server listening on \(address)")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extension RouteGuide {

try await withThrowingTaskGroup(of: Void.self) { group in
group.addTask {
try await server.run()
try await server.serve()
}

if let address = try await server.listeningAddress {
Expand Down
12 changes: 6 additions & 6 deletions Sources/GRPCCore/GRPCClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ internal import Atomics
/// }
///
/// // The RPC has completed, close the client.
/// client.close()
/// client.beginGracefulShutdown()
/// }
/// ```
///
/// The ``run()`` method won't return until the client has finished handling all requests. You can
/// signal to the client that it should stop creating new request streams by calling ``close()``.
/// signal to the client that it should stop creating new request streams by calling ``beginGracefulShutdown()``.
/// This gives the client enough time to drain any requests already in flight. To stop the client
/// more abruptly you can cancel the task running your client. If your application requires
/// additional resources that need their lifecycles managed you should consider using [Swift Service
Expand Down Expand Up @@ -159,7 +159,7 @@ public struct GRPCClient: Sendable {

/// Start the client.
///
/// This returns once ``close()`` has been called and all in-flight RPCs have finished executing.
/// This returns once ``beginGracefulShutdown()`` has been called and all in-flight RPCs have finished executing.
/// If you need to abruptly stop all work you should cancel the task executing this method.
///
/// The client, and by extension this function, can only be run once. If the client is already
Expand Down Expand Up @@ -210,7 +210,7 @@ public struct GRPCClient: Sendable {
/// The transport will be closed: this means that it will be given enough time to wait for
/// in-flight RPCs to finish executing, but no new RPCs will be accepted. You can cancel the task
/// executing ``run()`` if you want to abruptly stop in-flight RPCs.
public func close() {
public func beginGracefulShutdown() {
while true {
let (wasRunning, actualState) = self.state.compareExchange(
expected: .running,
Expand All @@ -220,7 +220,7 @@ public struct GRPCClient: Sendable {

// Transition from running to stopping: close the transport.
if wasRunning {
self.transport.close()
self.transport.beginGracefulShutdown()
return
}

Expand Down Expand Up @@ -351,7 +351,7 @@ public struct GRPCClient: Sendable {

/// Start a bidirectional streaming RPC.
///
/// - Note: ``run()`` must have been called and still executing, and ``close()`` mustn't
/// - Note: ``run()`` must have been called and still executing, and ``beginGracefulShutdown()`` mustn't
/// have been called.
///
/// - Parameters:
Expand Down
14 changes: 7 additions & 7 deletions Sources/GRPCCore/GRPCServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ internal import Atomics
///
/// ```swift
/// // Start running the server.
/// try await server.run()
/// try await server.serve()
/// ```
///
/// The ``run()`` method won't return until the server has finished handling all requests. You can
/// signal to the server that it should stop accepting new requests by calling ``stopListening()``.
/// signal to the server that it should stop accepting new requests by calling ``beginGracefulShutdown()``.
/// This allows the server to drain existing requests gracefully. To stop the server more abruptly
/// you can cancel the task running your server. If your application requires additional resources
/// that need their lifecycles managed you should consider using [Swift Service
Expand Down Expand Up @@ -154,13 +154,13 @@ public struct GRPCServer: Sendable {
///
/// This function returns when the configured transport has stopped listening and all requests have been
/// handled. You can signal to the transport that it should stop listening by calling
/// ``stopListening()``. The server will continue to process existing requests.
/// ``beginGracefulShutdown()``. The server will continue to process existing requests.
///
/// To stop the server more abruptly you can cancel the task that this function is running in.
///
/// - Note: You can only call this function once, repeated calls will result in a
/// ``RuntimeError`` being thrown.
public func run() async throws {
public func serve() async throws {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit wary of renaming this from run to serve. While I understand what you are getting at. We do have many libraries that now adopted the run method name to align with what Lifecycle expects. If you rename this here and conform to Service in the future your type will have two methods that do the same.

Copy link
Collaborator

@gjcairo gjcairo Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I agree with Franz here, I think it is more important to be consistent with the rest of the ecosystem on this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run is less clear and is worse for users which don't depend on lifecycle. Users would have to provide shims to use Lifecycle anyway (to avoid retroactive conformance). If we provide conformance in the future (e.g. via package traits) then having a run() method calling through to serve() isn't an issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the perspective of a user using Lifecycle the names of these methods don't matter at all because they won't be called directly by the user.

From the perspective of a user not using Lifecycle, these methods matter a lot: they will be called by the user so they should be clear.

Copy link
Collaborator

@FranzBusch FranzBusch Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still remain unconvinced. serve and run are pretty close in my mind. Even leaving Lifecycle aside just the proliferation of run methods across the ecosystem is IMO enough to justify aligning. It becomes very natural that you call run on your server/clients when you don't use something like Lifecycle.

I don't want to block this PR on this and you can merge over my unconvincedness.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. I'm pretty strongly against aligning for the sake of aligning on a generic name: I think we should pick names that express their intent and provide the most clarity, the APIs should be approachable for all users including those new to Swift and SoS!

let (wasNotStarted, actualState) = self.state.compareExchange(
expected: .notStarted,
desired: .running,
Expand Down Expand Up @@ -209,15 +209,15 @@ public struct GRPCServer: Sendable {
/// against this server. Once the server has processed all requests the ``run()`` method returns.
///
/// Calling this on a server which is already stopping or has stopped has no effect.
public func stopListening() {
public func beginGracefulShutdown() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let (wasRunning, actual) = self.state.compareExchange(
expected: .running,
desired: .stopping,
ordering: .sequentiallyConsistent
)

if wasRunning {
self.transport.stopListening()
self.transport.beginGracefulShutdown()
} else {
switch actual {
case .notStarted:
Expand All @@ -229,7 +229,7 @@ public struct GRPCServer: Sendable {

// Lost a race with 'run()', try again.
if !exchanged {
self.stopListening()
self.beginGracefulShutdown()
}

case .running:
Expand Down
4 changes: 2 additions & 2 deletions Sources/GRPCCore/Transport/ClientTransport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public protocol ClientTransport: Sendable {
///
/// Implementations of this function will typically create a long-lived task group which
/// maintains connections. The function exits when all open streams have been closed and new connections
/// are no longer required by the caller who signals this by calling ``close()``, or by cancelling the
/// are no longer required by the caller who signals this by calling ``beginGracefulShutdown()``, or by cancelling the
/// task this function runs in.
func connect() async throws

Expand All @@ -46,7 +46,7 @@ public protocol ClientTransport: Sendable {
///
/// If you want to forcefully cancel all active streams then cancel the task
/// running ``connect()``.
func close()
func beginGracefulShutdown()

/// Opens a stream using the transport, and uses it as input into a user-provided closure.
///
Expand Down
4 changes: 2 additions & 2 deletions Sources/GRPCCore/Transport/ServerTransport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public protocol ServerTransport: Sendable {
/// and start accepting new connections. Each accepted inbound RPC stream will be handed over to
/// the provided `streamHandler` to handle accordingly.
///
/// You can call ``stopListening()`` to stop the transport from accepting new streams. Existing
/// You can call ``beginGracefulShutdown()`` to stop the transport from accepting new streams. Existing
/// streams must be allowed to complete naturally. However, transports may also enforce a grace
/// period after which any open streams may be cancelled. You can also cancel the task running
/// ``listen(_:)`` to abruptly close connections and streams.
Expand All @@ -38,5 +38,5 @@ public protocol ServerTransport: Sendable {
///
/// Existing streams are permitted to run to completion. However, the transport may also enforce
/// a grace period, after which remaining streams are cancelled.
func stopListening()
func beginGracefulShutdown()
}
12 changes: 6 additions & 6 deletions Sources/GRPCHTTP2Core/Client/Connection/GRPCChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ package final class GRPCChannel: ClientTransport {
for try await result in self.resolver.names {
self.input.continuation.yield(.handleResolutionResult(result))
}
self.close()
self.beginGracefulShutdown()
} catch {
self.close()
self.beginGracefulShutdown()
}
}

Expand Down Expand Up @@ -183,7 +183,7 @@ package final class GRPCChannel: ClientTransport {

/// Signal to the transport that no new streams may be created and that connections should be
/// closed when all streams are closed.
package func close() {
package func beginGracefulShutdown() {
self.input.continuation.yield(.close)
}

Expand Down Expand Up @@ -393,7 +393,7 @@ extension GRPCChannel {
self.updateLoadBalancer(serviceConfig: config, endpoints: result.endpoints, in: &group)

case .failure:
self.close()
self.beginGracefulShutdown()
}
}

Expand Down Expand Up @@ -567,10 +567,10 @@ extension GRPCChannel {
if let result = try await iterator.next() {
self.handleNameResolutionResult(result, in: &group)
} else {
self.close()
self.beginGracefulShutdown()
}
} catch {
self.close()
self.beginGracefulShutdown()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ extension HTTP2ClientTransport {
self.channel.configuration(forMethod: descriptor)
}

public func close() {
self.channel.close()
public func beginGracefulShutdown() {
self.channel.beginGracefulShutdown()
}

public func withStream<T: Sendable>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ extension HTTP2ServerTransport {
}
}

public func stopListening() {
public func beginGracefulShutdown() {
self.serverQuiescingHelper.initiateShutdown(promise: nil)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ extension HTTP2ServerTransport {
}
}

public func stopListening() {
public func beginGracefulShutdown() {
self.serverQuiescingHelper.initiateShutdown(promise: nil)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public final class InProcessClientTransport: ClientTransport {
/// will result in an ``RPCError`` with code ``RPCError/Code/failedPrecondition`` being thrown.
///
/// If you want to forcefully cancel all active streams then cancel the task running ``connect()``.
public func close() {
public func beginGracefulShutdown() {
let maybeContinuation: AsyncStream<Void>.Continuation? = self.state.withLock { state in
switch state {
case .unconnected:
Expand Down
6 changes: 3 additions & 3 deletions Sources/GRPCInProcessTransport/InProcessServerTransport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public import GRPCCore
///
/// To use this server, you call ``listen(_:)`` and iterate over the returned `AsyncSequence` to get all
/// RPC requests made from clients (as ``RPCStream``s).
/// To stop listening to new requests, call ``stopListening()``.
/// To stop listening to new requests, call ``beginGracefulShutdown()``.
///
/// - SeeAlso: ``ClientTransport``
@available(macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0, *)
Expand All @@ -44,7 +44,7 @@ public struct InProcessServerTransport: ServerTransport, Sendable {
///
/// - Parameter stream: The new ``RPCStream`` to publish.
/// - Throws: ``RPCError`` with code ``RPCError/Code-swift.struct/failedPrecondition``
/// if the server transport stopped listening to new streams (i.e., if ``stopListening()`` has been called).
/// if the server transport stopped listening to new streams (i.e., if ``beginGracefulShutdown()`` has been called).
internal func acceptStream(_ stream: RPCStream<Inbound, Outbound>) throws {
let yieldResult = self.newStreamsContinuation.yield(stream)
if case .terminated = yieldResult {
Expand All @@ -70,7 +70,7 @@ public struct InProcessServerTransport: ServerTransport, Sendable {
/// Stop listening to any new ``RPCStream`` publications.
///
/// - SeeAlso: ``ServerTransport``
public func stopListening() {
public func beginGracefulShutdown() {
self.newStreamsContinuation.finish()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ struct InteroperabilityTestsExecutable: AsyncParsableCommand {
),
services: [TestService()]
)
try await server.run()
try await server.serve()
}
}

Expand Down Expand Up @@ -97,7 +97,7 @@ struct InteroperabilityTestsExecutable: AsyncParsableCommand {
await self.runTest(testCase, using: client)
}

client.close()
client.beginGracefulShutdown()
}
}

Expand Down
Loading
Loading