-
Notifications
You must be signed in to change notification settings - Fork 816
Create v8Schema with series index #430
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
Conversation
As discussed, we're going to go with a sha256. |
itemComponents := decodeRangeKey(items[i].rangeValue) | ||
if !bytes.Equal(itemComponents[3], metricNameRangeKeyV1) { | ||
if !bytes.Equal(itemComponents[3], metricNameRangeKeyV1) && | ||
!bytes.Equal(itemComponents[3], seriesRangeKeyV1) { | ||
return fmt.Errorf("Dupe write") |
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.
We should change this to error if its a duplicate within the batch, to match server behaviour.
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.
We are already doing so a few lines above. I didn't remove the Dupe write check across all writes as it's still useful for checking if chunks are no duplicated.
pkg/chunk/schema.go
Outdated
seriesBytes, err := json.Marshal(labels) | ||
if err != nil { | ||
return nil, err | ||
} |
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.
Can we just use Metric.String()?
https://github.com/prometheus/common/blob/master/model/metric.go#L54
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.
Can we decode the string back into model.Metric
?
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.
I couldn't find a convenient function, no.
I don't like the idea of putting JSON in here, its a PITA to deal with backwards compatibility etc. But I guess it'll have to do for now.
03cefcf
to
83bb69d
Compare
First step for #416.
This index will be used to fetch series from the index. It will replace the metric name index and we will make the switch after the next steps are complete.
Decided to encode the
model.Metric
as JSON because it already implements thejson.Unmarshaler
interface.Used sha256 for identifying the series as the variant of fnv64a which if used for fingerprints is not unique enough for indexing and using sha256 keeps it simple (we don't have to deal with collision logic).
Test plan