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: rename failover tests #103576

Conversation

erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented May 18, 2023

This will require a fair bit of manual renaming of roachperf data, I'll handle that once this merges.


This patch restructures the naming scheme to make it more readable and understandable. The new names are:

failover/chaos/read-only/lease=epoch
failover/chaos/read-only/lease=expiration
failover/chaos/read-write/lease=epoch
failover/chaos/read-write/lease=expiration
failover/crash/liveness/lease=epoch
failover/crash/liveness/lease=expiration
failover/crash/system/lease=epoch
failover/crash/system/lease=expiration
failover/crash/user/lease=epoch
failover/crash/user/lease=expiration
failover/deadlock/liveness/lease=epoch
failover/deadlock/liveness/lease=expiration
failover/deadlock/system/lease=epoch
failover/deadlock/system/lease=expiration
failover/deadlock/user/lease=epoch
failover/deadlock/user/lease=expiration
failover/disk-stall/liveness/lease=epoch
failover/disk-stall/liveness/lease=expiration
failover/disk-stall/system/lease=epoch
failover/disk-stall/system/lease=expiration
failover/disk-stall/user/lease=epoch
failover/disk-stall/user/lease=expiration
failover/partition-full/liveness/lease=epoch
failover/partition-full/liveness/lease=expiration
failover/partition-full/system/lease=epoch
failover/partition-full/system/lease=expiration
failover/partition-full/user/lease=epoch
failover/partition-full/user/lease=expiration
failover/partition-receive/liveness/lease=epoch
failover/partition-receive/liveness/lease=expiration
failover/partition-receive/system/lease=epoch
failover/partition-receive/system/lease=expiration
failover/partition-receive/user/lease=epoch
failover/partition-receive/user/lease=expiration
failover/partition-send/liveness/lease=epoch
failover/partition-send/liveness/lease=expiration
failover/partition-send/system/lease=epoch
failover/partition-send/system/lease=expiration
failover/partition-send/user/lease=epoch
failover/partition-send/user/lease=expiration
failover/partition-partial/lease-gateway/lease=epoch
failover/partition-partial/lease-gateway/lease=expiration
failover/partition-partial/lease-leader/lease=epoch
failover/partition-partial/lease-leader/lease=expiration
failover/partition-partial/lease-liveness/lease=epoch
failover/partition-partial/lease-liveness/lease=expiration
failover/pause/liveness/lease=epoch
failover/pause/liveness/lease=expiration
failover/pause/system/lease=epoch
failover/pause/system/lease=expiration
failover/pause/user/lease=epoch
failover/pause/user/lease=expiration

Epic: none
Release note: None

@erikgrinaker erikgrinaker requested review from andrewbaptist and a team May 18, 2023 10:13
@erikgrinaker erikgrinaker requested a review from a team as a code owner May 18, 2023 10:13
@erikgrinaker erikgrinaker self-assigned this May 18, 2023
@erikgrinaker erikgrinaker requested review from a team as code owners May 18, 2023 10:13
@erikgrinaker erikgrinaker requested review from herkolategan and renatolabs and removed request for a team May 18, 2023 10:13
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@andrewbaptist andrewbaptist left a comment

Choose a reason for hiding this comment

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

:lgtm: Thanks for the rename, the names were getting unwieldy.

@erikgrinaker
Copy link
Contributor Author

I'm unsure about "user" for regular non-system ranges. It could be confused with the user table. Any better ideas? Normal?

@andrewbaptist
Copy link
Collaborator

I like "user", although I agree it might be confusing. Maybe "customer", "client" or "consumer" could work as well to not confuse it with the user table. Any would be fine with me.

@erikgrinaker
Copy link
Contributor Author

Ok thanks, let's stick with user then.

@erikgrinaker
Copy link
Contributor Author

Changed partition-(input|output) to partition-(receive|send).

