no overlapping on compaction when an existing block is not within default boundaries. #461
Conversation
|
@gouthamve , @brian-brazil current master is broken by d50b9a5 The logic for loading the WAL depends on knowing if there are already blocks so The test in this PR ensures this behaviour and will pass after reverting d50b9a5 |
head.go
Outdated
| @@ -640,13 +640,20 @@ func (h *Head) Appender() Appender { | |||
| func (h *Head) appender() *headAppender { | |||
| return &headAppender{ | |||
| head: h, | |||
| minValidTime: h.MaxTime() - h.chunkRange/2, | |||
| minValidTime: max(h.MinTime(), h.MaxTime()-h.chunkRange/2), | |||
There was a problem hiding this comment.
I am not entirely sure about this one so let me know if you think it might brake anything else.
With this change when there is no wal and no block the appender is limited to the timestamp of the first appended sample so it brakes few test that rely on adding samples with timestamps lower than the first appended sample.
There was a problem hiding this comment.
adding samples with timestamps lower than the first appended sample.
That's a valid use case.
head.go
Outdated
| @@ -640,13 +640,20 @@ func (h *Head) Appender() Appender { | |||
| func (h *Head) appender() *headAppender { | |||
| return &headAppender{ | |||
| head: h, | |||
| minValidTime: h.MaxTime() - h.chunkRange/2, | |||
| minValidTime: max(h.MinTime(), h.MaxTime()-h.chunkRange/2), | |||
There was a problem hiding this comment.
adding samples with timestamps lower than the first appended sample.
That's a valid use case.
…ault boundaries. Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
This reverts commit c772180. Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
krasi-georgiev
force-pushed
the
custom-block-ranges
branch
from
3fda834
to
527ba6f
Compare
|
@gouthamve, @brian-brazil I have refactored the logic to what I think makes sense.
|
|
That shouldn't be GCed, as the GC code isn't called in that test. |
|
@brian-brazil as I mentioned I refactored the logic , I am asking if the behaviour is correct and whether series should remain in the head if they don't have any sample after loading the WAL. |
|
Ah, I see that you added a manual gc. That behaviour is correct then, and the test needs updating. |
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
|
all tests passing and also updated the changelog |
|
@gouthamve one final look would be good. |
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
Signed-off-by: Krasi Georgiev <kgeorgie@redhat.com>
closes prometheus/prometheus#4643
Signed-off-by: Krasi Georgiev kgeorgie@redhat.com