Skip to content

Commit

Permalink
query/endpointset: fix some races (#5972)
Browse files Browse the repository at this point in the history
Fix races around reading `e.endpoints`. Example race:

```
==================
WARNING: DATA RACE
Write at 0x00c00006a668 by main goroutine:
  github.com/thanos-io/thanos/pkg/query.(*EndpointSet).Close()
      /home/giedrius/dev/thanos/pkg/query/endpointset.go:582 +0x164
  main.runQuery.func3()
      /home/giedrius/dev/thanos/cmd/thanos/query.go:534 +0x44
  github.com/oklog/run.(*Group).Run()
      /home/giedrius/go/pkg/mod/github.com/oklog/run@v1.1.0/group.go:47 +0x1ce
  main.main()
      /home/giedrius/dev/thanos/cmd/thanos/main.go:159 +0x2669

Previous read at 0x00c00006a668 by goroutine 276:
  github.com/thanos-io/thanos/pkg/query.(*EndpointSet).Update()
      /home/giedrius/dev/thanos/pkg/query/endpointset.go:396 +0xb9d
  main.runQuery.func2.1()
      /home/giedrius/dev/thanos/cmd/thanos/query.go:529 +0x4c
  github.com/thanos-io/thanos/pkg/runutil.Repeat()
      /home/giedrius/dev/thanos/pkg/runutil/runutil.go:74 +0xd8
  main.runQuery.func2()
      /home/giedrius/dev/thanos/cmd/thanos/query.go:528 +0x99
  github.com/oklog/run.(*Group).Run.func1()
      /home/giedrius/go/pkg/mod/github.com/oklog/run@v1.1.0/group.go:38 +0x41
  github.com/oklog/run.(*Group).Run.func2()
      /home/giedrius/go/pkg/mod/github.com/oklog/run@v1.1.0/group.go:39 +0x58

Goroutine 276 (running) created at:
  github.com/oklog/run.(*Group).Run()
      /home/giedrius/go/pkg/mod/github.com/oklog/run@v1.1.0/group.go:37 +0x349
  main.main()
      /home/giedrius/dev/thanos/cmd/thanos/main.go:159 +0x2669
==================
```

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
  • Loading branch information
GiedriusS committed Dec 15, 2022
1 parent 215465d commit fa95b8a
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/query/endpointset.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ func (e *EndpointSet) Update(ctx context.Context) {
wg.Wait()

timedOutRefs := e.getTimedOutRefs()
e.endpointsMtx.RLock()
for addr, er := range e.endpoints {
_, isNew := newRefs[addr]
_, isExisting := existingRefs[addr]
Expand All @@ -401,6 +402,7 @@ func (e *EndpointSet) Update(ctx context.Context) {
staleRefs[addr] = er
}
}
e.endpointsMtx.RUnlock()

e.endpointsMtx.Lock()
defer e.endpointsMtx.Unlock()
Expand Down Expand Up @@ -450,6 +452,8 @@ func (e *EndpointSet) updateEndpoint(ctx context.Context, spec *GRPCEndpointSpec
// successful health check is older than the unhealthyEndpointTimeout.
// Strict endpoints are never considered as timed out.
func (e *EndpointSet) getTimedOutRefs() map[string]*endpointRef {
e.endpointsMtx.RLock()
defer e.endpointsMtx.RUnlock()
result := make(map[string]*endpointRef)

endpoints := e.endpoints
Expand Down

0 comments on commit fa95b8a

Please sign in to comment.