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

Bug fixes #7024

Merged
merged 6 commits into from
Jul 18, 2016
Merged

Bug fixes #7024

merged 6 commits into from
Jul 18, 2016

Conversation

jwilder
Copy link
Contributor

@jwilder jwilder commented Jul 18, 2016

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass

This PR fixes a few bugs related to writes and deletes:

  • Prevent a panic in the tsm engine when stats are collected before the engine is opened.
  • Abort compactions when a measurement or series is deleted. This fixes an issues where series or measurements re-appear after deleting.
  • Removed some logging around disabling compactions which was noisy
  • Limits the number of in-flight writes that hold large allocations in the WAL. When users with slower spinning disks, this allocations can cause OOMs.

jwilder added 5 commits July 17, 2016 23:53
If a delete is issued while a compaction is running, the a newly
deleted series could re-appear after the compaction completed. This
could occur the compaction had already written the blocks for series
that were just deleted.  When the compaction completes, the newly
written tombstone files would be deleted, essentially undeleting the
series.
A slower disk can can cause excessive allocations to occur when
writing to the WAL because the slower encoding and compression occurs
before taking the write lock.  The encoding/compression grabs a large
byte slice from a pool and ultimately waits until it can acquire the
write lock.

This adds a throttle to limit how many inflight WAL writes can be queued
up to prevent OOMing the processess with slower disks and heavy writes.
@jwilder jwilder added this to the 1.0.0 milestone Jul 18, 2016
@@ -179,8 +177,6 @@ func (e *Engine) SetCompactionsEnabled(enabled bool) {

// Wait for compaction goroutines to exit
e.wg.Wait()

e.logger.Printf("compactions disabled for: %v", e.path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall I add these back in when I get the trace logger comments done, and the PR merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trace logging for these would be good. Too verbose for regular logger though.

@e-dard
Copy link
Contributor

e-dard commented Jul 18, 2016

Just the nit about the package name, otherwise LGTM 👍

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

Successfully merging this pull request may close these issues.

2 participants