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

bench: BenchmarkTracing broken on master #62081

Closed
nvanbenschoten opened this issue Mar 16, 2021 · 0 comments · Fixed by #62118
Closed

bench: BenchmarkTracing broken on master #62081

nvanbenschoten opened this issue Mar 16, 2021 · 0 comments · Fixed by #62118
Assignees
Labels
A-benchmarking C-test-failure Broken test (automatically or manually discovered).

Comments

@nvanbenschoten
Copy link
Member

--- FAIL: BenchmarkTracing/Cockroach
    bench_test.go:429: error executing 'SET CLUSTER SETTING trace.mode = background': pq: unknown cluster setting 'trace.mode'
--- FAIL: BenchmarkTracing/MultinodeCockroach
    bench_test.go:429: error executing 'SET CLUSTER SETTING trace.mode = background': pq: unknown cluster setting 'trace.mode'

I'm not sure why our nightly bench CI target didn't catch this.

@nvanbenschoten nvanbenschoten added C-test-failure Broken test (automatically or manually discovered). A-benchmarking labels Mar 16, 2021
irfansharif added a commit to irfansharif/cockroach that referenced this issue Mar 17, 2021
Fixes cockroachdb#62081. It had rotted after we removed the trace.mode setting. It
now makes use of a debug-only cluster setting that if enabled, installs
real spans all sql statements. We'll defer the actual investigation into
the cause of the slow-down in future PRs.

    $ make bench PKG=./pkg/bench  TESTFLAGS='-benchtime=10000x -count 20' \
        BENCHES='BenchmarkTracing' BENCHTIMEOUT=50m > benchmark-tracing.txt
    $ cat benchmark-tracing.txt | grep -v tracing=t | sed 's/tracing=f//' > old.txt
    $ cat benchmark-tracing.txt | grep -v tracing=f | sed 's/tracing=t//' > new.txt
    $ benchstat old.txt new.txt

    name                                   old time/op    new time/op    delta
    Tracing/Cockroach//Scan1-24               382µs ± 2%     412µs ± 8%   +7.68%  (p=0.000 n=10+10)
    Tracing/Cockroach//Insert-24              579µs ± 2%     609µs ± 6%   +5.10%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Scan1-24      799µs ± 2%     885µs ± 1%  +10.76%  (p=0.000 n=10+9)
    Tracing/MultinodeCockroach//Insert-24    1.09ms ± 1%    1.14ms ± 2%   +5.04%  (p=0.000 n=9+10)

    name                                   old alloc/op   new alloc/op   delta
    Tracing/Cockroach//Scan1-24              25.3kB ± 5%    32.0kB ± 4%  +26.55%  (p=0.000 n=10+10)
    Tracing/Cockroach//Insert-24             38.0kB ± 6%    42.2kB ± 5%  +11.02%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Scan1-24     77.7kB ± 8%    97.1kB ±12%  +25.05%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Insert-24     107kB ± 8%     115kB ± 7%   +7.66%  (p=0.023 n=10+10)

    name                                   old allocs/op  new allocs/op  delta
    Tracing/Cockroach//Scan1-24                 256 ± 1%       280 ± 1%   +9.10%  (p=0.000 n=9+9)
    Tracing/Cockroach//Insert-24                301 ± 2%       321 ± 1%   +6.64%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Scan1-24        787 ± 2%       921 ± 2%  +17.04%  (p=0.000 n=9+9)
    Tracing/MultinodeCockroach//Insert-24       748 ± 1%       805 ± 2%   +7.61%  (p=0.000 n=9+9)

Release note: None
craig bot pushed a commit that referenced this issue Mar 23, 2021
62118: sql: revive BenchmarkTracing r=irfansharif a=irfansharif

Fixes #62081. It had rotted after we removed the trace.mode setting. It
now makes use of a testing knob if enabled, installs real spans all sql
statements. We'll defer the actual investigation into the cause of the
slow-down in future PRs (prototyped in #62227).

    $ make bench PKG=./pkg/bench  TESTFLAGS='-benchtime=10000x -count 20' \
        BENCHES='BenchmarkTracing' BENCHTIMEOUT=50m > benchmark-tracing.txt
    $ cat benchmark-tracing.txt | grep -v tracing=t | sed 's/tracing=f//' > old.txt
    $ cat benchmark-tracing.txt | grep -v tracing=f | sed 's/tracing=t//' > new.txt
    $ benchstat old.txt new.txt

    name                                   old time/op    new time/op    delta
    Tracing/Cockroach//Scan1-24               382µs ± 2%     412µs ± 8%   +7.68%  (p=0.000 n=10+10)
    Tracing/Cockroach//Insert-24              579µs ± 2%     609µs ± 6%   +5.10%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Scan1-24      799µs ± 2%     885µs ± 1%  +10.76%  (p=0.000 n=10+9)
    Tracing/MultinodeCockroach//Insert-24    1.09ms ± 1%    1.14ms ± 2%   +5.04%  (p=0.000 n=9+10)

    name                                   old alloc/op   new alloc/op   delta
    Tracing/Cockroach//Scan1-24              25.3kB ± 5%    32.0kB ± 4%  +26.55%  (p=0.000 n=10+10)
    Tracing/Cockroach//Insert-24             38.0kB ± 6%    42.2kB ± 5%  +11.02%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Scan1-24     77.7kB ± 8%    97.1kB ±12%  +25.05%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Insert-24     107kB ± 8%     115kB ± 7%   +7.66%  (p=0.023 n=10+10)

    name                                   old allocs/op  new allocs/op  delta
    Tracing/Cockroach//Scan1-24                 256 ± 1%       280 ± 1%   +9.10%  (p=0.000 n=9+9)
    Tracing/Cockroach//Insert-24                301 ± 2%       321 ± 1%   +6.64%  (p=0.000 n=10+10)
    Tracing/MultinodeCockroach//Scan1-24        787 ± 2%       921 ± 2%  +17.04%  (p=0.000 n=9+9)
    Tracing/MultinodeCockroach//Insert-24       748 ± 1%       805 ± 2%   +7.61%  (p=0.000 n=9+9)

Release note: None

62193: geo/geomfn: add point in polygon optimization to st_intersects r=otan a=andyyang890

This patch improves the performance of st_intersects for the
common use case of testing whether a (multi)polygon and a
(multi)point intersect.

Release note: None

Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Andy Yang <ayang@cockroachlabs.com>
@craig craig bot closed this as completed in af9e646 Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-benchmarking C-test-failure Broken test (automatically or manually discovered).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants