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

Use slice pooling to populate the query stream response #6466

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

alanprot
Copy link
Member

@alanprot alanprot commented Dec 31, 2024

What this PR does:
Use slice pooling to populate the query stream response.

Under very high query load (lots of rules), the slice created when returning the query response is a big part of the alloc memory causing more frequent GC.

Screenshot 2024-12-30 at 5 40 51 PM

This PR is proposing to reuse those slices in order to safe GC CPU cycles.

Benchmark:

goos: darwin
goarch: amd64
pkg: github.com/cortexproject/cortex/pkg/ingester
cpu: VirtualApple @ 2.50GHz
                                                                │   /tmp/old   │              /tmp/new               │
                                                                │    sec/op    │    sec/op     vs base               │
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=10-10    20.74µ ± 11%   20.56µ ± 14%        ~ (p=1.000 n=6)
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=50-10    92.38µ ± 11%   84.27µ ± 17%        ~ (p=0.132 n=6)
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=100-10   175.2µ ± 16%   144.9µ ± 21%  -17.30% (p=0.015 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=10-10    37.53µ ±  4%   35.24µ ±  5%   -6.09% (p=0.009 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=50-10    173.6µ ±  6%   163.4µ ±  3%        ~ (p=0.065 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=100-10   338.0µ ±  8%   329.8µ ±  5%        ~ (p=0.065 n=6)
geomean                                                           95.09µ         88.39µ         -7.04%

                                                                │   /tmp/old   │              /tmp/new               │
                                                                │     B/op     │     B/op      vs base               │
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=10-10    27.96Ki ± 1%   17.01Ki ± 4%  -39.16% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=50-10    86.71Ki ± 3%   76.44Ki ± 4%  -11.84% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=100-10   158.8Ki ± 5%   148.1Ki ± 1%   -6.73% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=10-10    27.73Ki ± 1%   16.98Ki ± 5%  -38.75% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=50-10    88.28Ki ± 3%   77.86Ki ± 4%  -11.80% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=100-10   158.7Ki ± 4%   148.3Ki ± 1%   -6.59% (p=0.002 n=6)
geomean                                                           72.86Ki        57.92Ki       -20.50%

                                                                │  /tmp/old   │             /tmp/new              │
                                                                │  allocs/op  │  allocs/op   vs base              │
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=10-10     177.0 ± 0%    176.0 ± 0%  -0.56% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=50-10     699.0 ± 0%    698.0 ± 0%  -0.14% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=10;_seriesCount=100-10   1.349k ± 0%   1.348k ± 0%  -0.07% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=10-10     179.0 ± 0%    178.0 ± 0%  -0.56% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=50-10     699.0 ± 0%    698.0 ± 0%  -0.14% (p=0.002 n=6)
Ingester_QueryStream_Chunks/samplesCount=50;_seriesCount=100-10   1.349k ± 0%   1.348k ± 0%  -0.07% (p=0.002 n=6)
geomean                                                            551.6         550.2       -0.26%

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • [NA] Documentation added
  • [NA] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Comment on lines +3069 to +3093
func (i *Ingester) getInstanceLimits() *InstanceLimits {
// Don't apply any limits while starting. We especially don't want to apply series in memory limit while replaying WAL.
if i.State() == services.Starting {
return nil
}

if i.cfg.InstanceLimitsFn == nil {
return defaultInstanceLimits
}

l := i.cfg.InstanceLimitsFn()
if l == nil {
return defaultInstanceLimits
}

return l
}

// stopIncomingRequests is called during the shutdown process.
func (i *Ingester) stopIncomingRequests() {
i.stoppedMtx.Lock()
defer i.stoppedMtx.Unlock()
i.stopped = true
}

Copy link
Member Author

Choose a reason for hiding this comment

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

No changes on these functions.. I just moved to be close to the other ingesters functions.

@alanprot alanprot merged commit 9d9d4bf into cortexproject:master Dec 31, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants