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

Lock openTSDB and Graphite listener address #2426

Merged
merged 3 commits into from
Apr 25, 2015
Merged

Lock openTSDB and Graphite listener address #2426

merged 3 commits into from
Apr 25, 2015

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Apr 24, 2015

No description provided.

@@ -39,6 +39,7 @@ type Server struct {
wg sync.WaitGroup

addr net.Addr
mu sync.Mutex
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just went with a simple mutex, since this code is not accessed very often.

@otoolep otoolep changed the title Lock openTSDB listener address Lock openTSDB and Graphite listener address Apr 24, 2015
@otoolep
Copy link
Contributor Author

otoolep commented Apr 24, 2015

Fixes issues flagged by Go race detector.

@otoolep
Copy link
Contributor Author

otoolep commented Apr 24, 2015

@corylanou

@corylanou
Copy link
Contributor

It's strange that this ended up as a data race. We initialize the servers outside of go routines, and then only ever read (not write) to that value. I'm going to have to look through the code to figure out why this is getting reported as a data race. In theory, even if it's really a "data race", it should never be possible to ever get dirty data from those values.

I'm not overly concerned, as the little overhead this creates is only on startup anyway.

+1

otoolep added a commit that referenced this pull request Apr 25, 2015
Lock openTSDB and Graphite listener address
@otoolep otoolep merged commit b2de822 into master Apr 25, 2015
@otoolep otoolep deleted the opentsdb_race branch April 25, 2015 17:03
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.

None yet

2 participants