Speed up shutdown #7441
Merged
Speed up shutdown #7441
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mark-rushakoff
force-pushed
the
mr-speedup-shutdown
branch
from
d607e11 to
1501ee8
Compare
jwilder
suggested changes
Oct 10, 2016
| // Close all the shards in parallel. | ||
| // If we get any errors, return the first one. | ||
| var wg sync.WaitGroup | ||
| errs := make(chan error) | ||
| for _, sh := range s.shards { |
There was a problem hiding this comment.
Any reason this could not use the walkShards funcs?
There was a problem hiding this comment.
I wasn't aware of walkShard. I'll amend the commit to use that.
There was a problem hiding this comment.
Amended and pushed.
mark-rushakoff
force-pushed
the
mr-speedup-shutdown
branch
from
1501ee8 to
5ae8cf8
Compare
On my machine with about 20 shards, it would take 10+ seconds to shut down InfluxDB with SIGINT. After this change, it shuts down in nearly instantly. (*tsdb.Store).Close was shutting down each of its shards sequentially. Each shard's engine would signal to its compaction goroutines to quit, and because each compaction goroutine has a hardcoded 1-second sleep in between checks, waiting for the goroutines would often block for up to a second. This change closes all of the TSDB store's shards in parallel. This means it's possible that multiple close values could error at once, but we're still only returning the first error, consistent with previous behavior. That being said, the return value of (*tsdb.Store).Close is ignored in (*cmd/influxd/run.Server).Close anyway.
jwilder
approved these changes
Oct 10, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Required for all non-trivial PRs
On my machine with about 20 shards, it would take 10+ seconds to shut
down InfluxDB with SIGINT. After this change, it shuts down in nearly
instantly.
(*tsdb.Store).Close was shutting down each of its shards sequentially.
Each shard's engine would signal to its compaction goroutines to quit,
and because each compaction goroutine has a hardcoded 1-second sleep in
between checks, waiting for the goroutines would often block for up to a
second.
This change closes all of the TSDB store's shards in parallel. This
means it's possible that multiple close values could error at once, but
we're still only returning the first error, consistent with previous
behavior. That being said, the return value of (*tsdb.Store).Close is
ignored in (*cmd/influxd/run.Server).Close anyway.