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

[vtctld] sleep/ping tablets #8826

Merged
merged 5 commits into from
Sep 21, 2021
Merged

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Sep 15, 2021

Description

This PR migrates the Sleep and Ping legacy vtctl commands, renamed to SleepTablet and PingTablet since that is the component they operate on.

Demo

❯ vtctldclient --server ":15999" SleepTablet zone1-100 10ms
❯ time vtctldclient --server ":15999" SleepTablet zone1-100 10s
vtctldclient --server ":15999" SleepTablet zone1-100 10s  0.01s user 0.01s system 0% cpu 10.032 total
❯ time vtctldclient --server ":15999" SleepTablet zone1-100 10ms
vtctldclient --server ":15999" SleepTablet zone1-100 10ms  0.01s user 0.01s system 59% cpu 0.040 total
❯ time vtctldclient --server ":15999" SleepTablet zone1-100 1s
vtctldclient --server ":15999" SleepTablet zone1-100 1s  0.02s user 0.01s system 2% cpu 1.035 total
❯ vtctldclient --server ":15999" PingTablet zone1-100

Related Issue(s)

#7135

Checklist

  • Should this PR be backported? No
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@ajm188
Copy link
Contributor Author

ajm188 commented Sep 15, 2021

Run gofmt -l . | grep -vF vendor/ && exit 1 || echo "All files formatted correctly"
vitess-mixin/tools.go
Error: Process completed with exit code 1.

Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Copy link
Contributor

@doeg doeg left a comment

Choose a reason for hiding this comment

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

This looks good as always. I have two small questions.

// SleepTablet makes a SleepTablet gRPC call to a vtctld.
SleepTablet = &cobra.Command{
Use: "SleepTablet <alias> <duration>",
Short: "Blocks the action queue on the specified tablet for the specified amount of time. This is typically used for testing.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe an inline example would be helpful for "specified amount of time"? (Unless it's super obvious to Vitess operators.) We could steal what's noted here

The value is a string that contains a possibly signed sequence of decimal numbers, each with optional fraction and a unit suffix, such as “300ms” or “1h45m”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do!!

if err != nil {
return nil, err
} else if !ok {
dur = *topo.RemoteOperationTimeout
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm kinda surprised that we don't return if we can't parse the duration. Does using RemoteOperationTimeout here mean "sleep for the maximum timeout"? Or will tmc.Sleep fail?

(I did a bit of hunting in the codebase to answer this for myself but wound up in an interface declaration, and Sleep is... a cumbersome search term, haha.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the three-tuple here is (parsedDuration, wasDurationSet, error), so if the duration is nil you get back 0, false, nil, and we default to *topo.RemoteOperationTimeout. If we actually fail to parse, then the third value is non-nil and we take the earlier case (err != nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(it's named "ok" to mirror the map two-value semantic of (value, isThereAValue))

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 merged commit f8afa20 into vitessio:main Sep 21, 2021
@ajm188 ajm188 deleted the am_vtctld_sleep_ping branch September 21, 2021 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants