Add options to Cassandra to control indexing #680
Conversation
- Add `StoreIndexesOnly` and `StoreWithoutIndexing` options to the cassandra storage which controls whether indexes are written along with spans. - This allows us to deploy, scale, and prioritize writers independently for indexing and storage workloads. Signed-off-by: Prithvi Raj <p.r@uber.com>
vprithvi
requested review from
black-adder,
pavolloffay and
yurishkuro
as code owners
ghost
assigned 
| @@ -63,8 +63,13 @@ const ( | |||
| defaultNumBuckets = 10 | |||
|
|
|||
| durationBucketSize = time.Hour | |||
|
|
|||
| indexOnly = storageMode(1) | |||
There was a problem hiding this comment.
- use the
iotapattern - I would put indexAndStore as the first item (the default mode)
Signed-off-by: Prithvi Raj <p.r@uber.com>
| const ( | ||
| indexAndStore = storageMode(iota) | ||
| indexOnly = storageMode(iota) | ||
| storeOnly = storageMode(iota) |
There was a problem hiding this comment.
this is not the iota pattern, see https://dlintw.github.io/gobyexample/public/constants-and-iota.html
There was a problem hiding this comment.
e.g.
const (
Spring = Season(iota)
Summer
Autumn
Winner
)
| @@ -65,6 +65,13 @@ const ( | |||
| durationBucketSize = time.Hour | |||
| ) | |||
|
|
|||
| const ( | |||
There was a problem hiding this comment.
above section is already "const", don't need to start a new one
There was a problem hiding this comment.
I wanted to reset the index of iota, and it seems that it only happens in a new const block according to https://golang.org/ref/spec#Iota
Signed-off-by: Prithvi Raj <p.r@uber.com>
| } | ||
| if err := s.writeIndexes(span, ds); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
duplication of code is a sign of design problem. You could use bitmasks instead of iota-based constants
There was a problem hiding this comment.
That was what I was doing previously in 7509d91#diff-6eb76ee5c7e114abadac32d9b372742fR135
There was a problem hiding this comment.
I like the previous version better, why did you change it? We might have gotten the wires crossed. If your original version of the constants was bitmap-based, then it wasn't clear from the code - I would just declare two constants, saveSpanFlag 1 and indexSpanFlag 2, and add a comment that they can be combined in a bitmap (also name the struct variable somethingFlags)
There was a problem hiding this comment.
I thought that we wanted the default of indexing and storing to have an ordinal of 0; which didn't seem compatible with the previous approach.
I add the flags, and named them as suggested. I didn't add the comment because people would be setting these using options.
There was a problem hiding this comment.
if we're not using the sequential enum then it doesn't matter.
| @@ -38,15 +38,16 @@ type spanWriterTest struct { | |||
| writer *SpanWriter | |||
| } | |||
|
|
|||
| func withSpanWriter(writeCacheTTL time.Duration, fn func(w *spanWriterTest)) { | |||
| func withSpanWriter(writeCacheTTL time.Duration, fn func(w *spanWriterTest), options ...Option, | |||
| ) { | |||
Add
StoreIndexesOnlyandStoreWithoutIndexingoptions to the cassandrastorage which controls whether indexes are written along with spans.
This allows us to deploy, scale, and prioritize writers independently for
indexing and storage workloads.
Signed-off-by: Prithvi Raj p.r@uber.com