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.

Sign up for GitHub

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

Jump to bottom

Fix TestSessionFetchIDs flaky test #3132

Merged
merged 1 commit into from
Jan 28, 2021
Merged

Fix TestSessionFetchIDs flaky test #3132

merged 1 commit into from
Jan 28, 2021

Conversation

ryanhall07
Copy link
Collaborator

This blocks reading the testFetch values until they have been written to
by the go routine.

Previous race

go test ./src/dbnode/client -run TestProtoSessionFetchIDs -race -count=100

Read at 0x00c00a14a630 by goroutine 457:
  github.com/jhump/protoreflect/codec.(*Buffer).DecodeVarint()
      /Users/rhall/go/pkg/mod/github.com/jhump/protoreflect@v1.6.1/codec/decode.go:76 +0xa6
  github.com/jhump/protoreflect/codec.(*Buffer).DecodeTagAndWireType()
      /Users/rhall/go/pkg/mod/github.com/jhump/protoreflect@v1.6.1/codec/decode.go:172 +0x3c
  github.com/jhump/protoreflect/codec.(*Buffer).DecodeFieldValue()
      /Users/rhall/go/pkg/mod/github.com/jhump/protoreflect@v1.6.1/codec/decode_fields.go:62 +0x90
  github.com/jhump/protoreflect/dynamic.(*Message).unmarshal()
      /Users/rhall/go/pkg/mod/github.com/jhump/protoreflect@v1.6.1/dynamic/binary.go:154 +0xfe
  github.com/jhump/protoreflect/dynamic.(*Message).UnmarshalMerge()
      /Users/rhall/go/pkg/mod/github.com/jhump/protoreflect@v1.6.1/dynamic/binary.go:149 +0xa3
  github.com/jhump/protoreflect/dynamic.(*Message).Unmarshal()
      /Users/rhall/go/pkg/mod/github.com/jhump/protoreflect@v1.6.1/dynamic/binary.go:138 +0x68
  github.com/m3db/m3/src/dbnode/testdata/prototest.RequireEqual()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/testdata/prototest/fixture.go:159 +0x13e
  github.com/m3db/m3/src/dbnode/client.glob..func3()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/client/session_proto_test.go:46 +0x97
  github.com/m3db/m3/src/dbnode/client.assertFetchResults()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/client/session_fetch_test.go:610 +0x866
  github.com/m3db/m3/src/dbnode/client.testSessionFetchIDs()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/client/session_fetch_test.go:184 +0x1528
  github.com/m3db/m3/src/dbnode/client.TestProtoSessionFetchIDs()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/client/session_proto_test.go:79 +0x4d4
  testing.tRunner()
      /usr/local/Cellar/go@1.13/1.13.15/libexec/src/testing/testing.go:909 +0x199

Previous write at 0x00c00a14a630 by goroutine 525:
  runtime.slicecopy()
      /usr/local/Cellar/go@1.13/1.13.15/libexec/src/runtime/slice.go:197 +0x0
  github.com/m3db/m3/src/dbnode/encoding/proto.(*customUnmarshaller).unmarshal()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/encoding/proto/custom_unmarshal.go:139 +0xc49
  github.com/m3db/m3/src/dbnode/encoding/proto.(*customUnmarshaller).resetAndUnmarshal()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/encoding/proto/custom_unmarshal.go:385 +0xf6
  github.com/m3db/m3/src/dbnode/encoding/proto.(*Encoder).Encode()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/encoding/proto/encoder.go:139 +0x17a
  github.com/m3db/m3/src/dbnode/client.fulfillFetchBatchOps()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/client/session_fetch_test.go:549 +0x7e9
  github.com/m3db/m3/src/dbnode/client.testSessionFetchIDs.func1()
      /Users/rhall/go/src/github.com/m3db/m3/src/dbnode/client/session_fetch_test.go:176 +0x12f

Now:

go test ./src/dbnode/client -run TestProtoSessionFetchIDs -race -count=1000                                                                                 
ok  	github.com/m3db/m3/src/dbnode/client	85.289s

@ryanhall07
Fix TestSessionFetchIDs flaky test
56aed3c 
This blocks reading the testFetch values until they have been written to
by the go routine.
linasm
linasm approved these changes Jan 28, 2021
View reviewed changes
Copy link
Collaborator

@linasm linasm left a comment

Choose a reason for hiding this comment

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

LGTM

@ryanhall07 ryanhall07 merged commit b973d23 into master Jan 28, 2021
@ryanhall07 ryanhall07 deleted the rhall-flaky-test branch January 28, 2021 17:47
soundvibe added a commit that referenced this pull request Jan 29, 2021
@soundvibe
Merge branch 'master' into linasn/bootstrap-profiler
aef0d2c 
* master:
  [dbnode] Add aggregate term limit regression test (#3135)
  [DOCS] Adding Prometheus steps to quickstart (#3043)
  [dbnode] Revert AggregateQuery changes (#3133)
  Fix TestSessionFetchIDs flaky test (#3132)
  [dbnode] Alter multi-segments builder to order by size before processing (#3128)
  [dbnode] Emit aggregate usage metrics (#3123)
  [dbnode] Add Shard.OpenStreamingReader method (#3119)
  [dtests] Docker tests integration with docker-compose (#3031)
  [dbnode] Comments / remove unused var (#3124)
  [query] Handle context.Canceled and map to 499 http status (#3069)
  [dbnode] Use StreamingReadMetadata for bootstrapping (#2938)
  [dbnode] Use DefaultTestOptions in test code (#3113)

# Conflicts:
#	src/dbnode/storage/bootstrap/bootstrapper/fs/source.go
soundvibe added a commit that referenced this pull request Feb 1, 2021
@soundvibe
Merge branch 'master' into linasn/bootstrap-out-of-retention-blocks-fix
8e87325 
* master:
  [dtest] endpoint to fetch tagged (#3138)
  Refactor FetchTagged to return an Iterator of results (#3141)
  [dbnode] Add aggregate term limit regression test (#3135)
  [DOCS] Adding Prometheus steps to quickstart (#3043)
  [dbnode] Revert AggregateQuery changes (#3133)
  Fix TestSessionFetchIDs flaky test (#3132)
  [dbnode] Alter multi-segments builder to order by size before processing (#3128)
  [dbnode] Emit aggregate usage metrics (#3123)
  [dbnode] Add Shard.OpenStreamingReader method (#3119)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linasm linasm linasm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryanhall07 @linasm