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

receive: Fixed small options race; Removed unused StartTime feature. #2816

Merged
merged 1 commit into from
Jun 29, 2020

Conversation

bwplotka
Copy link
Member

@bwplotka bwplotka commented Jun 29, 2020

NOTE: Those are minor fixes, this does not fix any issue we see with CPU usage or cut block.

  • startTimeMargin and StartTime is used only by Prometheus remote read, Thanos does not use it.
  • Fixed following race:
=== RUN   TestMultiTSDB/run_on_existing_storage
==================
WARNING: DATA RACE
Read at 0x00c00073ae80 by goroutine 69:
  github.com/prometheus/prometheus/tsdb.validateOpts()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:510 +0x55
  github.com/prometheus/prometheus/tsdb.Open()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:502 +0x61
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:268 +0x56b
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:302 +0x4ef
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:142 +0x66
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57 +0x85

Previous write at 0x00c00073ae80 by goroutine 57:
  github.com/prometheus/prometheus/tsdb.validateOpts()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:511 +0x1f2
  github.com/prometheus/prometheus/tsdb.Open()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:502 +0x61
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:268 +0x56b
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:302 +0x4ef
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:142 +0x66
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57 +0x85

Goroutine 69 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:54 +0x73
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:141 +0x2af
  github.com/thanos-io/thanos/pkg/receive.TestMultiTSDB.func3()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb_test.go:118 +0x6d3
  testing.tRunner()
      /home/bwplotka/.gvm/gos/go1.14.2/src/testing/testing.go:991 +0x1eb

Goroutine 57 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:54 +0x73
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:141 +0x2af
  github.com/thanos-io/thanos/pkg/receive.TestMultiTSDB.func3()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb_test.go:118 +0x6d3
  testing.tRunner()
      /home/bwplotka/.gvm/gos/go1.14.2/src/testing/testing.go:991 +0x1eb
==================

Signed-off-by: Bartlomiej Plotka [email protected]

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Verification

@bwplotka bwplotka requested review from brancz and squat and removed request for brancz June 29, 2020 15:39
startTimeMargin and StartTime is used only by Prometheus remote read, Thanos does not use it.
Fixed following race:

```
=== RUN   TestMultiTSDB/run_on_existing_storage
==================
WARNING: DATA RACE
Read at 0x00c00073ae80 by goroutine 69:
  github.com/prometheus/prometheus/tsdb.validateOpts()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:510 +0x55
  github.com/prometheus/prometheus/tsdb.Open()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:502 +0x61
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:268 +0x56b
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:302 +0x4ef
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:142 +0x66
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57 +0x85

Previous write at 0x00c00073ae80 by goroutine 57:
  github.com/prometheus/prometheus/tsdb.validateOpts()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:511 +0x1f2
  github.com/prometheus/prometheus/tsdb.Open()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:502 +0x61
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:268 +0x56b
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:302 +0x4ef
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open.func1()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:142 +0x66
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57 +0x85

Goroutine 69 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:54 +0x73
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:141 +0x2af
  github.com/thanos-io/thanos/pkg/receive.TestMultiTSDB.func3()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb_test.go:118 +0x6d3
  testing.tRunner()
      /home/bwplotka/.gvm/gos/go1.14.2/src/testing/testing.go:991 +0x1eb

Goroutine 57 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /home/bwplotka/Repos/thanosgopath/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:54 +0x73
  github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).Open()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb.go:141 +0x2af
  github.com/thanos-io/thanos/pkg/receive.TestMultiTSDB.func3()
      /home/bwplotka/Repos/thanos/pkg/receive/multitsdb_test.go:118 +0x6d3
  testing.tRunner()
      /home/bwplotka/.gvm/gos/go1.14.2/src/testing/testing.go:991 +0x1eb
==================
```

Signed-off-by: Bartlomiej Plotka <[email protected]>
@brancz brancz merged commit 789ef71 into master Jun 29, 2020
@brancz brancz deleted the receive-blocked branch June 29, 2020 16:29
paulfantom added a commit to paulfantom/thanos that referenced this pull request Jul 9, 2020
* upstream/release-0.14: (46 commits)
  Cut release v0.14.0-rc.1 (thanos-io#2853)
  Query: correctly marshal errors to JSON and ignore if nil (thanos-io#2848)
  ci: Manually download promu in crossbuild stage (thanos-io#2828)
  Cut release v0.14.0-rc.0 (thanos-io#2826)
  Soft cut changelog on master to indicate v0.14.0 being in progress (thanos-io#2824)
  Update ThanosReceiveNoUpload to select sum == 0 (thanos-io#2819)
  receive: Added more observability, fixed leaktest, to actually check leaks ): (thanos-io#2817)
  Query: always return a string in the `lastError` field (thanos-io#2809)
  Added missing CHANGELOG entry for PR 2613 (thanos-io#2820)
  receive: Fixed small options race; Removed unused StartTime feature. (thanos-io#2816)
  go.mod: Bump Prometheus to current latest (thanos-io#2814)
  Implement CLI Flags page in React UI (thanos-io#2796)
  Improve ThanosReceiveNoUpload to only alert on current instances
  store: Preallocate output buffer when encoding postings. (thanos-io#2812)
  compact: introduce flag --block-viewer.global.sync-block-interval (thanos-io#2752)
  docs: compact: add blurb about how retention policy works (thanos-io#2808)
  Reduced memory allocations in readIndexRange() (thanos-io#2807)
  ui: Add Stores page to React UI (thanos-io#2754)
  Added Kemal to Maintainer Role; Kemal is volounteering to be next release shephard (thanos-io#2804)
  proposal: Add scalable rule storage proposal (thanos-io#2661)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants