Don't catch SIGQUIT; safer close of Quit channel

[e-dard]

Required for all non-trivial PRs

• Rebased/mergable
• Tests pass
• CHANGELOG.md updated

A SIGQUIT signal should indicate to a running process that it should exit immediately, and also dump its core. This is the default behaviour for a Go program, unless the program catches the signal using signal.Notify.

We were catching this signal in influx and then attempting to exit gracefully, which is the incorrect behaviour, especially if influx is stuck somewhere, or in a long running query.

Also, we were catching other signals that didn't make sense, e.g., SIGHUP—usually used to indicate a live configuration change, os.Kill—we're not even allowed to catch this.

This PR reduces the signals we catch down to syscall.SIGINT and syscall.SIGTERM. It also ensure that the Quit channel is closed properly.

Helps to fix #6474

[mention-bot]

By analyzing the blame information on this pull request, we identified @pires, @jsternberg and @sczk to be potential reviewers

[adamsvoboda]

Nice catch! For what it's worth, this looks good to me, although you have more authority than I do! :)

[jsternberg]

This isn't needed since this section is single threaded. The close should always succeed. If the channel is already closed, then the Quit channel will always be the one chosen by select.

[e-dard]

@jsternberg I'm not sure that's technically the case. If two signals are received in quick succession the scheduler could decide to chose the case <-c.osSignals case twice in succession.

I don't think there is anything that guarantees a closed-channel-receive will always be chosen if there are competing cases available to select over is there? I could be wrong though.

[jsternberg]

I thought there was since I had done this previously, but this example seems to indicate there isn't.

package main

import "fmt"

func main() {
    for i := 0; i < 1000; i++ {
        ch := make(chan bool, 1)
        ch <- true
        closing := make(chan struct{})
        close(closing)
        select {
        case <-closing:
        case <-ch:
            fmt.Println("bad value", i)
            return
        }
    }
}

It prints bad value fairly quickly. So never mind.

It may just be more clear to duplicate the code from the c.Quit case rather than do some semi-convoluted for loop with a select statement. Whichever change is fine though.

[e-dard]

Doh. I can completely remove the need for these selects

[pires]

The rationale behind the PR LGTM.

[jwilder]

👍