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

TSM: TSMReader.Close blocks until reads complete #9792

Merged
merged 2 commits into from
Apr 30, 2018
Merged

Conversation

jacobmarble
Copy link
Member

Fixes #9786

TSMReader.Close returns an error when queries are referencing the TSM file, but FileStore.Close ignores the error.

In this change, FileStore.Close checks the error, and TSMReader.Close blocks, rather than returning the error. This makes it clear to the caller of FileStore.Close (drop shard X for example) that the file is safely closed, rather than requiring the caller to retry up to thousands of times when the query load is constantly high.

After this change, TSM files are always closed when drop shard X returns, and writing to a shard that is currently being deleted returns a partial error.

@jacobmarble jacobmarble self-assigned this Apr 30, 2018
@ghost ghost added the review label Apr 30, 2018
@jacobmarble jacobmarble force-pushed the jm-rm-tsm-files branch 2 times, most recently from 255f2aa to 98cfb30 Compare April 30, 2018 17:26
@influxdata influxdata deleted a comment from hercules-influx Apr 30, 2018
@influxdata influxdata deleted a comment from hercules-influx Apr 30, 2018
@jacobmarble jacobmarble requested a review from e-dard April 30, 2018 17:40
@jacobmarble
Copy link
Member Author

Tests are timing out. That's probably important in this change, so I'm digging in now.

@jacobmarble jacobmarble modified the milestone: 1.6.0 Apr 30, 2018
@jacobmarble
Copy link
Member Author

This should fix tests. Just needed to close iterators and cursors.

Copy link
Contributor

@e-dard e-dard left a comment

Choose a reason for hiding this comment

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

LGTM 👍 great find @jacobmarble!

@jacobmarble jacobmarble merged commit 63b9c98 into master Apr 30, 2018
@ghost ghost removed the review label Apr 30, 2018
@jacobmarble jacobmarble deleted the jm-rm-tsm-files branch April 30, 2018 20:46
@jacobmarble
Copy link
Member Author

@e-dard Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TSM files not closed when shard deleted
3 participants