@erikgrinaker erikgrinaker force-pushed the roachtest-failover-rename branch 3 times, most recently from 59b8485 to ab5b3a7 Compare May 27, 2023 17:37
This patch restructures the naming scheme to make it more readable and
understandable. The new names are:

```
failover/chaos/read-only/lease=epoch
failover/chaos/read-only/lease=expiration
failover/chaos/read-write/lease=epoch
failover/chaos/read-write/lease=expiration
failover/crash/liveness/lease=epoch
failover/crash/liveness/lease=expiration
failover/crash/system/lease=epoch
failover/crash/system/lease=expiration
failover/crash/user/lease=epoch
failover/crash/user/lease=expiration
failover/deadlock/liveness/lease=epoch
failover/deadlock/liveness/lease=expiration
failover/deadlock/system/lease=epoch
failover/deadlock/system/lease=expiration
failover/deadlock/user/lease=epoch
failover/deadlock/user/lease=expiration
failover/disk-stall/liveness/lease=epoch
failover/disk-stall/liveness/lease=expiration
failover/disk-stall/system/lease=epoch
failover/disk-stall/system/lease=expiration
failover/disk-stall/user/lease=epoch
failover/disk-stall/user/lease=expiration
failover/partition-full/liveness/lease=epoch
failover/partition-full/liveness/lease=expiration
failover/partition-full/system/lease=epoch
failover/partition-full/system/lease=expiration
failover/partition-full/user/lease=epoch
failover/partition-full/user/lease=expiration
failover/partition-receive/liveness/lease=epoch
failover/partition-receive/liveness/lease=expiration
failover/partition-receive/system/lease=epoch
failover/partition-receive/system/lease=expiration
failover/partition-receive/user/lease=epoch
failover/partition-receive/user/lease=expiration
failover/partition-send/liveness/lease=epoch
failover/partition-send/liveness/lease=expiration
failover/partition-send/system/lease=epoch
failover/partition-send/system/lease=expiration
failover/partition-send/user/lease=epoch
failover/partition-send/user/lease=expiration
failover/partition-partial/lease-gateway/lease=epoch
failover/partition-partial/lease-gateway/lease=expiration
failover/partition-partial/lease-leader/lease=epoch
failover/partition-partial/lease-leader/lease=expiration
failover/partition-partial/lease-liveness/lease=epoch
failover/partition-partial/lease-liveness/lease=expiration
failover/pause/liveness/lease=epoch
failover/pause/liveness/lease=expiration
failover/pause/system/lease=epoch
failover/pause/system/lease=expiration
failover/pause/user/lease=epoch
failover/pause/user/lease=expiration
```

Epic: none
Release note: None
Copy link
Collaborator

@herkolategan herkolategan left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @renatolabs)

@andrewbaptist
Copy link
Collaborator

@erikgrinaker Do you plan to merge this? It would be nice to get this in prior to 24.1 stability so we have the new names.

@erikgrinaker
Copy link
Contributor Author

It needs a bunch of manual work to move the existing artifacts around. test-eng was going to look into this, but I don't think it ever happened.

@cockroachdb/test-eng Do we have an easy way to move these artifacts around, or is it all manual?

@srosenberg
Copy link
Member

@cockroachdb/test-eng Do we have an easy way to move these artifacts around, or is it all manual?

Renaming artifacts is a manual step. It shouldn't take more than gsutil mv or a one-time update of syncRawData to match the renaming rules.

@erikgrinaker
Copy link
Contributor Author

Yep, I'll take a pass at this once I've landed all the required 24.1 pre-stability work.

@erikgrinaker
Copy link
Contributor Author

I won't have time to get to this, closing.

@andrewbaptist
Copy link
Collaborator

I created a new PR #121211 to take this over and merge after the 24.1 branch cut (on the new master for 24.2). @srosenberg I will work with you over the couple weeks to figure out if this is something we should do and if so when. There are some other reporting changes I want to make to these tests as well over the next few weeks, and worst case we can just "lose" the history of this test if we can't figure out how to migrate this.

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.

None yet

5 participants