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.

Sign up for GitHub

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

Jump to bottom

[vtctldserver] Add RefreshState RPCs #8232

Merged
merged 6 commits into from
Jun 14, 2021
Merged

[vtctldserver] Add RefreshState RPCs #8232

merged 6 commits into from
Jun 14, 2021

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Jun 2, 2021 •
edited

Description

This PR migrates the following RPCs to VtctldServer:

  • RefreshState
  • RefreshStateByShard

I also modified the return signature of topotools.RefreshTabletsByShard to indicate whether or not every tablet was (a) discoverable and (b) did not error on its RefreshState call so that we could signal back partial success to callers.

Usage 
❯ vtctldclient --server "localhost:15999" RefreshState zone1-100
Refreshed state on zone1-0000000100
❯ vtctldclient --server "localhost:15999" RefreshStateByShard commerce/0
Refreshed state on commerce/0
❯ ps aux | grep zone1-0000000102 | grep -v grep
amason           19397   1.6  0.2  5055664  83436 s011  S    10:04PM   0:01.79 vttablet -topo_implementation etcd2 -topo_global_server_address 127.0.0.1:2379 -topo_global_root /vitess/global -log_dir /Users/amason/work/vitess/vtdataroot/tmp -log_queries_to_file /Users/amason/work/vitess/vtdataroot/tmp/vttablet_0000000102_querylog.txt -tablet-path zone1-0000000102 -tablet_hostname  -init_keyspace commerce -init_shard 0 -init_tablet_type rdonly -health_check_interval 5s -enable_semi_sync -enable_replication_reporter -backup_storage_implementation file -file_backup_storage_root /Users/amason/work/vitess/vtdataroot/backups -restore_from_backup -port 15102 -grpc_port 16102 -service_map grpc-queryservice,grpc-tabletmanager,grpc-updatestream -pid_file /Users/amason/work/vitess/vtdataroot/vt_0000000102/vttablet.pid -vtctld_addr http://SFO-M-AMASON02:15000/
❯ kill -9 19397
❯ vtctldclient --server "localhost:15999" RefreshStateByShard commerce/0
Refreshed state on commerce/0
State refresh was partial; some tablets in the shard may not have succeeded.

Related Issue(s)

Knocks out two more of the items in #7135 (an oldie but a goodie).

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@ajm188 ajm188 added this to In progress in Vtctld Service via automation Jun 2, 2021
@ajm188 ajm188 requested a review from rafael June 2, 2021 02:26
@ajm188 ajm188 force-pushed the am_vtctldserver_refresh_state_rpcs branch from e8fe096 to 11e5c9d Compare June 8, 2021 14:55
@ajm188
Copy link
Contributor Author

ajm188 commented Jun 8, 2021

rebased and regenerated the protos

doeg
doeg approved these changes Jun 11, 2021
View reviewed changes
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 to me. Since this changes some ~ spicy call sites, I suggest an additional review from @deepthi, @rohit-nayak-ps, and/or @rafael.

rohit-nayak-ps
rohit-nayak-ps approved these changes Jun 13, 2021
View reviewed changes
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

lgtm

@ajm188
Add rpcs, run generators for RefreshState(ByShard)? methods
98d3cd4 
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188
[topotools] Refactor RefreshTabletsByShard to indicate if the refresh…
4c7b37e 
… was only partial

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188
[vtctldserver] Implement RefreshState(ByShard)? with tests
d246b9a 
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188
Add CLI entrypoints for RefreshState and RefreshStateByShard
69b7861 
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188
Add help texts to other tablet commands while I'm here
e9f08e1 
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188
[vtctl] Swap out old implementation for new, transparently to callers
96c7457 
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 force-pushed the am_vtctldserver_refresh_state_rpcs branch 2 times, most recently from fcbdc73 to 96c7457 Compare June 14, 2021 12:15
@ajm188 ajm188 merged commit 5374002 into vitessio:main Jun 14, 2021
Vtctld Service automation moved this from In progress to Done Jun 14, 2021
@ajm188 ajm188 deleted the am_vtctldserver_refresh_state_rpcs branch June 14, 2021 20:32
ajm188 added a commit to tinyspeck/vitess that referenced this pull request Jul 23, 2021
@ajm188
Merge pull request vitessio#8232 from tinyspeck/am_vtctldserver_refre…
9b12dec 
…sh_state_rpcs

[vtctldserver] Add RefreshState RPCs

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 mentioned this pull request Jul 23, 2021
Closed
8 tasks
@vmogilev vmogilev mentioned this pull request May 23, 2022
Open
33 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doeg doeg doeg approved these changes

@rohit-nayak-ps rohit-nayak-ps rohit-nayak-ps approved these changes

@deepthi deepthi Awaiting requested review from deepthi deepthi is a code owner

@rafael rafael Awaiting requested review from rafael

Assignees
No one assigned
Labels
Component: Cluster management Type: Feature Request
Projects
Vtctld Service
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ajm188 @doeg @rohit-nayak-ps