-
Notifications
You must be signed in to change notification settings - Fork 449
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
guides/features: add document for how to gracefully shutdown server #1388
base: main
Are you sure you want to change the base?
Conversation
7023c5a
to
cd5c53b
Compare
cd5c53b
to
1735602
Compare
1735602
to
d2aa9ab
Compare
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.
Thanks Purnesh, great to see this!
Generally looks good - left a few comments on some tweaks.
Let's have @dfawley also check this to make sure the details make sense from a cross-language perspective.
| Python | | | ||
|
||
[Go example]: https://github.com/grpc/grpc-go/tree/master/examples/features/gracefulstop | ||
[grpc doc]: https://github.com/grpc/grpc/blob/master/doc/server-graceful-stop.md |
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.
This link takes me to a 404.
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.
i think the link will work only after merge? That's what i see in other guides.
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.
FWIW, you can use the "deploy preview" at the bottom to view the page as it would appear on grpc.io
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.
I have removed the self link
@@ -0,0 +1,86 @@ | |||
--- | |||
title: GracefulStop |
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.
We would want the title to be "Graceful Stop".
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.
I would suggest "Gracefully stopping a server"
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.
Change to "Gracefully stopping a server"
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.
Graceful Shutdown
?
If you look at the website preview, this becomes the second guide that word wraps. I'd rather avoid that if at all possible.
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.
Changed to Graceful Shutdown
|
||
gRPC servers often need to shut down gracefully, ensuring that in-flight RPCs | ||
are completed within a reasonable timeframe and new RPCs are no longer | ||
accepted. `GracefulStop()` facilitates this process, allowing the server to |
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.
I know you are using "GracefulStop()" here as a placeholder for the language specific functions, but I still think it might still lead readers to look for a function with this exact name. How about just referring it to as the "graceful stop function"? It's more wordy but can avoid a potential confusion.
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.
Changed to "Graceful Shutdown Function"
|
||
The following shows the sequence of events that occur, when a server graceful | ||
shutdown is invoked while active RPCs are being process and client is sending | ||
new RPCs. |
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.
Can you also mention here the two alternative scenarios the diagram covers?
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.
Added
Other languages use "shutdown" for the stopping functionality. In Java, forceful stop is |
Having a self-referential link for "detailed semantics" doesn't make sense
to me. It should either reference another file that does have details or a
section of this document with details. Otherwise, just remove it.
…On Tue, Dec 10, 2024 at 10:47 AM Purnesh Dixit ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In content/en/docs/guides/server-graceful-stop.md
<#1388 (comment)>:
> +### Alternatives
+
+- Forceful Shutdown: Immediately terminates the server using `Stop()` on server
+ object, potentially interrupting active RPCs and leading to errors on the
+ client-side. Use only as a last resort.
+
+### Language Support
+
+| Language | Example |
+|----------|-------------------|
+| Java | |
+| Go | [Go example] |
+| Python | |
+
+[Go example]: https://github.com/grpc/grpc-go/tree/master/examples/features/gracefulstop
+[grpc doc]: https://github.com/grpc/grpc/blob/master/doc/server-graceful-stop.md
i think the link will work only after merge? That's what i see in other
guides.
—
Reply to this email directly, view it on GitHub
<#1388 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZQMCXV4WSWAHTUV2WKINJD2E4ZNTAVCNFSM6AAAAABTLVFWUSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOJTGI3TSOJVGA>
.
You are receiving this because you commented.Message ID: <grpc/grpc.
io/pull/1388/review/2493279950 ***@***.***>
|
yeah removed. I saw in wait-for-ready document so thought that was something all guides have to do. |
in the title explaination i mentioned Shutdown/Stop |
301668c
to
4f1bf98
Compare
deadline is reached. Once all active RPCs finish or the deadline expires, the | ||
server shuts down completely. | ||
|
||
### How to use Wait-for-Ready |
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.
Need to replace "Wait-for-Ready"
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
|
||
- Initiating the graceful shutdown process by calling "Graceful shutdown | ||
Function" on your gRPC server object. This function blocks until all | ||
currently running RPCs completed. This ensures that in-flight requests are |
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.
s/completed/complete/
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.
Done
finish. It's crucial to call "Forceful Shutdown Function" on server object | ||
with a timeout before calling "Graceful shutdown function". This acts as a |
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.
These need to be reversed, first graceful, then forceful.
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.
Oh this is intentional. So, forceful shutdown function needs to be scheduled to execute in a separate thread after some time because graceful shutdown is a blocking call. In go we use a timeAfter function which executes the forceful shutdown in a new goroutine after the specified time passes
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.
This is very golang specific. Other languages would initiate a graceful shutdown with a timeout and then if that indicated that it timed out would call a forceful shutdown. For example, in Java it looks like:
server.shutdown();
try {
// Wait for RPCs to complete processing
if (!server.awaitTermination(30, TimeUnit.SECONDS)) {
// That was plenty of time. Let's cancel the remaining RPCs
server.shutdownNow();
// shutdownNow isn't instantaneous, so give a bit of time to clean resources up
// gracefully. Normally this will be well under a second.
server.awaitTermination(5, TimeUnit.SECONDS);
}
} catch (InterruptedException ex) {
server.shutdownNow();
}
I'm not sure of the best approach, because for Go it is very important to setup a timer to run the forceful stop on another thread, while for Java the stop and wait are separate commands so can be done in the same thread.
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.
I made it more generic. Let me know if this looks good to you.
in-flight RPCs don't complete within a reasonable timeframe. This prevents | ||
indefinite blocking. | ||
|
||
The following shows the sequence of events that occur, when a server graceful |
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.
s/server/server's/
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.
Done
indefinite blocking. | ||
|
||
The following shows the sequence of events that occur, when a server graceful | ||
stop is invoked, in-flight RPCs continue to prorcess but new RPCs are rejected. |
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.
Replace comma with a period.
s/prorcess but/process, but/
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.
Done
|
||
The following shows the sequence of events that occur, when a server graceful | ||
stop is invoked, in-flight RPCs continue to prorcess but new RPCs are rejected. | ||
If all in-flight RPCs are not finished in time, server is forcefully shutdown. |
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.
s/If all in-flight RPCs are not finished/If some in-flight RPCS do not finish/
s/, server/, the server/
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.
Done
### Alternatives | ||
|
||
- Forceful Shutdown: Immediately terminates the server using "Forceful Shutdown | ||
Function" on server object, potentially interrupting active RPCs and leading |
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.
s/"Forceful Shutdown Function" on server/the "Forceful Shutdown Function" on the server/
s/potentially interrupting active RPCs and leading to/any active RPCs are interrupted, which will send/
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.
Done everywhere including graceful shutdown function
|
||
| Language | Example | | ||
|----------|-------------------| | ||
| Java | | |
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.
Can point to an existing example for Java: https://github.com/grpc/grpc-java/tree/master/examples/example-hostname/src/main/java/io/grpc/examples/hostname
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.
Added
currently running RPCs complete. This ensures that in-flight requests are | ||
allowed to finish processing. | ||
- Specify a timeout period to limit the time allowed for in-progress RPCs to | ||
finish. It's crucial to call the "Forceful shutdown function" on the server |
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.
How about "It's crucial in Go to call", since this is really go specific?
Server->>Client: Processing RPC 1 | ||
Server->>Client: Processing RPC 2 |
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.
I found these lines to be confusing when initially reading. I thought they indicated the RPCs had completed. Can the same meaning be conveyed if these lines are just deleted?
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.
yeah i removed. The fact that New RPC Request 1 and New RPC Request 2 is Client->Server, it means request is received by server for processing.
Server-->>Server: Shutdown Complete | ||
Client->>Server: New RPC Request 4 (Rejected) | ||
else Timeout reached | ||
Server->>Client: RPC 2 and other pending RPCs forcefully terminated |
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.
This should clearly show the forceful shutdown function being called by the application.
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.
Updated
Server->>Client: Completes RPC 1 | ||
Server->>Client: Completes RPC 2 | ||
Server-->>Server: Shutdown Complete | ||
Client->>Server: New RPC Request 4 (Rejected) |
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.
Once a connection has been closed, RPCs aren't technically "rejected" anymore. They literally can't even be attempted.
Also should we discuss that the client will detect the server shutting down and go find other servers to send its traffic to, if at all possible?
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.
Removed the rejected part and added the client will find another server if applicable
involves: | ||
|
||
- Initiating the graceful shutdown process by calling "Graceful shutdown | ||
Function" on your gRPC server object. This function blocks until all |
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.
Let's be consistent with our capitalization please. Either "Function" everywhere when used in quotes like this, or "function" instead.
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 everywhere
|
||
### Alternatives | ||
|
||
- Forceful Shutdown: Immediately terminates the server using "Forceful Shutdown |
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.
And here "Shutdown" is capitalized.
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
|
||
- Forceful Shutdown: Immediately terminates the server using "Forceful Shutdown | ||
Function" on server object, potentially interrupting active RPCs and leading | ||
to errors on the client-side. Use only as a last resort. |
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.
This "alternative" section is awkward. And this "last resort" comment as well. Maybe the overview can just mention at the end that gracefully shutting down is not mandatory -- you can forcefully shut down without a graceful shutdown first, but that would not be recommended as it would result in waste. Or don't even mention it since it's not recommended?
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.
Removed alternative section and added a sentence in Overview
@dfawley could you do a one more pass? Thanks |
prevents indefinite blocking. | ||
|
||
The following shows the sequence of events that occur, when a server's graceful | ||
stop is invoked, in-flight RPCs continue to prorcess, but new RPCs are |
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.
*process -- please run a spell check.
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 all the spellings
b42317b
to
53a97a5
Compare
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.
One last set of changes, otherwise LGTM
--- | ||
title: Graceful Shutdown | ||
description: >- | ||
Explains how to gracefully shut down [or stop, if you prefer] a gRPC server |
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.
The brackets were there for you to read and choose either "stop" or "shut down", not to copy verbatim. :)
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.
Ah yeah. Forgot to remove. Removed now.
connections. | ||
|
||
When the "Graceful shutdown function" is called, the server immediately | ||
notifies all clients that they should stop sending new RPCs. Then after the |
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.
Nit: "that they should" -> "to"
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.
Done
Client->>Server: New RPC Request 2 | ||
Server-->>Server: Graceful Shutdown Invoked | ||
Server->>Client: Continues Processing In-Flight RPCs | ||
Client->>Client: Detects server shutdown and go finds other servers if available |
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.
Remove "go" here please.
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.
Done
The following shows the sequence of events that occur, when a server's graceful | ||
stop is invoked, in-flight RPCs continue to process, but new RPCs are | ||
rejected. If some in-flight RPCs are not finished in time, server is forcefully |
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.
This is a run-on sentence, and has an unnecessary comma. Also stop->shutdown for consistency.
The following shows the sequence of events that occur during the graceful shutdown process.
When a server's graceful shutdown is invoked, in-flight RPCs continue to process, but new RPCs are
rejected. If some.......
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.
Updated
The following shows the sequence of events that occur, when a server's graceful | ||
stop is invoked, in-flight RPCs continue to process, but new RPCs are | ||
rejected. If some in-flight RPCs are not finished in time, server is forcefully | ||
shutdown. |
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.
Verb form, so should be "shut down"
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.
Updated
[*] --> RUNNING : Server Started | ||
RUNNING --> GRACEFUL_STOP_INITIATED : Graceful Stop Called (with Timeout) | ||
GRACEFUL_STOP_INITIATED --> GRACEFUL_STOPPING | ||
GRACEFUL_STOPPING --> TERMINATED : In-Flight RPCs Completed (Before Timeout) | ||
GRACEFUL_STOPPING --> TIMER_EXPIRED : Timeout Reached | ||
TIMER_EXPIRED --> TERMINATED : Force Stop Called |
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.
[*] --> RUNNING : Server Started | |
RUNNING --> GRACEFUL_STOP_INITIATED : Graceful Stop Called (with Timeout) | |
GRACEFUL_STOP_INITIATED --> GRACEFUL_STOPPING | |
GRACEFUL_STOPPING --> TERMINATED : In-Flight RPCs Completed (Before Timeout) | |
GRACEFUL_STOPPING --> TIMER_EXPIRED : Timeout Reached | |
TIMER_EXPIRED --> TERMINATED : Force Stop Called | |
[*] --> SERVING : Server Started | |
SERVING --> GRACEFUL_SHUTDOWN : Graceful Shutdown Called (with Timeout) | |
GRACEFUL_SHUTDOWN --> TERMINATED : In-Flight RPCs Completed (Before Timeout) | |
GRACEFUL_SHUTDOWN --> TIMER_EXPIRED : Timeout Reached | |
TIMER_EXPIRED --> TERMINATED : Forceful Shutdown Called |
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.
Changed
Thanks @dfawley. I have addressed the comments. Ptal |
No description provided.