[db] Add an option to use db writes in index mode #1596
Conversation
| @@ -1281,6 +1280,8 @@ func withEncodingAndPoolingOptions( | |||
| aggregateQueryResultsPool := index.NewAggregateResultsPool( | |||
| poolOptions(policy.IndexResultsPool, scope.SubScope("index-aggregate-results-pool"))) | |||
|
|
|||
| // Set index options. | |||
| opts = opts.SetTruncateType(cfg.Transforms.TruncateBy) | |||
There was a problem hiding this comment.
nit: Maybe move this above the // Set index options since this transforms values? i.e. maybe just comment with // Set value transformations?
src/dbnode/storage/namespace_test.go
Outdated
| ns, closer := newTestNamespace(t) | ||
| if index != nil { | ||
| ns.reverseIndex = index | ||
| } | ||
| return ns, closer | ||
| } | ||
|
|
||
| func newTestNamespaceWithUseAsIndex( |
There was a problem hiding this comment.
Rename to newTestNamespaceWithTruncateType(..)?
|
|
||
| const ( | ||
| TypeNone TruncateType = iota | ||
| TypeBlock |
There was a problem hiding this comment.
Maybe instead of just TypeBlock we could do TypeTimestampBlock so it is clear that it's truncating just the timestamp?
src/dbnode/storage/series/buffer.go
Outdated
| @@ -60,6 +60,10 @@ const ( | |||
| // enabled to see if this is a sane number. | |||
| evictedTimesArraySize = 8 | |||
| writableBucketVer = 0 | |||
|
|
|||
| // NB: this is the value all points will be truncated to. | |||
| truncatedValue float64 = 0 | |||
There was a problem hiding this comment.
Maybe we don't set it to a specific value and just let whatever the client is writing be the value it sets?
There was a problem hiding this comment.
Fair; pulling this into configs as discussed
src/dbnode/storage/series/buffer.go
Outdated
|
|
||
| // NB: this is the value all points will be truncated to. | ||
| truncatedValue float64 = 0 | ||
| truncatedUnit = xtime.Second |
There was a problem hiding this comment.
We also probably don't need to override the unit no? Client should always be sending the same unit unless it has a bug or is writing datapoints at different timestamps (that would not wish to be snapped to block boundary).
There was a problem hiding this comment.
Yeah fair, this was mostly to kinda standardize all values going into the index but don't suppose it matters very much here
| @@ -1,4 +1,12 @@ | |||
| db: | |||
| transforms: | |||
There was a problem hiding this comment.
oops will revert this
|
Also looks like some big unit tests and docker integration tests are strangely failing? |
Codecov Report
@@ Coverage Diff @@
## master #1596 +/- ##
======================================
Coverage 70.2% 70.2%
======================================
Files 953 953
Lines 87398 87398
======================================
Hits 61377 61377
Misses 21895 21895
Partials 4126 4126
Continue to review full report at Codecov.
|
What this PR does / why we need it:
Adds an option to use a namespace in index mode; i.e. all writes to this namespace will get truncated to block size and have a set value, meaning we write at most one point per block per series, allowing this namespace to be used as a dedicated index more easily.
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: