Columnar data structures and array-oriented APIs for decoding TSM data #10024
Conversation
ghost
added
the
stuartcarnie
force-pushed
the
sgc-batch
branch
2 times, most recently
from
d65143c to
eb057ad
Compare
stuartcarnie
force-pushed
the
sgc-batch
branch
2 times, most recently
from
0f2d534 to
f03d890
Compare
| @@ -0,0 +1,994 @@ | |||
| // Package simple8b implements the 64bit integer encoding algoritm as published | |||
There was a problem hiding this comment.
@stuartcarnie let's maybe just add a note that this is mainly github.com/jwilder/encoding and document the additions in the source code?
tsdb/engine/tsm1/bool_test.go
Outdated
| b.Fatalf("expected to read %d booleans, but read %d", size, n) | ||
| } | ||
| func BenchmarkBooleanDecoder_DecodeAll(b *testing.B) { | ||
| benchmarks := []struct { |
tsdb/engine/tsm1/bool_test.go
Outdated
| } | ||
| for _, bm := range benchmarks { | ||
| b.Run(fmt.Sprintf("%d", bm.n), func(b *testing.B) { | ||
| size := bm.n |
There was a problem hiding this comment.
This really doesn't matter, but an alternative is to move the setup outside of b.Run, then you don't need to reset the timer.
There was a problem hiding this comment.
I like it; going to update all the benchmarks I touched with this
| @@ -0,0 +1,1007 @@ | |||
| // Generated by tmpl | |||
| // https://github.com/benbjohnson/tmpl | |||
There was a problem hiding this comment.
Was this generated by https://github.com/benbjohnson/tmpl or does this need to be updated?
There was a problem hiding this comment.
Indeed this was generated by https://github.com/benbjohnson/tmpl
tsdb/arrayvalues.gen.go
Outdated
| } | ||
|
|
||
| // Include returns the subset values between min and max inclusive. The values must | ||
| // be deduplicated and sorted before calling Exclude or the results are undefined. |
| // Normally, both a and b should not contain duplicates. Due to a bug in older versions, it's | ||
| // possible stored blocks might contain duplicate values. Remove them if they exists before | ||
| // merging. | ||
| // a = a.Deduplicate() |
There was a problem hiding this comment.
Are we sure this is safe to do? Was the bug in post-1.0 releases?
There was a problem hiding this comment.
I'd like to talk about this a bit more – I'm thinking 2.0, this may be important
There was a problem hiding this comment.
@edd, here is the commit for 1.0.0beta that adds the duplicate point check. Presumably the bug was discovered earlier, which may be this one, fixed in 0.11.0
|
|
||
| if b.MaxTime() < a.MinTime() { | ||
| var tmp FloatArray | ||
| tmp.Timestamps = append(b.Timestamps, a.Timestamps...) |
There was a problem hiding this comment.
It didn't occur to me that this needed to be a temp slice. I would have thought a.Timestamps = append(b.Timestamps, a.Timestamps...) would be OK?
There was a problem hiding this comment.
I am maintaining that b is not mutated. Perhaps it is ok to do this, as len(b.Timestamps) doesn't change. I'll check assumptions elsewhere to make sure this is ok.
* includes additional APIs for batch decoding of byte slices to improve performance * fix for `unpack120` that was decoding 240 values rather than 120
These benchmarks will be implemented in batched decoders to compare performance.
* APIs decode an entire byte slice of encoded data into the provided `dst` slice * APIs are stateless and in almost all cases avoid any allocations * Intended to be used future batch-oriented TSM block decode APIs * duplicated tests from original iterator-based APIs
* separate slices for time and values * structured to be Arrow ready * batch decoders fill time and value slices independently that vastly improves performance (benchmarks linked in PR)
* These APIs will be used by `TSMReader` and `KeyCursor` types via new APIs, using similar naming convention (Array)
|
During a run of megacheck the following issues were discovered: |
Partial implementation of #9981 through to and including Add batch block decoder APIs.
NOTE: The goal of this PR is to teach the low-level TSM decoders how to operate on entire blocks of encoded TSM data. These new APIs will be used by the storage read in a follow-up PR.