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

TBS: Optimize for ReadTraceEvents miss #13464

Merged
merged 4 commits into from
Jun 24, 2024
Merged

Conversation

carsonip
Copy link
Member

@carsonip carsonip commented Jun 20, 2024

Motivation/summary

When a sampling decision is received from a remote apm-server, if the trace is not stored locally, prefetching values in the iterator is causing mmap vlog to be read for no value, resulting in high memory usage and disk read IO.

Add an initial pass with PrefetchValues=false to optimize for a miss in ReadTraceEvents.

e.g. if there are 10 apm-servers all running TBS and synced via ES, and all transactions/spans from a trace are always sent to in 1 apm-server (i.e., a single trace never stored across multiple apm-servers), this PR should cut disk read IO related to sampling decision handling to 1/10 of before.

Benchmark results

Since the benchmark runs badger DB with in-memory mode, I expect in reality, hit=false after this fix will stay the same, while all the other numbers to be 1 order of magnitude slower. i.e. the performance gain from this PR will be even greater.

goos: linux
goarch: amd64
pkg: github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
                                       │ bench-read-events-hit.before │     bench-read-events-hit.after     │
                                       │            sec/op            │    sec/op     vs base               │
ReadEventsHit/bigTX=true/hit=false-16                   167.06µ ± 14%   14.31µ ± 32%  -91.43% (p=0.002 n=6)
ReadEventsHit/bigTX=true/hit=true-16                     174.4µ ±  4%   186.3µ ±  4%   +6.81% (p=0.004 n=6)
ReadEventsHit/bigTX=false/hit=false-16                  129.49µ ±  3%   16.93µ ± 21%  -86.92% (p=0.002 n=6)
ReadEventsHit/bigTX=false/hit=true-16                    136.5µ ±  3%   139.8µ ±  3%   +2.40% (p=0.026 n=6)
geomean                                                  150.7µ         50.13µ        -66.73%

Checklist

- [ ] Update CHANGELOG.asciidoc Require backport release changelog
- [ ] Documentation has been updated

For functional changes, consider:

  • Is it observable through the addition of either logging or metrics?
  • Is its use being published in telemetry to enable product improvement?
  • Have system tests been added to avoid regression?

How to test these changes

See benchmark

Related issues

Closes #13463

@carsonip carsonip requested a review from a team as a code owner June 20, 2024 16:34
@carsonip carsonip added the backport-8.14 Automated backport with mergify label Jun 20, 2024
@carsonip carsonip marked this pull request as draft June 20, 2024 16:44
@carsonip carsonip changed the title TBS: Reduce memory usage and disk IO by disabling PrefetchValues TBS: Do not prefetch values for missing traces Jun 20, 2024
Prefetching values for non-existent traces from remote apm-servers is
causing mmap vlog to be read for no value, resulting in high memory
usage and disk IO.

Add an initial pass with PrefetchValues=false to optimize for a miss.
@carsonip carsonip changed the title TBS: Do not prefetch values for missing traces TBS: Optimize for ReadTraceEvents miss Jun 20, 2024
@carsonip carsonip marked this pull request as ready for review June 20, 2024 22:12
@vikmik vikmik requested a review from a team June 20, 2024 22:56
axw
axw previously approved these changes Jun 21, 2024
@carsonip carsonip enabled auto-merge (squash) June 24, 2024 10:53
@carsonip carsonip merged commit 2558f92 into elastic:main Jun 24, 2024
10 checks passed
mergify bot pushed a commit that referenced this pull request Jun 24, 2024
When a sampling decision is received from a remote apm-server, if the trace is not stored locally, prefetching values in the iterator is causing mmap vlog to be read for no value, resulting in high memory usage and disk read IO.

Add an initial pass with PrefetchValues=false to optimize for a miss in ReadTraceEvents.

e.g. if there are 10 apm-servers all running TBS and synced via ES, and all transactions/spans from a trace are always sent to in 1 apm-server (i.e., a single trace never stored across multiple apm-servers), this PR should cut disk read IO related to sampling decision handling to 1/10 of before.

(cherry picked from commit 2558f92)
mergify bot added a commit that referenced this pull request Jun 24, 2024
When a sampling decision is received from a remote apm-server, if the trace is not stored locally, prefetching values in the iterator is causing mmap vlog to be read for no value, resulting in high memory usage and disk read IO.

Add an initial pass with PrefetchValues=false to optimize for a miss in ReadTraceEvents.

e.g. if there are 10 apm-servers all running TBS and synced via ES, and all transactions/spans from a trace are always sent to in 1 apm-server (i.e., a single trace never stored across multiple apm-servers), this PR should cut disk read IO related to sampling decision handling to 1/10 of before.

(cherry picked from commit 2558f92)

Co-authored-by: Carson Ip <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.14 Automated backport with mergify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High "mapped" memory usage and disk IO when tail-based sampling is enabled
3 participants