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: make roachtest recognize 21.2 #69829

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

celiala
Copy link
Collaborator

@celiala celiala commented Sep 3, 2021

Shortly after Creating a release branch for release-21.2, we'll want to merge PRs as instructed by the New major version checklist.

This PR is for Step 2c of the New major version checklist, where we update the ORM tests. Specifically, adding block/ignore lists for the vNEXT major version for these files:

ls pkg/cmd/roachtest/tests/*_blocklist.go

This PR is one of the tasks detailed in #70751, which tracks all the steps relevant to creating a release branch for ${vBRANCH_CUT} and preparing master for the ${vNEXT} major version.

Release justification: Non-production code change.
Release note: None

@celiala celiala added the do-not-merge bors won't merge a PR with this label. label Sep 3, 2021
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@celiala celiala changed the title clusterversion: mint 21.2 cluster version roachtest: make roachtest recognize 21.2 Sep 3, 2021
@celiala celiala force-pushed the roachtest-recognize-22.1 branch from d5f34c6 to 70567ad Compare September 3, 2021 22:51
var activeRecordBlockList21_2 = blocklist{}

var activeRecordBlockList21_1 = blocklist{}

var activeRecordBlockList20_2 = blocklist{}

// TODO(celia) -- what should be in blocklist{}, if anything?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe @rafiss ?

var djangoBlocklist21_2 = djangoBlocklist21_1

var djangoBlocklist21_1 = djangoBlocklist20_2

var djangoBlocklist20_2 = blocklist{}

// TODO(celia) -- what should be in blocklist{}, if anything?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe @RichardJCai ?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably just start with empty blocklists here and see what errors we run into.

var hibernateSpatialBlockList21_2 = blocklist{}

var hibernateSpatialBlockList21_1 = blocklist{}

// TODO(celia) -- what should be in blocklist{}, if anything?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}

// Please keep these lists alphabetized for easy diffing.
// After a failed run, an updated version of this blocklist should be available
// in the test log.
// TODO(celia) -- what should be in blocklist{}, if anything?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rafiss - here too, thx!

@@ -27,6 +28,9 @@ var psycopgBlocklists = blocklistsForVersion{
// Please keep these lists alphabetized for easy diffing.
// After a failed run, an updated version of this blocklist should be available
// in the test log.
// TODO(celia) -- what should be in blocklist{}, if anything?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

here too @rafiss - thank you :)

@@ -31,6 +31,9 @@ import (
"github.com/cockroachdb/cockroach/pkg/workload/tpch"
)

// TODO(celia) -- there used to be a toCRDBVersion() here -- confirm that
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe @yuzefovich ?

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @celiala)


pkg/cmd/roachtest/tests/tpchvec.go, line 34 at r1 (raw file):

Previously, celiala wrote…

Maybe @yuzefovich ?

It's all good - it was no longer needed and was removed.

@celiala celiala force-pushed the roachtest-recognize-22.1 branch from 70567ad to 2762308 Compare September 8, 2021 22:33
@celiala celiala requested a review from rafiss September 8, 2021 22:36
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @celiala)


pkg/cmd/roachtest/tests/django_blocklist.go, line 180 at r1 (raw file):

Previously, RichardJCai (Richard Cai) wrote…

We can probably just start with empty blocklists here and see what errors we run into.

+1 here's the full approach we should use:

  • if the v21.2 blocklist is empty, then the new v22.1 blocklist should be empty too
  • if the v21.2 is not empty, then set var toolBlocklist22_1 = toolBlocklist21_2

craig bot pushed a commit that referenced this pull request Oct 1, 2021
69826: clusterversion: mint 21.2 cluster version r=celiala a=celiala

Shortly after [Creating a release branch](https://cockroachlabs.atlassian.net/wiki/spaces/ENG/pages/187859111/Creating+a+release+branch) for release-21.2, we'll want to merge PRs as instructed by the [New major version checklist](https://cockroachlabs.atlassian.net/wiki/spaces/ENG/pages/1522270228/New+major+version+checklist).

This PR is for Step 1a of the [New major version checklist](https://cockroachlabs.atlassian.net/wiki/spaces/ENG/pages/1522270228/New+major+version+checklist), where we add a cluster version StartX for the corresponding .0 release.

Notes: 
- **This should NOT be merged until we've released a beta** (adding `do-not-merge` until this happens)
- These are all PRs created as part of the steps of the [New major version checklist](https://cockroachlabs.atlassian.net/wiki/spaces/ENG/pages/1522270228/New+major+version+checklist):
	- Step 1a (you are here): #69826
	- Step 1b: #69828
	- Step 2a + Step 2b: #69827
	- Step 2c: #69829
	- Step 3: TODO

Release justification: Non-production code change.
Release note: None

70750: tenant: add endpoint with instant metrics r=darinpp a=darinpp

Previously the tenant process was serving various metrics on
`/_status/vars`. This endpoint has all the available metrics and these are
updated every 10 sec. Many of the metrics show a rate that is calculated
over the 10 sec interval. Some of the metrics are used by the cockroach
operator to monitor the CPU workload of the tenant process and use that
workload for automatic scaling. The 10 sec interval however is too long
and causes a slow scaling up. The reporting of high CPU utilization can
take up to 20 sec (to compute a delta). To resolve this, the PR adds a
new endpoint `/_status/load` that provides an instant reading of a
very small subset of the normal metrics - user and system CPU time for
now. By having these be instant, the client can retrieve in quick
succession, consecutive snapshots and compute a precise CPU utulization.
It also allows the client to control the interval between the two pulls
(as opposed to having it hard coded to 10 sec).

Release note: None

Co-authored-by: Celia La <[email protected]>
Co-authored-by: Darin Peshev <[email protected]>
Release justification: Non-production code change.
Release note: None
@celiala celiala force-pushed the roachtest-recognize-22.1 branch from 2762308 to d6ba75a Compare October 1, 2021 18:54
Copy link
Collaborator Author

@celiala celiala left a comment

Choose a reason for hiding this comment

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

Implemented feedback - this is RFAL:

  • In terms of exactly when to merge this, let me know if I should hold off until a specific time? Otherwise, I'll plan to merge this as soon as this looks good to reviewers. Thanks!

+1 here's the full approach we should use:

  • if the v21.2 blocklist is empty, then the new v22.1 blocklist should be empty too
  • if the v21.2 is not empty, then set var toolBlocklist22_1 = toolBlocklist21_2

Done

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rafiss, @RichardJCai, and @yuzefovich)


pkg/cmd/roachtest/tests/django_blocklist.go, line 180 at r1 (raw file):

+1 here's the full approach we should use:
if the v21.2 blocklist is empty, then the new v22.1 blocklist should be empty too
if the v21.2 is not empty, then set var toolBlocklist22_1 = toolBlocklist21_2

Done

@celiala celiala removed the do-not-merge bors won't merge a PR with this label. label Oct 1, 2021
@celiala celiala requested a review from rafiss October 1, 2021 20:15
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

thanks!

@celiala
Copy link
Collaborator Author

celiala commented Oct 1, 2021

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 1, 2021

Build succeeded:

@craig craig bot merged commit fb0940f into cockroachdb:master Oct 1, 2021
@celiala celiala deleted the roachtest-recognize-22.1 branch April 5, 2022 04:25
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.

5 participants