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 concurrent map access panic #6647

Merged
merged 1 commit into from
May 18, 2016
Merged

Fix concurrent map access panic #6647

merged 1 commit into from
May 18, 2016

Conversation

e-dard
Copy link
Contributor

@e-dard e-dard commented May 17, 2016

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

Fixes #6235

@@ -18,6 +18,8 @@
- [#6618](https://github.com/influxdata/influxdb/pull/6618): Optimize shard loading
- [#6629](https://github.com/influxdata/influxdb/issues/6629): query-log-enabled in config not ignored anymore.
- [#6607](https://github.com/influxdata/influxdb/issues/6607): SHOW TAG VALUES accepts != and !~ in WHERE clause.
- [#6235](https://github.com/influxdata/influxdb/issues/6235): Fix meaurement field panic in tsm1 engine.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo.

@jsternberg
Copy link
Contributor

Minor nitpick but 👍 otherwise.

@e-dard
Copy link
Contributor Author

e-dard commented May 17, 2016

Fixed. Will merge on circle green and a another +1

e.measurementFields[measurement] = mf
}

mf := e.MeasurementFields(measurement)
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this deadlock when called from LoadMetaDataIndex?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eugh, it could if the measurement field doesn't exist.

Since addToIndexFromKey is sometimes called while under an RLock and sometimes isn't, this is going to take a little re-working. I'll push something up tomorrow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jwilder I just looked at this and realised it's the Cache's RLock that's taken in LoadMetaDataIndex, not the Engine's.

So we're good to go?

@e-dard e-dard merged commit bac08b7 into master May 18, 2016
@e-dard e-dard mentioned this pull request May 20, 2016
@e-dard e-dard deleted the er-fix-map branch May 20, 2016 13:05
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