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

Data race in consumer #36

Closed
tymeshifter opened this issue May 10, 2021 · 5 comments
Closed

Data race in consumer #36

tymeshifter opened this issue May 10, 2021 · 5 comments

Comments

@tymeshifter
Copy link

tymeshifter commented May 10, 2021

Version: 0.6.14

==================
WARNING: DATA RACE
Write at 0x00c0007e50b0 by goroutine 148:
  runtime.mapassign_fast64()
      runtime/map_fast64.go:92 +0x0
  github.com/twmb/franz-go/pkg/kgo.(*usedCursors).use()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:134 +0x186
  github.com/twmb/franz-go/pkg/kgo.(*consumerSession).handleListOrEpochResults.func2()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:1135 +0xef
  github.com/twmb/franz-go/pkg/kgo.(*consumerSession).handleListOrEpochResults()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:1144 +0x429
  github.com/twmb/franz-go/pkg/kgo.(*consumerSession).listOrEpoch()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:1070 +0xa44

Previous write at 0x00c0007e50b0 by goroutine 70:
  runtime.mapassign_fast64()
      runtime/map_fast64.go:92 +0x0
  github.com/twmb/franz-go/pkg/kgo.(*usedCursors).use()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:134 +0x186
  github.com/twmb/franz-go/pkg/kgo.(*consume  github.com/twmb/franz-go/pkg/kgo.(*consumerSession).handleListOrEpochResults()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:1144 +0x429
  github.com/twmb/franz-go/pkg/kgo.(*consumerSession).listOrEpoch()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:1070 +0xa44

Goroutine 148 (running) created at:
  github.com/twmb/franz-go/pkg/kgo.listOrEpochLoads.loadWithSession()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:748 +0xca
  github.com/twmb/franz-go/pkg/kgo.(*consumerSession).handleListOrEpochResults.func1.1()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:1124 +0x259

Goroutine 70 (finished) created at:
  github.com/twmb/franz-go/pkg/kgo.listOrEpochLoads.loadWithSession()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:748 +0xca
  github.com/twmb/franz-go/pkg/kgo.(*consumerSession).handleListOrEpochResults.func1.1()
      github.com/twmb/[email protected]/pkg/kgo/consumer.go:1124 +0x259
==================
@twmb
Copy link
Owner

twmb commented May 10, 2021

Interesting, thank you. I see that this can be triggered if two handleListOrEpochResults are running. I've checked and the only other place that the field is modified is when the consumer session is stopped.

I forgot about the fact that the function could run multiple times concurrently. :(

There's a few ways to go about guard running that segment of code. I'll have a fix out imminently once I figure out which approach I like best.

@twmb twmb closed this as completed in 1aaa1ef May 10, 2021
@twmb
Copy link
Owner

twmb commented May 10, 2021

Pushed a fix, I'll tag a release likely later today.

Just out of curiosity, what were the conditions that triggered this? I mention one scenario in my commit message, but the more likely scenario I think is that the first few ListOffsets requests failed, which independently triggered backoffs, which then trigger this scenario (I'm going to push a make a small change here as well, unrelated and not necessary for this bugfix)

@twmb
Copy link
Owner

twmb commented May 11, 2021

Tag likely tomorrow actually, I'm close to finishing the documentation I've been trying to finish for a few weeks now.

@tymeshifter
Copy link
Author

Just out of curiosity, what were the conditions that triggered this?

Just stress tested an application that uses the library. Use case is normal consumption loop with PollFetches.

@twmb
Copy link
Owner

twmb commented May 13, 2021

The tag took a but, but 0.7.0 is now released that contains this bugfix.

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

No branches or pull requests

2 participants