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

Fix race condition on continuous queries #2203

Merged
merged 4 commits into from
Apr 9, 2015
Merged

Fix race condition on continuous queries #2203

merged 4 commits into from
Apr 9, 2015

Conversation

corylanou
Copy link
Contributor

Need a Lock instead of an RLock as we are writing inside the loop.

@toddboom
Copy link
Contributor

toddboom commented Apr 8, 2015

👍 on green

@otoolep
Copy link
Contributor

otoolep commented Apr 8, 2015

Are you talking about this change?

c.intoRP = d.defaultRetentionPolicy

Is this what we need to lock?

@corylanou
Copy link
Contributor Author

@otoolep correct.

@otoolep
Copy link
Contributor

otoolep commented Apr 8, 2015

OK, thanks.

That code actually seems problematic, though it is there now, and not actually being changed. Is intoRP meant to persist, so it is set next the time loop is set? If so, it won't persist past a reboot.

@corylanou
Copy link
Contributor Author

No idea. I didn't look that closely at it, just that it was a race condition. @pauldix would know the answer.

@otoolep
Copy link
Contributor

otoolep commented Apr 8, 2015

OK, makes sense if the race detector flagged. Definitely have a question about it for @pauldix

corylanou added a commit that referenced this pull request Apr 9, 2015
Fix race condition on continuous queries
@corylanou corylanou merged commit af68206 into master Apr 9, 2015
@corylanou corylanou deleted the cq-race branch April 9, 2015 00:05
mark-rushakoff pushed a commit that referenced this pull request Jan 11, 2019
* chore(http): make test diffs more readable

* fixes

* add back telegraf output spacing

* whitespace hell

* jsonEqual2 -> jsonEqual
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.

3 participants