server: drain accepted Gorums work before graceful shutdown

Open
  • Is something wrong?
#444 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Active
Tech stack
go, grpc
Domain
backend-api-design, distributed-systems

Research direction

Start by reviewing the shutdown and GOAWAY handling discussed in #402, the framework error reporting work in #443, and related issue #205. The issue does not name implementation files or tests; first trace how NodeStream dispatches requests and tracks handlers, then identify the stream lifecycle paths for ordinary clients, local calls, back channels, and shared peer streams. Done means defining and implementing a bounded drain protocol that covers the listed queue, handler, reply, restart, and compatibility cases, with validation for each.

Written by the indexing model from the issue text.

Description

enhancement

Problem

Graceful shutdown must account for individual Gorums requests carried by a long-lived NodeStream.
Waiting for the outer gRPC stream to finish does not ensure that its Gorums handlers have finished.

The current GOAWAY handling drains a client session until no calls await replies, then cancels it.
One-way calls have no pending reply entry, so that condition does not account for their queued or running handlers.
A review of #402 reproduced GracefulStop returning while an accepted one-way handler was still blocked.
The dispatcher can also discard queued requests when the channel ends.

Goal

Add a Gorums-level draining protocol that stops new requests and finishes accepted work before streams close.
It must work for one-way calls without adding a success acknowledgement to every request.

Proposed protocol

  1. Mark the server as draining and notify senders before transport shutdown can cancel their streams.
  2. On notification, the sending runtime stops submitting new requests to that server.
    Requests that have not been sent fail with a recognizable framework error indicating that the server is draining.
  3. Establish an ordered cutoff for requests already in flight, for example with a final-send marker from the sender.
    Define which requests the server accepts for draining and how it rejects requests outside that boundary.
  4. Finish accepted queued and running handlers.
    ServerContext.Release allows the next handler to start; it must not count as handler completion.
  5. Flush outstanding replies and report drain completion before closing the stream.
    Senders keep the stream available for this exchange even when no application calls await replies.
  6. Bound shutdown with a deadline and a force-stop fallback.
sequenceDiagram
    participant C as Sending runtime
    participant S as Draining server
    S->>C: Server is draining
    Note over C: Stop new sends to this server
    C->>S: Final-send marker after in-flight requests
    Note over S: Finish accepted handlers and flush replies
    S->>C: Drain complete
    Note over C,S: Close stream

This is a proposed exchange; the exact wire representation and cutoff rules need to be decided during implementation.
Keep ordinary GOAWAY connection rotation distinct from server shutdown, and define how a sender resumes after the server restarts.
Cover ordinary clients, local calls, back channels, and both directions of shared peer streams.
Ordinary client streams need lifecycle tracking even though they are absent from ConnectedClients.

Framework error reporting

#443 describes forwarding framework errors to the caller and logging or counting them on the caller side when no pending response channel exists.
A draining notification or rejection could use that mechanism to signal one-way senders.
The runtime must act on shutdown signals regardless of whether diagnostic logging is enabled.
An error arriving after a one-way Send or Wait has returned cannot change that earlier result.
An error notification alone does not establish the cutoff or prove that accepted work has finished.

Validation

Exercise shutdown with queued one-way requests, running handlers, and handlers that release before returning.
Verify that new sends fail once the sender learns of draining, while accepted work can finish.
Cover full queues, shared streams, restart, and ordinary connection rotation.
Control messages must not be silently discarded when a reply queue is full.
Account for #399: a full dispatch queue can delay reading a control frame on the same stream, so a control message alone does not guarantee progress.
Define behavior for peers without drain-protocol support and verify that the shutdown deadline bounds the wait.

Related: #205 and #402.

Dominant language
Go
Stars
149
Forks
15
Avg merge
1d 54m
Merged PRs (30d)
53

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from relab/gorums

All issues in relab/gorums

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.