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

roachtest: tpchvec/perf failed #53884

Closed
cockroach-teamcity opened this issue Sep 3, 2020 · 2 comments
Closed

roachtest: tpchvec/perf failed #53884

cockroach-teamcity opened this issue Sep 3, 2020 · 2 comments
Assignees
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Milestone

Comments

@cockroach-teamcity
Copy link
Member

(roachtest).tpchvec/perf failed on master@11a82264044ed8edc7d1fca2b912296d6f576a1a:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /home/agent/work/.go/src/github.com/cockroachdb/cockroach/artifacts/tpchvec/perf/run_1
	tpchvec.go:379,tpchvec.go:671,tpchvec.go:683,test_runner.go:754: [q12] vec ON is slower by 17.25% than vec OFF
		vec ON times: [9.45 9.79 10.05]
		vec OFF times: [8.32 8.35 8.73]

More

Artifacts: /tpchvec/perf

See this test on roachdash
powered by pkg/cmd/internal/issues

@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Sep 3, 2020
@cockroach-teamcity cockroach-teamcity added this to the 20.2 milestone Sep 3, 2020
@asubiotto
Copy link
Contributor

cc @yuzefovich

@yuzefovich yuzefovich self-assigned this Sep 3, 2020
@yuzefovich
Copy link
Member

