Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

fix race in kafka config code #179

Merged
merged 1 commit into from
Apr 27, 2017
Merged

fix race in kafka config code #179

merged 1 commit into from
Apr 27, 2017

Conversation

kirg
Copy link
Contributor

@kirg kirg commented Apr 27, 2017

Race that occurs on concurrent calls to 'GetKafkaClusterConfig' .. which can happen in outputhost if multiple KFC streams get initialized concurrently.

WARNING: DATA RACE
Read at 0x00c4202290f0 by goroutine 2293:
  github.com/uber/cherami-server/common/configure.(*KafkaConfig).loadClusterConfigFileIfNecessary()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/common/configure/commonkafkaconfig.go:73 +0x40
  github.com/uber/cherami-server/common/configure.(*KafkaConfig).GetKafkaClusterConfig()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/common/configure/commonkafkaconfig.go:66 +0x6e
  github.com/uber/cherami-server/services/outputhost.getKafkaBrokersForCluster()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/services/outputhost/extcache.go:583 +0x9e
  github.com/uber/cherami-server/services/outputhost.(*extentCache).loadKafkaStream()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/services/outputhost/extcache.go:378 +0x2c7
  github.com/uber/cherami-server/services/outputhost.(*extentCache).load()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/services/outputhost/extcache.go:180 +0x1ea

Previous write at 0x00c4202290f0 by goroutine 148:
  github.com/uber/cherami-server/common/configure.(*KafkaConfig).loadClusterConfigFile()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/common/configure/commonkafkaconfig.go:97 +0x619
  github.com/uber/cherami-server/common/configure.(*KafkaConfig).loadClusterConfigFileIfNecessary()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/common/configure/commonkafkaconfig.go:77 +0x80
  github.com/uber/cherami-server/common/configure.(*KafkaConfig).GetKafkaClusterConfig()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/common/configure/commonkafkaconfig.go:66 +0x6e
  github.com/uber/cherami-server/services/outputhost.getKafkaBrokersForCluster()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/services/outputhost/extcache.go:583 +0x9e
  github.com/uber/cherami-server/services/outputhost.(*extentCache).loadKafkaStream()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/services/outputhost/extcache.go:378 +0x2c7
  github.com/uber/cherami-server/services/outputhost.(*extentCache).load()
      /Users/kiranrg/gocode/src/github.com/uber/cherami-server/services/outputhost/extcache.go:180 +0x1ea

@kirg kirg requested a review from hiboyang April 27, 2017 06:19
@kirg kirg self-assigned this Apr 27, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 68.181% when pulling 8533c35 on fix-commonkafkaconfig-race into 8984278 on master.

@kirg kirg requested review from GuillaumeBailey and datoug April 27, 2017 08:25
@kirg kirg merged commit b856a7a into master Apr 27, 2017
@kirg kirg deleted the fix-commonkafkaconfig-race branch April 27, 2017 17:56
kirg pushed a commit that referenced this pull request Apr 28, 2017
* ttt

* integration test for kafka-for-cherami

* integration.testDLQFromCassandra: Fix flakiness, speedup by 4x (#168)

* parallelize

* log kafka errors, if any

* disable flaky tests: TestStoreHostTimerQueueWrite{Then,With}Read (#177)

* retry on kafka publish error

* fix race when updating kafka config (#179)

* ttt

* integration test for kafka-for-cherami

* parallelize

* log kafka errors, if any

* retry on kafka publish error

* ~

* print msg on validation failure

* ~
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants