Skip to content

roachtest: add MinVersion to scrub tests#33573

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
thoszhang:scrub-roachtest-version
Jan 9, 2019
Merged

roachtest: add MinVersion to scrub tests#33573
craig[bot] merged 1 commit intocockroachdb:masterfrom
thoszhang:scrub-roachtest-version

Conversation

@thoszhang
Copy link
Copy Markdown

The scrub tests have been passing on master, but failing on 2.1 because there
was a bug (fixed in #32908).

Release note: None

The `scrub` tests have been passing on master, but failing on 2.1 because there
was a bug (fixed in #32908).

Release note: None
@thoszhang thoszhang requested a review from vivekmenezes January 8, 2019 21:26
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@vivekmenezes vivekmenezes left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

Copy link
Copy Markdown
Contributor

@vivekmenezes vivekmenezes left a comment

Choose a reason for hiding this comment

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

thanks!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@thoszhang
Copy link
Copy Markdown
Author

bors r+

craig bot pushed a commit that referenced this pull request Jan 9, 2019
33573: roachtest: add MinVersion to scrub tests r=lucy-zhang a=lucy-zhang

The `scrub` tests have been passing on master, but failing on 2.1 because there
was a bug (fixed in #32908).

Release note: None

33582: storage: bump RaftDelaySplitToSuppressSnapshotTicks r=knz a=knz

The admin split path must accommodate a scenario where a range with
not-yet-replicated followers is being manually split multiple
times (eg. IMPORT during TPCC test fixtures). This scenario results in
a bunch of replicas that all need to be populated with
snapshots. To avoid backing up the raft snapshot queue, a heuristic
was put in place (#32594) to delay the admin split if there is another
snapshot being applied already.

As shown by investigation in a failing test, there is a mismatch
between the configured max delay for this heuristic (20s) and the
actual duration of the snapshot application - the latter is limited by
the max bandwidth for snapshots, 2 MB/s resulting in 32s applications
in the worst case. We (Tobias and I) suspect that the heuristic thus
fails to wait long enough to have the protective effect it was
designed to provide.

The current patch increases this delay to exceed this snapshot
application duration estimate to about 50s.

Note that this scenario is not likely to occur now that #32782 has
been merged (this reduces the need for raft snapshots during splits);
however in release-2.1 where that patch was not applied, the scenario
is more likely.

Release note (bug fix): resolve a cluster degradation scenario that
could occur during IMPORT/RESTORE operations, manifested through a
high number of pending Raft snapshots.

Co-authored-by: Lucy Zhang <lucy-zhang@users.noreply.github.com>
Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 9, 2019

Build succeeded

@craig craig bot merged commit 6a295c8 into cockroachdb:master Jan 9, 2019
@thoszhang thoszhang deleted the scrub-roachtest-version branch February 12, 2019 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants