fix compactor unit test error #5248

yiyang5055 · 2023-04-04T04:04:52Z

What this PR does:
fix compactor unit test error for TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning case:
1.fix data race in ClientMock.MockGet(), the following code will cause a data race:

		// each time the mocked Get() is called we do create a new one, so
		// that getting the same mocked object twice works as expected.
		mockedGet := m.On("Get", mock.Anything, name)
		mockedGet.Run(func(args mock.Arguments) {
			mockedGet.Return(ioutil.NopCloser(bytes.NewReader([]byte(content))), err)
		})

2.multiple compactor instances run out of order. We cannot use the MockGetTimes method to ensure that visit-mark.json is accessed by the correct instance. We need to ensure that the mock operation is consistent with the actual operation logic, and the Get() can only succeed after Upload(), so i add MockGetRequireUpload() replace MockGetTimes()

error

==================
WARNING: DATA RACE
Read at 0x00c01a534228 by goroutine 349:
  bytes.(*Reader).Read()
      /usr/lib/golang/src/bytes/reader.go:41 +0x58
  io.(*nopCloser).Read()
      <autogenerated>:1 +0x76
  io.ReadAll()
      /usr/lib/golang/src/io/io.go:645 +0x102
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).loadMeta()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:278 +0xd91
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetchMetadata.func1()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:331 +0xc7
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /root/go/src/github.com/cortexproject/cortex/vendor/golang.org/x/sync/errgroup/errgroup.go:75 +0x86

Previous write at 0x00c01a534228 by goroutine 114:
  bytes.(*Reader).Read()
      /usr/lib/golang/src/bytes/reader.go:46 +0x11c
  io.(*nopCloser).Read()
      <autogenerated>:1 +0x76
  io.ReadAll()
      /usr/lib/golang/src/io/io.go:645 +0x102
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).loadMeta()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:278 +0xd91
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetchMetadata.func1()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:331 +0xc7
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /root/go/src/github.com/cortexproject/cortex/vendor/golang.org/x/sync/errgroup/errgroup.go:75 +0x86

Goroutine 349 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /root/go/src/github.com/cortexproject/cortex/vendor/golang.org/x/sync/errgroup/errgroup.go:72 +0x12e
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetchMetadata()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:329 +0x4b2
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetch.func2()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:447 +0x4c
  github.com/golang/groupcache/singleflight.(*Group).Do()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/golang/groupcache/singleflight/singleflight.go:56 +0x1e6
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetch()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:445 +0x1c5
  github.com/thanos-io/thanos/pkg/block.(*MetaFetcher).Fetch()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:505 +0xce
  github.com/thanos-io/thanos/pkg/compact.(*Syncer).SyncMetas()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/compact/compact.go:135 +0xec
  github.com/thanos-io/thanos/pkg/compact.(*BucketCompactor).Compact()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/compact/compact.go:1292 +0x425
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).compactUser()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:831 +0x151d
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).compactUserWithRetries()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:744 +0x1e8
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).compactUsers()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:688 +0x108f
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).running()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:590 +0x73
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).running-fm()
      <autogenerated>:1 +0x4d
  github.com/cortexproject/cortex/pkg/util/services.(*BasicService).main()
      /root/go/src/github.com/cortexproject/cortex/pkg/util/services/basic_service.go:190 +0x35b
  github.com/cortexproject/cortex/pkg/util/services.(*BasicService).StartAsync.func1.2()
      /root/go/src/github.com/cortexproject/cortex/pkg/util/services/basic_service.go:119 +0x39

