search

tikv / pd

Placement driver for TiKV
Apache License 2.0
1.05k stars 720 forks source link

why `saveNewKeyspace()` locks `keyspace.Id,`not `keyspace.Name`? #8206

Open KinWaiYuen opened 4 months ago

KinWaiYuen commented 4 months ago

https://github.com/tikv/pd/blob/2fabb74157938ced112b1d768cb8d4b995653275/pkg/keyspace/keyspace.go#L271C2-L271C36

if several clients call CreateKeyspace, function saveNewKeyspace locks by Id which is created by manager.allocID(), etcd transaction failed, conflicted and rolled back error may be happened as Ids are allocated saperately

ti-chi-bot[bot] commented 4 months ago

There are no type labels on this issue. Please add an appropriate label by using the following command:

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
HuSharp commented 4 months ago

@AmoebaProtozoa Hi, can you help to provide an answer to this question? :)

AmoebaProtozoa commented 4 months ago

It mostly comes down to uniqueness and consistency.

For uniqueness, the keyspaceID is system generated and guaranteed to be unique and immutable across a keyspace's life cycle. Latching it to user provided name seems kinda risky, especially if we were to implement keyspace renaming/aliasing in the future. For consistency, all other keyspace operations uses ID as lock, so the saveNewKeyspace() follows suite.