Skip to content

remote write 2.0: add write handler benchmark for V2 format#16687

Merged
cstyan merged 1 commit intoprometheus:mainfrom
SandroJijavadze:remote-write-handler-v2-benchmark-test
Jul 23, 2025
Merged

remote write 2.0: add write handler benchmark for V2 format#16687
cstyan merged 1 commit intoprometheus:mainfrom
SandroJijavadze:remote-write-handler-v2-benchmark-test

Conversation

@SandroJijavadze
Copy link
Contributor

@SandroJijavadze SandroJijavadze commented Jun 4, 2025

Fixes #14408

I am assuming the following was in reference to V1/V2 messages and removed it:

// TODO: test with other proto format(s)

Should V2 coverage for BenchmarkRemoteWriteOOOSamples also have a GitHub issue?

cstyan
cstyan requested changes Jun 11, 2025
View reviewed changes
Copy link
Member

@cstyan cstyan left a comment

Choose a reason for hiding this comment

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

Should V2 coverage for BenchmarkRemoteWriteOOOSamples also have a GitHub issue?

Doesn't necessarily need a GH issue but it could be updated to support v2 as well. Though that benchmark looks to need a bit of a refactor anyways as it wasn't utilizing b.N in the test at all and is currently being skipped.

@SandroJijavadze SandroJijavadze force-pushed the remote-write-handler-v2-benchmark-test branch 2 times, most recently from f89c3aa to a285b71 Compare June 19, 2025 20:10
@SandroJijavadze SandroJijavadze requested a review from cstyan June 20, 2025 13:52
@cstyan
Copy link
Member

cstyan commented Jun 20, 2025

Looks good, just curious what the difference in benchmark results look like now. I can run them myself sometime later today If you don't have time to post them here before then.

@SandroJijavadze
Copy link
Contributor Author

SandroJijavadze commented Jun 20, 2025

Sure,

goos: linux
goarch: amd64
pkg: github.com/prometheus/prometheus/storage/remote
cpu: 12th Gen Intel(R) Core(TM) i7-1260P
BenchmarkRemoteWriteHandler/V1_Write-16                   163921              8144 ns/op            3587 B/op         42 allocs/op
BenchmarkRemoteWriteHandler/V2_Write-16                   220556              5768 ns/op            2968 B/op         28 allocs/op
PASS
ok      github.com/prometheus/prometheus/storage/remote 55.937s

This is in comparison to main (cause test also changed):
benchstat:

$ benchstat old.txt new.txt
goos: linux
goarch: amd64
pkg: github.com/prometheus/prometheus/storage/remote
cpu: 12th Gen Intel(R) Core(TM) i7-1260P
                               │   old.txt    │       new.txt       │
                               │    sec/op    │    sec/op     vs base   │
RemoteWriteHandler-16            6.598µ ± ∞ ¹
RemoteWriteHandler/V1_Write-16                  8.144µ ± ∞ ¹
RemoteWriteHandler/V2_Write-16                  5.768µ ± ∞ ¹
geomean                          6.598µ         6.854µ        ? ² ³
¹ need >= 6 samples for confidence interval at level 0.95
² benchmark set differs from baseline; geomeans may not be comparable
³ ratios must be >0 to compute geomean

                               │    old.txt    │       new.txt        │
                               │     B/op      │     B/op       vs base   │
RemoteWriteHandler-16            3.166Ki ± ∞ ¹
RemoteWriteHandler/V1_Write-16                   3.503Ki ± ∞ ¹
RemoteWriteHandler/V2_Write-16                   2.898Ki ± ∞ ¹
geomean                          3.166Ki         3.186Ki        ? ² ³
¹ need >= 6 samples for confidence interval at level 0.95
² benchmark set differs from baseline; geomeans may not be comparable
³ ratios must be >0 to compute geomean

                               │   old.txt   │      new.txt       │
                               │  allocs/op  │  allocs/op   vs base   │
RemoteWriteHandler-16            34.00 ± ∞ ¹
RemoteWriteHandler/V1_Write-16                 42.00 ± ∞ ¹
RemoteWriteHandler/V2_Write-16                 28.00 ± ∞ ¹
geomean                          34.00         34.29        ? ² ³
¹ need >= 6 samples for confidence interval at level 0.95
² benchmark set differs from baseline; geomeans may not be comparable
³ ratios must be >0 to compute geomean

cstyan
cstyan reviewed Jul 9, 2025
View reviewed changes
@SandroJijavadze
remote write 2.0: add write handler benchmark for V2 format
b516f23 
Signed-off-by: Sandro Jijavadze <sandrojijavadze@protonmail.com>
@SandroJijavadze SandroJijavadze force-pushed the remote-write-handler-v2-benchmark-test branch from a285b71 to b516f23 Compare July 10, 2025 16:20
@SandroJijavadze SandroJijavadze requested a review from cstyan July 10, 2025 16:22
@cstyan cstyan merged commit f8b3fce into prometheus:main Jul 23, 2025
27 checks passed
tcp13equals2 pushed a commit to tcp13equals2/prometheus that referenced this pull request Aug 18, 2025
@SandroJijavadze @tcp13equals2
remote write 2.0: add write handler benchmark for V2 format (promethe…
ec5a11a 
…us#16687)

Signed-off-by: Sandro Jijavadze <sandrojijavadze@protonmail.com>
Signed-off-by: Andrew Hall <andrew.hall@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cstyan cstyan cstyan approved these changes

@bwplotka bwplotka Awaiting requested review from bwplotka bwplotka is a code owner

@tomwilkie tomwilkie Awaiting requested review from tomwilkie tomwilkie is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

remote write 2.0 - update write handler benchmarks for 2.0 format

2 participants

@SandroJijavadze @cstyan