-
Notifications
You must be signed in to change notification settings - Fork 219
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
keyspacepb #952
keyspacepb #952
Conversation
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
option java_package = "org.tikv.kvproto"; | ||
|
||
// Keyspace provides services to manage keyspaces. | ||
service Keyspace { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that service is readonly. I wonder how can we create/update/delete keyspaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create/ UpdateState/ UpdateConfig can be done through HTTP API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the client should create a keyspace via HTTP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Target Keyspace should exist prior to client's connection.
A default keyspace with name "DEFAULT"
and id 0
is created when KeyspaceManager initialize, if a client (with keyspace enabled) does not provide a keyspace name, if will connect to default keyspace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
KeyspaceMeta keyspace = 2; | ||
} | ||
|
||
message WatchKeyspacesRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need to watch all key-spaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
/merge |
* added keyspacepb Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
* added keyspacepb Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
ref issue: tikv/pd#5293