Channel shutdown process improvements: shutdown error added as an own…

Problem: Ambiguous behavior on Channel shutdown. If you consume using a Channel you cannot get the shutdown cause even if you use NotifyClose chan listener. Delivery chan could be selected (closed) earlier than close listener chan.

Scenario: Let's assume you have a connection and few consumer channels using it. The requirement is to restart the consumer job on any unexpected error... on unexpected errors/shutdowns only. When your connection closes from outside (unexpected error or gracefully closes from parallel goroutine) you have to handle a shutdown cause distinctly with no doubt. Look at this sample pseudo-code first:

delivCh, err := ch.Consume(...someConf...)
// some error processing
closeCh := ch.NotifyClose(make(chan *amqp.Error))
for {
    select {
    case msg, ok := <-delivCh:
        if !ok {
            // As we understand, delivCh could be selected earlier than closeCh and sometimes it works
                        // right in this way... but it's ok, it's normal behavior of select.
            // Deliveries chan is closed, but what's a cause? It could be a graceful shutdown then 
                        // we need no restart.
            // That's why such behaviour is ambiguous now.
        }
        // Process the message
    case err, ok := <-closeCh:
        if !ok {
            log.Println("channel closed gracefully")
            // Return nil
        } else if err != nil {
            log.Println("channel going to shutdown with an error")
            // Return an error, restart the job
        }
    }
}

In this scenario you cannot guarantee that closeCh will be selected early than delivCh.

PR proposal: To store a shutdown cause error into a Channel's field shutdownErr upon shutdown start and provide a getter method to get it any time at any place during shutdown process. New instance of Channel has default value for shutdownErr with "unexpected shutdown" Reason to provide a small guard for the future. It will not affect current behaviour because it's being set upon shutdown start, but if someday somebody will try to close the chans bypassing shutdown() func it will return a distinct error.

Hey folks,

I'm posting this on behalf of the core team.

As you have noticed, this client hasn't seen a lot of activity recently. Many users are unhappy about that and we fully recognize that it's a popular library that should be maintained more actively. There are also many community members who have contributed pull requests and haven't been merged for various reasons.

Because this client has a long tradition of "no breaking public API changes", certain reasonable changes will likely never be accepted. This is frustrating to those who have put in their time and effort into trying to improve this library.

We would like to thank @streadway for developing this client and maintaining it for a decade — that's a remarkable contribution to the RabbitMQ ecosystem. We this now is a good time to get more contributors involved.

Team RabbitMQ has adopted a "hard fork" of this client in order to give the community a place to evolve the API. Several RabbitMQ core team members will participate but we think it very much should be a community-driven effort.

What do we mean by "hard fork" and what does it mean for you? The entire history of the project is retained in the new repository but it is not a GitHub fork by design. The license remains the same 2-clause BSD. The contribution process won't change much (except that we hope to review and accept PRs reasonably quickly).

What does change is that this new fork will accept reasonable breaking API changes according to Semantic Versioning (or at least our understanding of it). At the moment the API is identical to that of streadway/amqp but the package name is different. We will begin reviewing PRs and merging them if they make sense in the upcoming weeks.

If your PR hasn't been accepted or reviewed, you are welcome to re-submit it for rabbitmq/amqp091-go. RabbitMQ core team members will evaluate the PRs currently open for streadway/amqp as time allows, and pull those that don't have any conflicts. We cannot promise that every PR would be accepted but at least we are open to changing the API going forward.

Note that it is a high season for holidays in some parts of the world, so we may be slower to respond in the next few weeks but otherwise, we are eager to review as many currently open PRs as practically possible soon.

Thank you for using RabbitMQ and contributing to this client. On behalf of the RabbitMQ core team, @chunyilyu and @michaelklishin.

streadway / amqp

Channel shutdown process improvements: shutdown error added as an own… #496