Skip to content
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

Reducing the memory footprint of active series custom trackers #2568

Merged
merged 16 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions pkg/ingester/activeseries/active_series.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type seriesStripe struct {
type seriesEntry struct {
lbs labels.Labels
nanos *atomic.Int64 // Unix timestamp in nanoseconds. Needs to be a pointer because we don't store pointers to entries in the stripe.
matches []bool // Which matchers of Matchers does this series match
matches []int // Index of the matcher matching
}

func NewActiveSeries(asm *Matchers, timeout time.Duration) *ActiveSeries {
Expand Down Expand Up @@ -193,10 +193,8 @@ func (s *seriesStripe) findOrCreateEntryForSeries(fingerprint uint64, series lab
matches := s.matchers.Matches(series)

s.active++
for i, ok := range matches {
if ok {
s.activeMatching[i]++
}
for _, matcherIdx := range matches {
s.activeMatching[matcherIdx]++
}

e := seriesEntry{
Expand Down Expand Up @@ -260,10 +258,8 @@ func (s *seriesStripe) purge(keepUntil time.Time) {
}

s.active++
for i, ok := range entries[0].matches {
if ok {
s.activeMatching[i]++
}
for _, matcherIdx := range entries[0].matches {
s.activeMatching[matcherIdx]++
}
if ts < oldest {
oldest = ts
Expand Down Expand Up @@ -292,10 +288,8 @@ func (s *seriesStripe) purge(keepUntil time.Time) {
} else {
s.active += cnt
for i := range entries {
for i, ok := range entries[i].matches {
if ok {
s.activeMatching[i]++
}
for _, matcherIdx := range entries[i].matches {
s.activeMatching[matcherIdx]++
}
}

Expand Down
9 changes: 6 additions & 3 deletions pkg/ingester/activeseries/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,16 @@ func (m *Matchers) Config() CustomTrackersConfig {
return m.cfg
}

func (m *Matchers) Matches(series labels.Labels) []bool {
// Matches returns a []int containing only matcher indexes which are matching
func (m *Matchers) Matches(series labels.Labels) []int {
if len(m.matchers) == 0 {
return nil
}
matches := make([]bool, len(m.matchers))
matches := []int{}
gubjanos marked this conversation as resolved.
Show resolved Hide resolved
for i, sm := range m.matchers {
matches[i] = sm.Matches(series)
if sm.Matches(series) {
matches = append(matches, i)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we see benchmarks of this? :)

This will have to allocate a new slice every 2^n matchers, which is probably more expensive than just allocating a slice of bools since it will push unnecessary pressure on the GC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're worried about memory consumption here, then IMO we should keep the previous approach and change []bool to a bitarray backed by []uint64

}
return matches
}
Expand Down
47 changes: 16 additions & 31 deletions pkg/ingester/activeseries/matchers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,60 +22,45 @@ func TestMatcher_MatchesSeries(t *testing.T) {

for _, tc := range []struct {
series labels.Labels
expected []bool
expected []int
}{
{
series: labels.Labels{{Name: "foo", Value: "true"}, {Name: "baz", Value: "unrelated"}},
expected: []bool{
false, // bar_starts_with_1
false, // does_not_have_foo_label
false, // has_foo_and_bar_starts_with_1
true, // has_foo_label
expected: []int{
3, // has_foo_label
},
},
{
series: labels.Labels{{Name: "foo", Value: "true"}, {Name: "bar", Value: "100"}, {Name: "baz", Value: "unrelated"}},
expected: []bool{
true, // bar_starts_with_1
false, // does_not_have_foo_label
true, // has_foo_and_bar_starts_with_1
true, // has_foo_label
expected: []int{
0, // bar_starts_with_1
2, // has_foo_and_bar_starts_with_1
3, // has_foo_label
},
},
{
series: labels.Labels{{Name: "foo", Value: "true"}, {Name: "bar", Value: "200"}, {Name: "baz", Value: "unrelated"}},
expected: []bool{
false, // bar_starts_with_1
false, // does_not_have_foo_label
false, // has_foo_and_bar_starts_with_1
true, // has_foo_label
expected: []int{
3, // has_foo_label
},
},
{
series: labels.Labels{{Name: "bar", Value: "200"}, {Name: "baz", Value: "unrelated"}},
expected: []bool{
false, // bar_starts_with_1
true, // does_not_have_foo_label
false, // has_foo_and_bar_starts_with_1
false, // has_foo_label
expected: []int{
1, // does_not_have_foo_label
},
},
{
series: labels.Labels{{Name: "bar", Value: "100"}, {Name: "baz", Value: "unrelated"}},
expected: []bool{
true, // bar_starts_with_1
true, // does_not_have_foo_label
false, // has_foo_and_bar_starts_with_1
false, // has_foo_label
expected: []int{
0, // bar_starts_with_1
1, // does_not_have_foo_label
},
},
{
series: labels.Labels{{Name: "baz", Value: "unrelated"}},
expected: []bool{
false, // bar_starts_with_1
true, // does_not_have_foo_label
false, // has_foo_and_bar_starts_with_1
false, // has_foo_label
expected: []int{
1, // does_not_have_foo_label
},
},
} {
Expand Down