Q12 spends most of its time performing a lookup join. When I ran tpchvec/perf last time with 10 runs, I got that vectorized was faster by 3.31% on average, so I think this issue is an outlier (I've recently decreased the threshold from 20% to 15%). If I see another instance of this noise, I'll bump it back up.

What's confusing to me is that EXPLAIN ANALYZE contains the traces only when vectorized engine was used, both for vectorize=off and vectorize=on configs. I'll add some logging to tpchvec to understand it better (I don't see a problem in the code).

craig bot pushed a commit that referenced this issue Sep 10, 2020
53911: roachtest: fix tpchvec/perf in some cases r=yuzefovich a=yuzefovich

Previously, whenever a slowness threshold is exceeded, we would run
EXPLAIN ANALYZE (DEBUG) on the query at fault using the same connection
for all cluster setups. This would result in running the query only with
vectorize=on which is not what we want. We will now be opening up new
connections to the database after the cluster settings have been updated
in order to get the correct behavior for each of the setups.

Informs: #53884.

Release note: None

54072: server: introduce Test{ClusterConnectivity,JoinVersionGate} r=irfansharif a=irfansharif

Re-adding a few tests that I didn't check in while working on #52526.

---

TestClusterConnectivity sets up an uninitialized cluster with custom join
flags (individual nodes point to specific others, constructed using
connectivity matrices). Included in the test configurations are the more
"standard" ones where every node points to n1, or where there are
bidirectional links set up between nodes. It tests various topologies
and ensures that cluster IDs are distributed throughout connected
components, and store/node IDs are allocated in the way we'd expect them
to be.

TestJoinVersionGate checks to see that improperly versioned cockroach
nodes are not able to join a cluster.

Release note: None


54159: opt: reduce allocations in Implicator and JoinOrderBuilder r=RaduBerinde a=mgartner

#### partialidx: lazily initialize the implicator constraint cache

This commit makes the constructing of the Implicator's constrain cache
map lazy. Instead of making the map in the `Init` method, it is made
only when it is needed.

This fixes performance regressions in microbenchmarks.

    name                                  old time/op  new time/op  delta
    Phases/kv-read-const/Normalize-16      170ns ± 0%   139ns ± 0%  -18.66%  (p=0.008 n=5+5)
    Phases/kv-read-const/OptBuild-16       173ns ± 1%   141ns ± 1%  -18.38%  (p=0.008 n=5+5)
    Phases/kv-read-const/Explore-16        183ns ± 1%   151ns ± 0%  -17.70%  (p=0.008 n=5+5)
    Phases/kv-read-const/ExecBuild-16      674ns ± 2%   636ns ± 1%   -5.58%  (p=0.008 n=5+5)
    Phases/kv-read/ExecBuild-16           13.3µs ± 1%  13.1µs ± 1%   -1.78%  (p=0.008 n=5+5)
    Phases/tpcc-delivery/ExecBuild-16     29.4µs ± 1%  28.9µs ± 0%   -1.64%  (p=0.016 n=5+4)
    Phases/tpcc-stock-level/Explore-16     153µs ± 1%   151µs ± 1%   -1.24%  (p=0.008 n=5+5)
    Phases/kv-read-no-prep/Explore-16     42.3µs ± 1%  41.9µs ± 0%   -0.85%  (p=0.008 n=5+5)
    Phases/tpcc-delivery/OptBuild-16      12.1µs ± 1%  12.1µs ± 0%   -0.65%  (p=0.032 n=5+5)
    Phases/kv-read/Parse-16               2.49ns ± 1%  2.48ns ± 0%     ~     (p=0.413 n=5+4)
    Phases/kv-read/OptBuild-16            6.46µs ± 2%  6.44µs ± 1%     ~     (p=0.841 n=5+5)
    Phases/kv-read/Normalize-16           6.48µs ± 3%  6.48µs ± 2%     ~     (p=1.000 n=5+5)
    Phases/kv-read/Explore-16             12.1µs ± 1%  11.9µs ± 2%     ~     (p=0.056 n=5+5)
    Phases/kv-read-no-prep/OptBuild-16    32.4µs ± 3%  32.2µs ± 1%     ~     (p=1.000 n=5+5)
    Phases/kv-read-no-prep/Normalize-16   34.9µs ± 1%  34.7µs ± 2%     ~     (p=0.222 n=5+5)
    Phases/kv-read-no-prep/ExecBuild-16   44.0µs ± 3%  43.7µs ± 1%     ~     (p=0.310 n=5+5)
    Phases/kv-read-const/Parse-16         2.60ns ± 1%  2.60ns ± 1%     ~     (p=0.643 n=5+5)
    Phases/tpcc-new-order/Normalize-16    12.8µs ± 1%  13.9µs ±10%     ~     (p=0.690 n=5+5)
    Phases/tpcc-new-order/ExecBuild-16    38.5µs ± 1%  38.6µs ± 2%     ~     (p=1.000 n=5+5)
    Phases/tpcc-delivery/Parse-16         2.55ns ± 1%  2.56ns ± 1%     ~     (p=0.921 n=5+5)
    Phases/tpcc-delivery/Normalize-16     12.6µs ± 2%  12.5µs ± 1%     ~     (p=0.151 n=5+5)
    Phases/tpcc-delivery/Explore-16       28.0µs ± 2%  27.6µs ± 1%     ~     (p=0.063 n=5+5)
    Phases/tpcc-stock-level/OptBuild-16   52.8µs ± 1%  52.9µs ± 1%     ~     (p=1.000 n=5+5)
    Phases/tpcc-stock-level/Normalize-16  53.7µs ± 1%  53.7µs ± 1%     ~     (p=1.000 n=5+5)
    Phases/tpcc-stock-level/ExecBuild-16   160µs ± 1%   161µs ± 1%     ~     (p=0.222 n=5+5)
    Phases/tpcc-new-order/Explore-16      36.2µs ± 0%  36.7µs ± 1%   +1.47%  (p=0.016 n=4+5)
    Phases/kv-read-no-prep/Parse-16       12.6µs ± 1%  12.8µs ± 1%   +1.58%  (p=0.016 n=4+5)
    Phases/tpcc-new-order/Parse-16        2.58ns ± 1%  2.67ns ± 7%   +3.33%  (p=0.048 n=5+5)
    Phases/tpcc-stock-level/Parse-16      2.52ns ± 1%  2.63ns ± 4%   +4.29%  (p=0.016 n=5+5)
    Phases/tpcc-new-order/OptBuild-16     13.0µs ± 1%  14.7µs ± 2%  +12.50%  (p=0.008 n=5+5)

Release note: None

#### opt: embed JoinOrderBuilder in Optimizer

This commit embeds the JoinOrderBuilder in the Optimizer struct. This
avoids allocating a new JoinOrderBuilder on the heap every time
`Optimizer.Init` is called.

BenchmarkPhase microbenchmarks comparing the current master branch to
this commit:

    name                                  old time/op  new time/op  delta
    Phases/kv-read-const/Normalize-16      171ns ± 1%    77ns ± 1%  -55.11%  (p=0.008 n=5+5)
    Phases/kv-read-const/OptBuild-16       175ns ± 1%    79ns ± 2%  -54.83%  (p=0.008 n=5+5)
    Phases/kv-read-const/Explore-16        189ns ± 7%    89ns ± 0%  -53.14%  (p=0.008 n=5+5)
    Phases/tpcc-new-order/OptBuild-16     16.6µs ± 6%  12.7µs ± 2%  -23.47%  (p=0.008 n=5+5)
    Phases/tpcc-new-order/Normalize-16    14.7µs ± 8%  12.4µs ± 1%  -15.39%  (p=0.008 n=5+5)
    Phases/kv-read-const/ExecBuild-16      687ns ± 4%   592ns ± 1%  -13.89%  (p=0.008 n=5+5)
    Phases/tpcc-new-order/Parse-16        2.94ns ± 8%  2.58ns ± 2%  -12.23%  (p=0.008 n=5+5)
    Phases/tpcc-delivery/Parse-16         2.86ns ± 8%  2.54ns ± 1%  -11.33%  (p=0.008 n=5+5)
    Phases/tpcc-new-order/ExecBuild-16    43.0µs ±10%  38.6µs ± 0%  -10.21%  (p=0.016 n=5+4)
    Phases/tpcc-new-order/Explore-16      39.0µs ± 3%  36.6µs ± 0%   -6.12%  (p=0.008 n=5+5)
    Phases/tpcc-delivery/OptBuild-16      12.4µs ± 1%  12.0µs ± 0%   -3.49%  (p=0.008 n=5+5)
    Phases/kv-read/ExecBuild-16           13.4µs ± 0%  12.9µs ± 0%   -3.37%  (p=0.008 n=5+5)
    Phases/kv-read/Explore-16             12.1µs ± 1%  11.7µs ± 2%   -3.04%  (p=0.008 n=5+5)
    Phases/tpcc-delivery/Normalize-16     12.8µs ± 1%  12.4µs ± 1%   -3.00%  (p=0.008 n=5+5)
    Phases/kv-read-no-prep/ExecBuild-16   44.1µs ± 2%  43.2µs ± 1%   -2.16%  (p=0.008 n=5+5)
    Phases/kv-read/Normalize-16           6.48µs ± 1%  6.36µs ± 1%   -1.73%  (p=0.008 n=5+5)
    Phases/kv-read-no-prep/Explore-16     42.6µs ± 1%  41.9µs ± 1%   -1.55%  (p=0.032 n=5+5)
    Phases/tpcc-delivery/ExecBuild-16     29.5µs ± 1%  29.1µs ± 0%   -1.41%  (p=0.008 n=5+5)
    Phases/kv-read-no-prep/OptBuild-16    32.2µs ± 0%  31.7µs ± 1%   -1.34%  (p=0.008 n=5+5)
    Phases/kv-read-no-prep/Normalize-16   34.8µs ± 1%  34.4µs ± 1%   -1.25%  (p=0.016 n=5+4)
    Phases/tpcc-stock-level/OptBuild-16   53.1µs ± 0%  52.7µs ± 0%   -0.83%  (p=0.016 n=5+5)
    Phases/kv-read/Parse-16               2.49ns ± 0%  2.49ns ± 1%     ~     (p=0.952 n=4+5)
    Phases/kv-read/OptBuild-16            6.48µs ± 0%  6.44µs ± 1%     ~     (p=0.310 n=5+5)
    Phases/kv-read-no-prep/Parse-16       12.7µs ± 2%  12.7µs ± 1%     ~     (p=0.508 n=5+5)
    Phases/kv-read-const/Parse-16         2.59ns ± 1%  2.59ns ± 1%     ~     (p=0.857 n=5+5)
    Phases/tpcc-delivery/Explore-16       27.9µs ± 0%  27.9µs ± 2%     ~     (p=0.310 n=5+5)
    Phases/tpcc-stock-level/Parse-16      2.57ns ± 2%  2.54ns ± 3%     ~     (p=0.254 n=5+5)
    Phases/tpcc-stock-level/Normalize-16  53.8µs ± 0%  53.5µs ± 2%     ~     (p=0.151 n=5+5)
    Phases/tpcc-stock-level/Explore-16     153µs ± 0%   152µs ± 3%     ~     (p=0.151 n=5+5)
    Phases/tpcc-stock-level/ExecBuild-16   162µs ± 1%   161µs ± 1%     ~     (p=0.095 n=5+5)

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Projects
None yet
Development

No branches or pull requests

3 participants