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: deflake autoupgrade #99479

Closed
wants to merge 1 commit into from

Conversation

tbg
Copy link
Member

@tbg tbg commented Mar 24, 2023

@tbg tbg requested a review from aliher1911 March 24, 2023 14:22
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member Author

tbg commented Mar 24, 2023

@renatolabs any chance I can convince you/your team to own this autoupgrade roachtest? I'll note this disclaimer:

// NB: if you're interested in mixed-version testing, don't look at this test
// but check out acceptance/version-upgrade.
//
// NOTE: DO NOT USE THIS TEST AS A TEMPLATE FOR MIXED-VERSION TESTING.
// You want to look at versionupgrade.go, which has a test harness you
// can use.

so an argument could be made that it should simply be removed. Whether this is the right course of action would depend on what coverage of this, if any, we have.

@renatolabs
Copy link
Contributor

renatolabs commented Mar 24, 2023

own this autoupgrade roachtest?

Yep, I think this makes sense. I already had plans for it which may or may not include its removal 😄 But for now a change in ownership is enough, I'll come back to it in due time.

I also had the question of whether the blocking behavior of show cluster setting version is new, but I see you already asked that question.

Copy link
Contributor

@aliher1911 aliher1911 left a comment

Choose a reason for hiding this comment

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

Fix looks good for the flake. But this failure means nodes are on different versions so while local version is bumped, one in settings is not as something else is delaying it. Maybe it'll surface the real issue.

@tbg
Copy link
Member Author

tbg commented Mar 31, 2023

#99558 (comment)

@tbg tbg closed this Mar 31, 2023
tbg added a commit to tbg/cockroach that referenced this pull request Mar 31, 2023
Discussed in cockroachdb#99479.

Epic: none
Release note: None
craig bot pushed a commit that referenced this pull request Mar 31, 2023
99312: sqlsmith: add DEFAULT expressions to newly added columns r=mgartner a=mgartner

Sqlsmith now builds `ALTER TABLE .. ADD COLUMN .. DEFAULT` statements
with default expressions that have different types than the column type.
This is allowed if the default expression type can be assignment-casted
to the column's type.

Fixes #98133

Release note: None


99348: testutils: move default test tenant message r=rharding6373 a=herkolategan

In order to reduce logging noise but still inform test authors of the default test tenant, the message has been moved to where there is a `testing.TB` interface.

Epic: CRDB-18499

99835: opt/execbuilder: add panic catching to buildRoutinePlanGenerator r=mgartner a=mgartner

This commit adds a panic catcher to callback functions created in
execbuilder and invoked during evaluation of UDFs and correlated
subqueries. It matches the panic catcher logic in `buildApplyJoin`.

Fixes #98786

Release note: None


100267: roachtest: own autoupgrade to TestEng r=renatolabs a=tbg

Discussed in #99479.

Epic: none
Release note: None


100286: roachtest: prevent aws roachtest panic r=rail a=msbutler

After #99723 merged as a bandaid for #98783, the aws roachtest nightly began to panic because of a different roachtest papercut #96655. Specifically, because roachtest filters which tests run on which cloud within the evaluation of the test closure, tests meant to run on gce will still get registered in an AWS run. During the registration of the gce test
`restore/tpce/400GB/gce/nodes=4/cpus=8/lowmem` _on aws_, the aws test harness panics because the aws roachprod implementation does not have a low memory cpu configuration. This patch prevents this panic and should be reverted once the pr #99402 merges.

Epic: None

Release note: None

100294: tenantcapabilitiestestutils: add a missing default case r=ajwerner a=ajwerner

The test should fail if we ever add a new type of capability and use it in the data driven test but don't update the test to handle it.

Epic: none

Follow-up from #100217 (review)

Release note: None

100296: rpc: correctly check for nil before cast r=ajwerner a=andrewbaptist

As part of the fix of #99104, a cast without a nil check was introduced. This PR addresses that by only casting if it is known to be not nil.

Epic: none
Fixes: #100275
Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: ajwerner <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
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.

roachtest: autoupgrade failed
4 participants