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.

Sign up for GitHub

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

Jump to bottom

execbuilder: deflake TestExecBuild #75304

Merged
merged 2 commits into from
Jan 25, 2022
Merged

execbuilder: deflake TestExecBuild #75304

merged 2 commits into from
Jan 25, 2022

Commits on Jan 24, 2022

  1. execbuilder: deflake TestExecBuild 

    Fixes cockroachdb#74933. This test asserts on physical plans of statements after
moving ranges around, i.e. whether distsql execution is "local" or
"distributed". This is determined by:
- where the distsql planner places processor cores,
- which in turn is a function of the span resolver,
- which delegates to the embedded distsender,
- which operates off of a range cache.

The range cache, like the name suggests, can hold stale data. This test
moved replicas of indexes around to induce distributed execution, but
was not accounting for the caches holding onto stale data. In my test
runs every so often I was seeing stale range descriptors from before the
relocate trip up the planner to generate a local execution, flaking the
test. Fortunately for us, all we need to do is slap on a retry. To
repro:

    # Took under a minute on my gceworker. Helped to trim down the test
    # file slightly.
    dev test pkg/sql/opt/exec/execbuilder \
      -f 'TestExecBuild/5node' -v --show-logs \
      --stress --stress-args '-p 4'

Release note: None
    @irfansharif
    irfansharif committed Jan 24, 2022
    Configuration menu
    Copy the full SHA
    c538f2a View commit details
    Browse the repository at this point in the history

  2. logictest: (speculatively) de-flake distsql_enum 

    We opted this test out of using the span configs infra in cockroachdb#75281 after
observing a CI flake. Following the analysis in the last commit, it may
have been due to stale distsender caches affecting the physical plan
generated.

This fix is speculative because I was unable to repro the original flake
under stress. Attempt (successful after 1000s of runs on GCE worker over
20+ minutes):

    dev test pkg/sql/logictest \
    -f 'TestLogic/^5node-disk$/distsql_enum' \
    --show-logs -v --stress --stress-args '-p 4' -- --test_arg -show-sql

Release note: None
    @irfansharif
    irfansharif committed Jan 24, 2022
    Configuration menu
    Copy the full SHA
    83f584d View commit details
    Browse the repository at this point in the history