Goroutine 114 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /root/go/src/github.com/cortexproject/cortex/vendor/golang.org/x/sync/errgroup/errgroup.go:72 +0x12e
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetchMetadata()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:329 +0x4b2
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetch.func2()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:447 +0x4c
  github.com/golang/groupcache/singleflight.(*Group).Do()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/golang/groupcache/singleflight/singleflight.go:56 +0x1e6
  github.com/thanos-io/thanos/pkg/block.(*BaseFetcher).fetch()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:445 +0x1c5
  github.com/thanos-io/thanos/pkg/block.(*MetaFetcher).Fetch()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/block/fetcher.go:505 +0xce
  github.com/thanos-io/thanos/pkg/compact.(*Syncer).SyncMetas()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/compact/compact.go:135 +0xec
  github.com/thanos-io/thanos/pkg/compact.(*BucketCompactor).Compact()
      /root/go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/compact/compact.go:1292 +0x425
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).compactUser()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:831 +0x151d
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).compactUserWithRetries()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:744 +0x1e8
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).compactUsers()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:688 +0x108f
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).running()
      /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor.go:590 +0x73
  github.com/cortexproject/cortex/pkg/compactor.(*Compactor).running-fm()
      <autogenerated>:1 +0x4d
  github.com/cortexproject/cortex/pkg/util/services.(*BasicService).main()
      /root/go/src/github.com/cortexproject/cortex/pkg/util/services/basic_service.go:190 +0x35b
  github.com/cortexproject/cortex/pkg/util/services.(*BasicService).StartAsync.func1.2()
      /root/go/src/github.com/cortexproject/cortex/pkg/util/services/basic_service.go:119 +0x39
==================
=== CONT  TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning
    compactor_test.go:1298:
          Error Trace:  /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor_test.go:1298
          Error:        Should be true
          Test:         TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning
    testing.go:1312: race detected during execution of test
--- FAIL: TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning (4.02s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
  panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1927329]

goroutine 3175 [running]:
testing.tRunner.func1.2({0x1f12b00, 0x65d4170})
  /usr/lib/golang/src/testing/testing.go:1389 +0x366
testing.tRunner.func1()
  /usr/lib/golang/src/testing/testing.go:1392 +0x5d2
panic({0x1f12b00, 0x65d4170})
  /usr/lib/golang/src/runtime/panic.go:844 +0x258
github.com/cortexproject/cortex/pkg/util/concurrency.(*SyncBuffer).String(0x0)
  /root/go/src/github.com/cortexproject/cortex/pkg/util/concurrency/buffer.go:22 +0x49
github.com/cortexproject/cortex/pkg/compactor.TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning(0xc01003a4e0)
  /root/go/src/github.com/cortexproject/cortex/pkg/compactor/compactor_test.go:1299 +0x156b
testing.tRunner(0xc01003a4e0, 0x21d8f50)
  /usr/lib/golang/src/testing/testing.go:1439 +0x214
created by testing.(*T).Run
  /usr/lib/golang/src/testing/testing.go:1486 +0x725
FAIL  github.com/cortexproject/cortex/pkg/compactor 15.787s
FAIL

Which issue(s) this PR fixes:
Fixes #

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

2.fix compactor unit test anomalous error Signed-off-by: yiyang5055 <[email protected]>

alanprot · 2023-04-05T00:13:47Z

Thanks! LGTM

2.fix compactor unit test anomalous error Signed-off-by: yiyang5055 <[email protected]> Signed-off-by: Ben Ye <[email protected]>

1.fix data race in ClientMock.MockGet()

b04d4f9

2.fix compactor unit test anomalous error Signed-off-by: yiyang5055 <[email protected]>

pull-request-size bot added the size/M label Apr 4, 2023

alanprot approved these changes Apr 5, 2023

View reviewed changes

friedrichg approved these changes Apr 10, 2023

View reviewed changes

friedrichg merged commit 668fd67 into cortexproject:master Apr 10, 2023

yeya24 pushed a commit to yeya24/cortex that referenced this pull request Apr 28, 2023

1.fix data race in ClientMock.MockGet() (cortexproject#5248)

1940883

2.fix compactor unit test anomalous error Signed-off-by: yiyang5055 <[email protected]> Signed-off-by: Ben Ye <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix compactor unit test error #5248

fix compactor unit test error #5248

Uh oh!

yiyang5055 commented Apr 4, 2023

Uh oh!

alanprot commented Apr 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix compactor unit test error #5248

fix compactor unit test error #5248

Uh oh!

Conversation

yiyang5055 commented Apr 4, 2023

Uh oh!

alanprot commented Apr 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants