Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

query/endpointset: fix some races #5972

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

GiedriusS
Copy link
Member

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/[email protected]/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/[email protected]/group.go:38 +0x41
  github.com/oklog/run.(*Group).Run.func2()
      /home/giedrius/go/pkg/mod/github.com/oklog/[email protected]/group.go:39 +0x58

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

Cannot reproduce the race after these changes.

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/[email protected]/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/[email protected]/group.go:38 +0x41
  github.com/oklog/run.(*Group).Run.func2()
      /home/giedrius/go/pkg/mod/github.com/oklog/[email protected]/group.go:39 +0x58

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

Signed-off-by: Giedrius Statkevičius <[email protected]>
@GiedriusS GiedriusS marked this pull request as ready for review December 15, 2022 08:38
Copy link
Contributor

@fpetkovski fpetkovski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@saswatamcode saswatamcode merged commit fa95b8a into thanos-io:main Dec 15, 2022
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request May 18, 2023
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/[email protected]/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/[email protected]/group.go:38 +0x41
  github.com/oklog/run.(*Group).Run.func2()
      /home/giedrius/go/pkg/mod/github.com/oklog/[email protected]/group.go:39 +0x58

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

Signed-off-by: Giedrius Statkevičius <[email protected]>

Signed-off-by: Giedrius Statkevičius <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants