Skip to content

Commit

Permalink
refactor: Rename the field called index to name in VitessShardTabletPool
Browse files Browse the repository at this point in the history
Signed-off-by: Yohei Yoshimuta <[email protected]>
  • Loading branch information
yoheimuta committed Jun 21, 2023
1 parent 54ce07d commit a042c49
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 73 deletions.
20 changes: 8 additions & 12 deletions deploy/crds/planetscale.com_vitessclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1461,11 +1461,6 @@ spec:
type: array
extraVolumes:
x-kubernetes-preserve-unknown-fields: true
index:
default: 0
format: int32
minimum: 0
type: integer
initContainers:
x-kubernetes-preserve-unknown-fields: true
mysqld:
Expand Down Expand Up @@ -1506,6 +1501,9 @@ spec:
required:
- resources
type: object
name:
default: ""
type: string
replicas:
format: int32
minimum: 0
Expand Down Expand Up @@ -1576,7 +1574,7 @@ spec:
x-kubernetes-list-map-keys:
- type
- cell
- index
- name
x-kubernetes-list-type: map
required:
- databaseInitScriptSecret
Expand Down Expand Up @@ -1862,11 +1860,6 @@ spec:
type: array
extraVolumes:
x-kubernetes-preserve-unknown-fields: true
index:
default: 0
format: int32
minimum: 0
type: integer
initContainers:
x-kubernetes-preserve-unknown-fields: true
mysqld:
Expand Down Expand Up @@ -1907,6 +1900,9 @@ spec:
required:
- resources
type: object
name:
default: ""
type: string
replicas:
format: int32
minimum: 0
Expand Down Expand Up @@ -1977,7 +1973,7 @@ spec:
x-kubernetes-list-map-keys:
- type
- cell
- index
- name
x-kubernetes-list-type: map
required:
- databaseInitScriptSecret
Expand Down
20 changes: 8 additions & 12 deletions deploy/crds/planetscale.com_vitesskeyspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -509,11 +509,6 @@ spec:
type: array
extraVolumes:
x-kubernetes-preserve-unknown-fields: true
index:
default: 0
format: int32
minimum: 0
type: integer
initContainers:
x-kubernetes-preserve-unknown-fields: true
mysqld:
Expand Down Expand Up @@ -554,6 +549,9 @@ spec:
required:
- resources
type: object
name:
default: ""
type: string
replicas:
format: int32
minimum: 0
Expand Down Expand Up @@ -624,7 +622,7 @@ spec:
x-kubernetes-list-map-keys:
- type
- cell
- index
- name
x-kubernetes-list-type: map
required:
- databaseInitScriptSecret
Expand Down Expand Up @@ -910,11 +908,6 @@ spec:
type: array
extraVolumes:
x-kubernetes-preserve-unknown-fields: true
index:
default: 0
format: int32
minimum: 0
type: integer
initContainers:
x-kubernetes-preserve-unknown-fields: true
mysqld:
Expand Down Expand Up @@ -955,6 +948,9 @@ spec:
required:
- resources
type: object
name:
default: ""
type: string
replicas:
format: int32
minimum: 0
Expand Down Expand Up @@ -1025,7 +1021,7 @@ spec:
x-kubernetes-list-map-keys:
- type
- cell
- index
- name
x-kubernetes-list-type: map
required:
- databaseInitScriptSecret
Expand Down
10 changes: 4 additions & 6 deletions deploy/crds/planetscale.com_vitessshards.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -492,11 +492,6 @@ spec:
type: array
extraVolumes:
x-kubernetes-preserve-unknown-fields: true
index:
default: 0
format: int32
minimum: 0
type: integer
initContainers:
x-kubernetes-preserve-unknown-fields: true
mysqld:
Expand Down Expand Up @@ -537,6 +532,9 @@ spec:
required:
- resources
type: object
name:
default: ""
type: string
replicas:
format: int32
minimum: 0
Expand Down Expand Up @@ -607,7 +605,7 @@ spec:
x-kubernetes-list-map-keys:
- type
- cell
- index
- name
x-kubernetes-list-type: map
topologyReconciliation:
properties:
Expand Down
12 changes: 6 additions & 6 deletions docs/api/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -7066,15 +7066,15 @@ <h3 id="planetscale.com/v2.VitessShardTabletPool">VitessShardTabletPool
</tr>
<tr>
<td>
<code>index</code></br>
<code>name</code></br>
<em>
int32
string
</em>
</td>
<td>
<p>Index is the pool&rsquo;s index within the (cell,type) pair.
This field is optional, and defaults to 0.
Assigning different numbers to this field enables the existence of multiple pools with a specific tablet type in a given cell,
<p>Name is the pool&rsquo;s unique name within the (cell,type) pair.
This field is optional, and defaults to an empty.
Assigning different names to this field enables the existence of multiple pools with a specific tablet type in a given cell,
which can be beneficial for unmanaged tablets.</p>
</td>
</tr>
Expand Down Expand Up @@ -7343,7 +7343,7 @@ <h3 id="planetscale.com/v2.VitessShardTemplate">VitessShardTemplate
<td>
<p>TabletPools specify groups of tablets in a given cell with a certain
tablet type and a shared configuration template.</p>
<p>There must be at most one pool in this list for each (cell,type,index) set.
<p>There must be at most one pool in this list for each (cell,type,name) set.
Each shard must have at least one &ldquo;replica&rdquo; pool (in at least one cell)
in order to be able to serve.</p>
</td>
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/planetscale/v2/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ const (
TabletUidLabel = LabelPrefix + "/" + "tablet-uid"
// TabletTypeLabel is the key for identifying the Vitess target tablet type for a Pod.
TabletTypeLabel = LabelPrefix + "/" + "tablet-type"
// TabletTypeLabel is the key for identifying the Vitess target pool index within the (cell,type) pair.
TabletPoolIndexLabel = LabelPrefix + "/" + "pool-index"
// TabletPoolNameLabel is the key for identifying the Vitess target pool name within the (cell,type) pair.
TabletPoolNameLabel = LabelPrefix + "/" + "pool-name"
// TabletIndexLabel is the key for identifying the index of a Vitess tablet within its pool.
TabletIndexLabel = LabelPrefix + "/" + "tablet-index"

Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/planetscale/v2/vitessshard_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ func (t *VitessTabletPoolType) InitTabletType() string {
}
}

// IsMatch indicates whether a tablet pool matches another tablet pool's type, cell and index.
// IsMatch indicates whether a tablet pool matches another tablet pool's type, cell and name.
func (t *VitessShardTabletPool) IsMatch(inputPool *VitessShardTabletPool) bool {
return t.Type == inputPool.Type && t.Cell == inputPool.Cell && t.Index == inputPool.Index
return t.Type == inputPool.Type && t.Cell == inputPool.Cell && t.Name == inputPool.Name
}

// UsingExternalDatastore indicates whether the VitessShard Spec is using
Expand Down
15 changes: 7 additions & 8 deletions pkg/apis/planetscale/v2/vitessshard_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,15 @@ type VitessShardTemplate struct {
// TabletPools specify groups of tablets in a given cell with a certain
// tablet type and a shared configuration template.
//
// There must be at most one pool in this list for each (cell,type,index) set.
// There must be at most one pool in this list for each (cell,type,name) set.
// Each shard must have at least one "replica" pool (in at least one cell)
// in order to be able to serve.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
// +listMapKey=cell
// +listMapKey=index
// +listMapKey=name
TabletPools []VitessShardTabletPool `json:"tabletPools,omitempty" patchStrategy:"merge" patchMergeKey:"type"`

// DatabaseInitScriptSecret specifies the init_db.sql script file to use for this shard.
Expand Down Expand Up @@ -171,13 +171,12 @@ type VitessShardTabletPool struct {
// +kubebuilder:validation:Enum=replica;rdonly;externalmaster;externalreplica;externalrdonly
Type VitessTabletPoolType `json:"type"`

// Index is the pool's index within the (cell,type) pair.
// This field is optional, and defaults to 0.
// Assigning different numbers to this field enables the existence of multiple pools with a specific tablet type in a given cell,
// Name is the pool's unique name within the (cell,type) pair.
// This field is optional, and defaults to an empty.
// Assigning different names to this field enables the existence of multiple pools with a specific tablet type in a given cell,
// which can be beneficial for unmanaged tablets.
// +kubebuilder:default=0
// +kubebuilder:validation:Minimum=0
Index int32 `json:"index,omitempty"`
// +kubebuilder:default=""
Name string `json:"name,omitempty"`

// Replicas is the number of tablets to deploy in this pool.
// This field is required, although it may be set to 0,
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/vitessshard/reconcile_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (r *ReconcileVitessShard) claimForTabletPod(ctx context.Context, pod *v1.Po
}

// tabletKeysForPool returns the list of targetKeys for a given pool type and cell.
// Note that this function does not care about the pool's index assignment.
// Note that this function does not care about the pool's name assignment.
func tabletKeysForPool(vts *planetscalev2.VitessShard, poolCell string, poolType planetscalev2.VitessTabletPoolType) ([]string, error) {
tabletKeys := vts.Status.TabletAliases()

Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/vitessshard/reconcile_tablets.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,9 @@ func vttabletSpecs(vts *planetscalev2.VitessShard, parentLabels map[string]strin
Uid: vttablet.UID(pool.Cell, keyspaceName, vts.Spec.KeyRange, pool.Type, uint32(tabletIndex)),
}

// If TabletPools has multiple pools within the same (cell,type) pair, we need to add a pool index to the UID generator.
if 0 < pool.Index {
tabletAlias.Uid = vttablet.UIDWithPoolIndex(pool.Cell, keyspaceName, vts.Spec.KeyRange, pool.Type, uint32(tabletIndex), uint32(pool.Index))
// If TabletPools has multiple pools within the same (cell,type) pair, we need to add a pool name to the UID generator.
if 0 < len(pool.Name) {
tabletAlias.Uid = vttablet.UIDWithPoolName(pool.Cell, keyspaceName, vts.Spec.KeyRange, pool.Type, uint32(tabletIndex), pool.Name)
}

// Copy parent labels map and add tablet-specific labels.
Expand All @@ -297,7 +297,7 @@ func vttabletSpecs(vts *planetscalev2.VitessShard, parentLabels map[string]strin
labels[planetscalev2.CellLabel] = tabletAlias.Cell
labels[planetscalev2.TabletUidLabel] = strconv.FormatUint(uint64(tabletAlias.Uid), 10)
labels[planetscalev2.TabletTypeLabel] = string(pool.Type)
labels[planetscalev2.TabletPoolIndexLabel] = strconv.FormatUint(uint64(pool.Index), 10)
labels[planetscalev2.TabletPoolNameLabel] = pool.Name
labels[planetscalev2.TabletIndexLabel] = strconv.FormatUint(uint64(tabletIndex), 10)

// Merge ExtraVitessFlags into the tablet spec ExtraFlags field.
Expand Down
12 changes: 6 additions & 6 deletions pkg/operator/vttablet/uid.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,19 @@ func UID(cellName, keyspaceName string, shardKeyRange planetscalev2.VitessKeyRan
}

/*
UIDWithPoolIndex function generates a 32-bit unsigned integer similar to the UID function above.
UIDWithPoolName function generates a 32-bit unsigned integer similar to the UID function above.
However, it additionally takes the poolIndex as an input.
However, it additionally takes the poolName as an input.
This allows the generation of a unique UID for a tablet that belongs to a different pool
but shares other common attributes.
To preserve the existing UID, it is recommended to use the UID function instead of this function
when the poolIndex is set to its default value of 0.
when the poolName is set to its default value of an empty string.
*/
func UIDWithPoolIndex(cellName, keyspaceName string, shardKeyRange planetscalev2.VitessKeyRange,
tabletPoolType planetscalev2.VitessTabletPoolType, tabletIndex uint32, poolIndex uint32) uint32 {
func UIDWithPoolName(cellName, keyspaceName string, shardKeyRange planetscalev2.VitessKeyRange,
tabletPoolType planetscalev2.VitessTabletPoolType, tabletName uint32, poolName string) uint32 {
h := md5.New()
fmt.Fprintln(h, cellName, keyspaceName, shardKeyRange.String(), string(tabletPoolType), tabletIndex, poolIndex)
fmt.Fprintln(h, cellName, keyspaceName, shardKeyRange.String(), string(tabletPoolType), tabletName, poolName)
sum := h.Sum(nil)
return binary.BigEndian.Uint32(sum[:4])
}
14 changes: 7 additions & 7 deletions pkg/operator/vttablet/uid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,20 @@ func TestUIDHash(t *testing.T) {
}
}

// TestUIDWithPoolIndexHash checks that nobody changed the hash function for UIDWithPoolIndex().
func TestUIDWithPoolIndexHash(t *testing.T) {
// TestUIDWithPoolNameHash checks that nobody changed the hash function for UIDWithPoolName().
func TestUIDWithPoolNameHash(t *testing.T) {
cell := "cell"
keyspace := "keyspace"
keyRange := planetscalev2.VitessKeyRange{Start: "10", End: "20"}
tabletType := planetscalev2.ReplicaPoolType
tabletIndex := uint32(1)
poolIndex := uint32(1)
tabletName := uint32(1)
poolName := "unmanaged-replica-1"

// DO NOT CHANGE THIS VALUE!
// This is intentionally a change-detection test. If it breaks, you messed up.
want := uint32(3840445776)
want := uint32(6333720)

if got := UIDWithPoolIndex(cell, keyspace, keyRange, tabletType, tabletIndex, poolIndex); got != want {
t.Fatalf("UIDWithPoolIndex() = %v, want %v", got, want)
if got := UIDWithPoolName(cell, keyspace, keyRange, tabletType, tabletName, poolName); got != want {
t.Fatalf("UIDWithPoolName() = %v, want %v", got, want)
}
}
14 changes: 7 additions & 7 deletions test/integration/vitesscluster/vitesscluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ spec:
storage: 1Gi
- cell: cell2
type: rdonly
index: 1
name: unmanaged-replica-2
replicas: 3
mysqld: {}
dataVolumeClaimTemplate:
Expand Down Expand Up @@ -202,19 +202,19 @@ func verifyBasicVitessShard(f *framework.Fixture, ns, cluster, keyspace, shard s
// VitessShard creates vttablet Pods.
cell1Pods := f.ExpectPods(&client.ListOptions{
Namespace: ns,
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell1", "replica", "0"),
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell1", "replica", ""),
}, expectedTabletCount[0])
cell2Pods := f.ExpectPods(&client.ListOptions{
Namespace: ns,
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell2", "rdonly", "0"),
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell2", "rdonly", ""),
}, expectedTabletCount[1])
cell2_1_Pods := f.ExpectPods(&client.ListOptions{
Namespace: ns,
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell2", "rdonly", "1"),
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell2", "rdonly", "unmanaged-replica-2"),
}, expectedTabletCount[2])
cell3Pods := f.ExpectPods(&client.ListOptions{
Namespace: ns,
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell3", "replica", "0"),
LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell3", "replica", ""),
}, expectedTabletCount[3])

// Each vttablet Pod should have a PVC.
Expand All @@ -236,7 +236,7 @@ func verifyBasicVitessShard(f *framework.Fixture, ns, cluster, keyspace, shard s
f.MustGet(ns, names.JoinWithConstraints(names.DefaultConstraints, cluster, keyspace, shard, "vtbackup", "init"), &corev1.PersistentVolumeClaim{})
}

func tabletPodSelector(cluster, keyspace, shard, cell, tabletType, poolIndex string) apilabels.Selector {
func tabletPodSelector(cluster, keyspace, shard, cell, tabletType, poolName string) apilabels.Selector {
// This intentionally does NOT use any shared constants because we want the
// test to fail if the labels change, since that's a breaking change.
return apilabels.Set{
Expand All @@ -245,6 +245,6 @@ func tabletPodSelector(cluster, keyspace, shard, cell, tabletType, poolIndex str
"planetscale.com/shard": shard,
"planetscale.com/cell": cell,
"planetscale.com/tablet-type": tabletType,
"planetscale.com/pool-index": poolIndex,
"planetscale.com/pool-name": poolName,
}.AsSelector()
}

0 comments on commit a042c49

Please sign in to comment.