-
Notifications
You must be signed in to change notification settings - Fork 527
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
Compressed Blocks? Compressed Blocks! #504
Compressed Blocks? Compressed Blocks! #504
Conversation
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Getting some errors running locally:
It looks like the compactor was trying to compact an existing block (version 0/none) with a new block (version 1/snappy). The block looks ok at first glance, data file is 22MB compared to 50MB from this test workload. Going to test again with a blank slate. Edit:
|
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
I have tested snappy, gzip and a mix of snappy and gzip blocks locally without errors. I'm using the local backend on my laptop. I have not yet tried a mix of v0 and v1 blocks. I will continue digging, but if you find some steps to repro, please share. |
Signed-off-by: Joe Elliott <number101010@gmail.com>
✔️ It is working now with both snappy and lz4-256K. I'm thinking this commit fixed it, since the input error happened towards the end of the compaction cycle, maybe the last flushes were bad: |
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
var record *common.Record | ||
|
||
i := sort.Search(numRecords, func(i int) bool { | ||
buff := r.index[i*recordLength : (i+1)*recordLength] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could these branches reuse At(i)
?
What this PR does:
This is a rather large PR that completes the versioned blocks refactor as well as adds the ability to configure compression for backend blocks. Compression is configured under a new block config like so:
The default is currently
lz4-256k
but other accepted values arenone
,gzip
,lz4-64k
,lz4-1M
,lz4
,snappy
. There were obviously quite a few additional changes to get to this point which are detailed here.Changes
Consequently the cli params also changed:
trace.wal.bloom-filter-false-positive
=>trace.block.bloom-filter-false-positive
trace.wal.index-downsample
=>trace.block.index-downsample
tempo_query_reads
andtempo_query_bytes_read
have been removed from code and the operational dashboard. These metrics provide some value but due to other conflicts were let go. They may be possible to restore in part down the road.versionedEncoding
interface through which the various blocks interact with the data in the backend. This shim is messy at the moment and could use some cleanup but I think is the right direction to support future block versions.encoding
) that contains the current block encoding.Benchmarks
Benchmarks are available in
./tempodb/encoding
. The following output was generated using a 2.1GB block:This PR still requires some additional testing and so is marked as a Draft, but is otherwise ready for review.Which issue(s) this PR fixes:
Fixes #54
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]