[dbnode] Use active index block which GCs expired series instead of explicit block rotations #3464
Conversation
| b.segments[i] = emptySegment | ||
| b.segments[i].negativeOffsets = negativeOffsets[:0] | ||
| } | ||
| b.segments = b.segments[:0] |
There was a problem hiding this comment.
Do we need to reset filter and filterCount in this method as well?
| @@ -50,6 +52,9 @@ var ( | |||
| errForegroundCompactorNoPlan = errors.New("index foreground compactor failed to generate a plan") | |||
| errForegroundCompactorBadPlanFirstTask = errors.New("index foreground compactor generated plan without mutable segment in first task") | |||
| errForegroundCompactorBadPlanSecondaryTask = errors.New("index foreground compactor generated plan with mutable segment a secondary task") | |||
|
|
|||
| numBackgroundCompactorsStandard = 1 | |||
| for _, taskSegment := range task.Segments { | ||
| if taskSegment.Segment == seg.Segment() { | ||
| alreadyHasTask = true | ||
| break |
There was a problem hiding this comment.
Do you want a goto here or some other mechanism to escape the middle for-loop? Seems like you'll do some unnecessary iterations if not.
|
|
||
| func newEntryIndexState() entryIndexState { | ||
| return entryIndexState{ | ||
| states: make(map[xtime.UnixNano]entryIndexBlockState, 4), |
| MinSizeInclusive: 0, | ||
| MaxSizeExclusive: 1 << 18, | ||
| }, | ||
| Level{ |
There was a problem hiding this comment.
Do we not need these levels any longer?
There was a problem hiding this comment.
In practice I found while testing the new active block semantics that the more extra levels of compactions caused more churn and really huge segments needing multiple GC passes. Whereas with just a fixed ceiling it got a lot better in terms of how difficult the speed of re-processing segments got, since things wouldn't get re-combined any longer at higher levels of compaction.
| @@ -541,6 +554,10 @@ func (i *nsIndex) reportStats() error { | |||
| return err | |||
| } | |||
| } | |||
| // Active block should always be open. | |||
There was a problem hiding this comment.
Is it worth checking activeBlock.IsOpen here?
There was a problem hiding this comment.
We don't close it until it's explicitly closed in Close().
Maybe the comment should read "Active block is always open until index closed, at which point report stats is not run, therefore no check required".
|
Would also be nice we could get some tests around the new functionality |
robskillington
force-pushed
the
r/index-active-block
branch
from
5056d45
to
10cd227
Compare
…locks for evicting series from in-mem segments
Codecov Report
@@ Coverage Diff @@
## master #3464 +/- ##
=======================================
Coverage 56.7% 56.8%
=======================================
Files 552 552
Lines 61915 62319 +404
=======================================
+ Hits 35145 35432 +287
- Misses 23625 23724 +99
- Partials 3145 3163 +18
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
What this PR does / why we need it:
Makes the index keep an active index block which GCs expired series instead of requiring building a fresh index at block rotation time.
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: