Write chunks via queue, predicting the refs #10051
Merged
Conversation
This file contains hidden or 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
Our load tests have shown that there is a latency spike in the remote write handler whenever the head chunks need to be written, because chunkDiskMapper.WriteChunk() blocks until the chunks are written to disk. This adds a queue to the chunk disk mapper which makes the WriteChunk() method non-blocking unless the queue is full. Reads can still be served from the queue. Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
colega
reviewed
Jan 3, 2022
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
fa52843 to
3e9160c
Compare
colega
reviewed
Jan 4, 2022
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
3f1544c to
210f9fa
Compare
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
colega
reviewed
Jan 5, 2022
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
codesome
reviewed
Jan 5, 2022
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Thanks, I fixed it. |
codesome
reviewed
Jan 5, 2022
|
/prombench main |
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
cbefe75 to
544c2ca
Compare
|
/prombench cancel All looks good |
|
Benchmark cancel is in progress. |
codesome
reviewed
Jan 6, 2022
| @@ -297,42 +326,39 @@ func TestChunkDiskMapper_Truncate_PreservesFileSequence(t *testing.T) { | |||
| // though files 4 and 6 are empty. | |||
| file2Maxt := hrw.mmappedChunkFiles[2].maxt | |||
| require.NoError(t, hrw.Truncate(file2Maxt+1)) | |||
| // As 6 was empty, it should not create another file. | |||
There was a problem hiding this comment.
We need to test this. We should add a chunk and see that 3,4,5,6 is present and no 7. And then truncate and add a chunk to create the file 7 (we need this truncate to make sure again that it preserves sequence of files).
There was a problem hiding this comment.
is this what you had in mind? 04e94b9
There was a problem hiding this comment.
Not exactly, we should be truncating file2Maxt+1 like before. Because the bug was the it was deleting empty files from later files.
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com> Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
f3802ad to
07ce3f9
Compare
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
07ce3f9 to
4d3d594
Compare
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com>
|
I also replaced most occurrences of |
codesome
reviewed
Jan 10, 2022
69db63b to
6bd1f8e
Compare
suyashtava
pushed a commit
to suyashtava/prometheus
that referenced
this pull request
Jan 18, 2022
* Write chunks via queue, predicting the refs Our load tests have shown that there is a latency spike in the remote write handler whenever the head chunks need to be written, because chunkDiskMapper.WriteChunk() blocks until the chunks are written to disk. This adds a queue to the chunk disk mapper which makes the WriteChunk() method non-blocking unless the queue is full. Reads can still be served from the queue. Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * address PR feeddback Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * initialize metrics without .Add(0) Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * change isRunningMtx to normal lock Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * do not re-initialize chunkrefmap Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * update metric outside of lock scope Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * add benchmark for adding job to chunk write queue Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * remove unnecessary "success" var Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * gofumpt -extra Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * avoid WithLabelValues call in addJob Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * format comments Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * addressing PR feedback Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * rename cutExpectRef to cutAndExpectRef Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * use head.Init() instead of .initTime() Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * address PR feedback Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * PR feedback Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com> Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * update test according to PR feedback Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * replace callbackWg -> awaitCb Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * better test of truncation with empty files Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> * replace callbackWg -> awaitCb Signed-off-by: Mauro Stettler <mauro.stettler@gmail.com> Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com> Signed-off-by: suyashtava <suyashtava@gmail.com>
This was referenced Jun 16, 2022
2 tasks
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.
Our load tests have shown that there is a latency spike in the
remote write handler whenever the head chunks need to be written,
because
chunkDiskMapper.WriteChunk()blocks until the chunks are writtento disk.
This adds a queue to the chunk disk mapper which makes the
WriteChunk()method non-blocking unless the queue is full. Reads can still be served
from the queue.