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

[vtadmin] create+delete shards #9012

Merged
merged 8 commits into from
Oct 26, 2021

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Oct 18, 2021

Description

This adds CreateShard and DeleteShards api endpoints to vtadmin. It also enhances (in a gross way) the testutil package to allow us to exercise code from the cluster down through the grpcvtctldserver (rather than stopping at the client interface boundary as we have up to this point), using the in-process localvtctldclient added in #8882

A brief demonstration

❯ curl -X POST -d '{"keyspace": "testks", "shard_name": "-", "include_parent": true}' localhost:14200/api/shards/local
{"result":{"keyspace":{"name":"testks","keyspace":{}},"shard":{"keyspace":"testks","name":"-","shard":{"key_range":{},"is_primary_serving":true}}},"ok":true}%                                                                                                                                  
❯ curl localhost:14200/api/keyspace/local/testks
{"result":{"cluster":{"id":"local","name":"local"},"keyspace":{"name":"testks","keyspace":{}},"shards":{"-":{"keyspace":"testks","name":"-","shard":{"key_range":{},"is_primary_serving":true}}}},"ok":true}%                                                                                   
❯ curl -X DELETE localhost:14200/api/shards/local\?keyspace_shard="testks/-"
{"result":{},"ok":true}%                                                                                                                        
❯ curl localhost:14200/api/keyspace/local/testks
{"result":{"cluster":{"id":"local","name":"local"},"keyspace":{"name":"testks","keyspace":{}}},"ok":true}% 

Related Issue(s)

#8732
#8740

Checklist

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

Deployment Notes

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.

If you get a chance, can you merge in main + push it up? I want to make sure it'll run the new check from #8993. (Kinda funny that it has to be on the branch to run, but I guess I get it!)

@@ -260,6 +262,29 @@ func (api *API) CreateKeyspace(ctx context.Context, req *vtadminpb.CreateKeyspac
}, nil
}

// CreateShard is part of the vtadminpb.VTAdminServver interface.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Servver

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 extra v is for .... vtadmin (thanks 😅 )

shards[i] = fmt.Sprintf("%s/%s", shard.Keyspace, shard.Name)
}

sort.Strings(shards)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a need to check for duplicate shards either here or in grpcvtctldclient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can de-dupe here, but it shouldn't matter because it's safe to delete the same shard over and over

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ajm188
Copy link
Contributor Author

ajm188 commented Oct 22, 2021

If you get a chance, can you merge in main + push it up?

sorry, just got back to this, doing it now!!

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>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
… tests

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>
@ajm188 ajm188 force-pushed the am_vtadmin_create_delete_shards branch from 5e1887d to 406bb1b Compare October 22, 2021 15:03
@ajm188 ajm188 merged commit 8fe3fe3 into vitessio:main Oct 26, 2021
@ajm188 ajm188 deleted the am_vtadmin_create_delete_shards branch October 26, 2021 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants