This caught me out recently; there's definitely a gap in my understanding as to how select works. Maybe it's caught other people out too?
For one of my clients, I'd written an application that would try to read data from a backend. There were multiple instances of this backend (say one in Oregon, and one in Virginia). In this context, it made sense to try reading from all of the backends concurrently, and return the first successful result. All of the slower responses could be ignored, as long as the code cleaned up any resources (goroutines, file descriptors etc). Reader, it did not. The code which demonstrates the failure is at https://github.com/jabley/fptp The code of interest is in composite.go [2] The failing version is in https://github.com/jabley/fptp/tree/bug/original-version This is the diff: --- a/composite.go +++ b/composite.go @@ -66,8 +66,7 @@ func (c *compositeSearcher) search(ctx context.Context, s Searcher, req *SearchR case <-ctx.Done(): // Another Searcher already returned a successful response. Ensure we don't leak things res.Close() - default: - respc <- searchResponse{res, err} + case respc <- searchResponse{res, err}: } } I've gone through the spec[1] and documentation, but I haven't been able to understand what I'm missing. So 2 questions: 1. Is the revised version correct? I think it is, and I've got better test coverage now. But tests aren't everything; I might still have a gap in my understanding. 2. Why is the revised version (more) correct, versus the original that used a default CommCase rather than a SendStmt? [1] https://golang.org/ref/spec#Select_statements [2] composite.go – https://github.com/jabley/fptp/blob/master/composite.go (in-line below) package fptp import ( "context" "errors" "io" "sync" "time" ) // ErrAllSearchersFailed is only really used when we don't have any Searchers at all var ErrAllSearchersFailed = errors.New("fptp: all searchers failed") type compositeSearcher struct { searchers []Searcher timeout time.Duration } // NewCompositeSearcher returns a Searcher that will try all of the provided Searchers func NewCompositeSearcher(searchers []Searcher, timeout time.Duration) Searcher { return &compositeSearcher{ searchers: searchers, timeout: timeout, } } // searchResponse is a nicer way of packaging the result of Searcher.Search for // use with channels. type searchResponse struct { closer io.Closer err error } func (c *compositeSearcher) Search(req *SearchRequest) (io.Closer, error) { ctx, cancel := context.WithTimeout(context.Background(), c.timeout) defer cancel() // We use a WaitGroup to keep track of when we've tried all of the Searcher instances var wg sync.WaitGroup // Synchronise on the channel respc := make(chan searchResponse) // Make all of the requests for _, s := range c.searchers { wg.Add(1) go func(req *SearchRequest, s Searcher) { defer wg.Done() c.search(ctx, s, req, respc) }(req, s) } // Housekeeping to track when we've tried all of the Searchers done := make(chan struct{}) go func() { defer close(done) wg.Wait() }() return c.waitForSearchCompletion(ctx, cancel, respc, done) } func (c *compositeSearcher) search(ctx context.Context, s Searcher, req *SearchRequest, respc chan<- searchResponse) { res, err := s.Search(req) select { case <-ctx.Done(): // Another Searcher already returned a successful response. Ensure we don't leak things res.Close() case respc <- searchResponse{res, err}: } } func (c *compositeSearcher) waitForSearchCompletion(ctx context.Context, cancel context.CancelFunc, respc chan searchResponse, done chan struct{}) (io.Closer, error) { var lastErr error for { select { case <-ctx.Done(): // timed out return nil, ctx.Err() // context.DeadlineExceeded case r := <-respc: // We got a response from one of the Searchers if r.err == nil { // success! return r.closer, r.err } lastErr = r.err // failed, keep track of why case <-done: // we've tried every Searcher, and none of them returned a successful response if lastErr != nil { return nil, lastErr } return nil, ErrAllSearchersFailed } } } -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
