Skip to content

Commit

Permalink
storage: improve index lookups
Browse files Browse the repository at this point in the history
tl;dr: This is not a fundamental solution to the indexing problem
(like tindex is) but it at least avoids utilizing the intersection
problem to the greatest possible amount.

In more detail:

Imagine the following query:

    nicely:aggregating:rule{job="foo",env="prod"}

While it uses a nicely aggregating recording rule (which might have a
very low cardinality), Prometheus still intersects the low number of
fingerprints for `{__name__="nicely:aggregating:rule"}` with the many
thousands of fingerprints matching `{job="foo"}` and with the millions
of fingerprints matching `{env="prod"}`. This totally innocuous query
is dead slow if the Prometheus server has a lot of time series with
the `{env="prod"}` label. Ironically, if you make the query more
complicated, it becomes blazingly fast:

    nicely:aggregating:rule{job=~"foo",env=~"prod"}

Why so? Because Prometheus only intersects with non-Equal matchers if
there are no Equal matchers. That's good in this case because it
retrieves the few fingerprints for
`{__name__="nicely:aggregating:rule"}` and then starts right ahead to
retrieve the metric for those FPs and checking individually if they
match the other matchers.

This change is generalizing the idea of when to stop intersecting FPs
and go into "retrieve metrics and check them individually against
remaining matchers" mode:

- First, sort all matchers by "expected cardinality". Matchers
  matching the empty string are always worst (and never used for
  intersections). Equal matchers are in general consider best, but by
  using some crude heuristics, we declare some better than others
  (instance labels or anything that looks like a recording rule).

- Then go through the matchers until we hit a threshold of remaining
  FPs in the intersection. This threshold is higher if we are already
  in the non-Equal matcher area as intersection is even more expensive
  here.

- Once the threshold has been reached (or we have run out of matchers
  that do not match the empty string), start with "retrieve metrics
  and check them individually against remaining matchers".

A beefy server at SoundCloud was spending 67% of its CPU time in index
lookups (fingerprintsForLabelPairs), serving mostly a dashboard that
is exclusively built with recording rules. With this change, it spends
only 35% in fingerprintsForLabelPairs. The CPU usage dropped from 26
cores to 18 cores. The median latency for query_range dropped from 14s
to 50ms(!). As expected, higher percentile latency didn't improve that
much because the new approach is _occasionally_ running into the worst
case while the old one was _systematically_ doing so. The 99th
percentile latency is now about as high as the median before (14s)
while it was almost twice as high before (26s).
  • Loading branch information
beorn7 committed Jul 20, 2016
1 parent 40f8da6 commit fc6737b
Show file tree
Hide file tree
Showing 7 changed files with 347 additions and 185 deletions.
19 changes: 6 additions & 13 deletions promql/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -953,11 +953,11 @@ func (p *parser) vectorSelector(name string) *VectorSelector {
}
}
// Set name label matching.
matchers = append(matchers, &metric.LabelMatcher{
Type: metric.Equal,
Name: model.MetricNameLabel,
Value: model.LabelValue(name),
})
m, err := metric.NewLabelMatcher(metric.Equal, model.MetricNameLabel, model.LabelValue(name))
if err != nil {
panic(err) // Must not happen with metric.Equal.
}
matchers = append(matchers, m)
}

if len(matchers) == 0 {
Expand All @@ -967,14 +967,7 @@ func (p *parser) vectorSelector(name string) *VectorSelector {
// implicit selection of all metrics (e.g. by a typo).
notEmpty := false
for _, lm := range matchers {
// Matching changes the inner state of the regex and causes reflect.DeepEqual
// to return false, which break tests.
// Thus, we create a new label matcher for this testing.
lm, err := metric.NewLabelMatcher(lm.Type, lm.Name, lm.Value)
if err != nil {
p.error(err)
}
if !lm.Match("") {
if !lm.MatchesEmptyString() {
notEmpty = true
break
}
Expand Down
Loading

0 comments on commit fc6737b

Please sign in to comment.