Skip to content

perf(storage): check dedicated column cache before decoding JSON - #7968

Open
julienduchesne wants to merge 1 commit into
mainfrom
julienduchesne/tempo-index-cache-first
Open

perf(storage): check dedicated column cache before decoding JSON#7968
julienduchesne wants to merge 1 commit into
mainfrom
julienduchesne/tempo-index-cache-first

Conversation

@julienduchesne

@julienduchesne julienduchesne commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Check the existing dedicated-column cache before parsing JSON during protobuf tenant-index reads. Repeated layouts currently enter Sonic before reaching the cache in UnmarshalJSON; cache misses retain the existing decoder and validation.

  • Preserve reused-receiver null behavior and the existing bounded cache.
  • Benchmark the complete Reader.TenantIndex read, decompression and decode operation. Correct the existing protobuf benchmark fixture, which used JSON and ignored decode errors.

Benchmarks

Go 1.27.1, Apple M3 Pro arm64, GOMAXPROCS=4; ten alternating before/after pairs at 300ms per case. Synthetic metadata varies IDs, timestamps and sizes, mixes active/compacted blocks, and uses ten shared layouts. In-memory object reads exclude network latency.

goos: darwin
goarch: arm64
pkg: github.com/grafana/tempo/tempodb/backend
cpu: Apple M3 Pro
                           │ index-before-final.txt │        index-after-final.txt        │
                           │         sec/op         │   sec/op     vs base                │
TenantIndex/shared/100-4              100.87µ ±  8%   34.56µ ± 8%  -65.74% (p=0.000 n=10)
TenantIndex/shared/1000-4              957.8µ ±  4%   291.2µ ± 6%  -69.59% (p=0.000 n=10)
TenantIndex/shared/10000-4             9.479m ± 11%   2.597m ± 5%  -72.60% (p=0.000 n=10)
TenantIndex/cold/1000-4               1028.7µ ±  4%   366.5µ ± 3%  -64.37% (p=0.000 n=10)
TenantIndex/churn/4096-4               22.31m ±  6%   21.66m ± 3%        ~ (p=0.165 n=10)
TenantIndex/none/1000-4                197.1µ ±  2%   201.7µ ± 7%        ~ (p=0.052 n=10)
geomean                                1.267m         589.2µ       -53.50%

                           │ index-before-final.txt │         index-after-final.txt          │
                           │          B/s           │      B/s       vs base                 │
TenantIndex/shared/100-4              38.41Mi ±  8%   112.09Mi ± 8%  +191.82% (p=0.000 n=10)
TenantIndex/shared/1000-4             36.28Mi ±  5%   119.31Mi ± 6%  +228.89% (p=0.000 n=10)
TenantIndex/shared/10000-4            33.52Mi ± 10%   122.34Mi ± 4%  +264.95% (p=0.000 n=10)
TenantIndex/cold/1000-4               33.77Mi ±  4%    94.80Mi ± 3%  +180.69% (p=0.000 n=10)
TenantIndex/churn/4096-4              6.495Mi ±  7%    6.685Mi ± 3%         ~ (p=0.159 n=10)
TenantIndex/none/1000-4               161.0Mi ±  2%    157.4Mi ± 7%         ~ (p=0.052 n=10)
geomean                               34.38Mi          73.92Mi       +115.04%

                           │ index-before-final.txt │        index-after-final.txt         │
                           │          B/op          │     B/op      vs base                │
TenantIndex/shared/100-4               239.1Ki ± 2%   101.1Ki ± 1%  -57.71% (p=0.000 n=10)
TenantIndex/shared/1000-4             2282.8Ki ± 1%   961.2Ki ± 1%  -57.89% (p=0.000 n=10)
TenantIndex/shared/10000-4            19.447Mi ± 1%   8.721Mi ± 1%  -55.15% (p=0.000 n=10)
TenantIndex/cold/1000-4                2.202Mi ± 1%   1.175Mi ± 1%  -46.62% (p=0.000 n=10)
TenantIndex/churn/4096-4               23.87Mi ± 1%   22.88Mi ± 1%   -4.16% (p=0.000 n=10)
TenantIndex/none/1000-4                527.1Ki ± 1%   526.6Ki ± 1%        ~ (p=0.684 n=10)
geomean                                2.548Mi        1.495Mi       -41.32%

                           │ index-before-final.txt │        index-after-final.txt        │
                           │       allocs/op        │  allocs/op   vs base                │
TenantIndex/shared/100-4                1045.0 ± 0%    645.0 ± 0%  -38.28% (p=0.000 n=10)
TenantIndex/shared/1000-4              10.238k ± 0%   6.234k ± 0%  -39.11% (p=0.000 n=10)
TenantIndex/shared/10000-4             102.06k ± 0%   62.04k ± 0%  -39.21% (p=0.000 n=10)
TenantIndex/cold/1000-4                10.918k ± 0%   6.954k ± 0%  -36.31% (p=0.000 n=10)
TenantIndex/churn/4096-4                259.0k ± 1%   257.2k ± 1%   -0.70% (p=0.024 n=10)
TenantIndex/none/1000-4                 6.236k ± 0%   6.236k ± 0%        ~ (p=0.387 n=10)
geomean                                 16.37k        11.86k       -27.56%

The 1,000-record shared case drops from 2,282.8 to 961.2 KiB/op and from 10,238 to 6,234 allocs/op. Fixed-workload CPU profiles for 5,000 complete reads total 5.17s before and 1.82s after (-64.8%). These are local benchmark gains; production backend-worker profiles attribute 1.15% of CPU and 5.10% of allocated bytes to this decoder, not the whole service.

Backend race tests, baseline/optimized compatibility tests, formatting and lint pass. The broad race suite was interrupted while live-store and WAL tests were still running; full-suite validation remains incomplete.

Ref: https://github.com/grafana/databases-sre/issues/419

@julienduchesne julienduchesne mentioned this pull request Sep 22, 2026
Closed
@julienduchesne
julienduchesne marked this pull request as ready for review September 23, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@knylander-grafana knylander-grafana Awaiting requested review from knylander-grafana knylander-grafana is a code owner
@mdisibio mdisibio Awaiting requested review from mdisibio mdisibio is a code owner
@mapno mapno Awaiting requested review from mapno mapno is a code owner
@yvrhdn yvrhdn Awaiting requested review from yvrhdn yvrhdn is a code owner
@zalegrala zalegrala Awaiting requested review from zalegrala zalegrala is a code owner
@electron0zero electron0zero Awaiting requested review from electron0zero electron0zero is a code owner
@ie-pham ie-pham Awaiting requested review from ie-pham ie-pham is a code owner
@stoewer stoewer Awaiting requested review from stoewer stoewer is a code owner
@javiermolinar javiermolinar Awaiting requested review from javiermolinar javiermolinar is a code owner
@carles-grafana carles-grafana Awaiting requested review from carles-grafana carles-grafana is a code owner
@ruslan-mikhailov ruslan-mikhailov Awaiting requested review from ruslan-mikhailov ruslan-mikhailov is a code owner
@mattdurham mattdurham Awaiting requested review from mattdurham mattdurham is a code owner
@zhxiaogg zhxiaogg Awaiting requested review from zhxiaogg zhxiaogg is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@julienduchesne