able to scale in/out with delete slots feature #1361

cofyc · 2019-12-18T12:59:52Z

What problem does this PR solve?

xref: #1348

What is changed and how does it work?

use advanced-statefulset v0.2.1
able to scale in/out with delete slots feature

Check List

Tests

Unit test
E2E test

Does this PR introduce a user-facing change?:

able to scale in/out with delete slots feature of advanced statefulset

gregwebs · 2019-12-18T18:05:01Z

go.mod

@@ -142,3 +142,5 @@ replace k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.0.0-20
 replace k8s.io/node-api => k8s.io/node-api v0.0.0-20190918163711-2299658ad911

 replace github.com/uber-go/atomic => go.uber.org/atomic v1.5.0
+
+replace github.com/pingcap/advanced-statefulset => /data/go/src/github.com/pingcap/advanced-statefulset


probably don't want to commit this

this is for local development only, I'll remove this

cofyc · 2019-12-19T13:16:34Z

/run-e2e-test

weekface · 2019-12-23T02:57:38Z

manifests/advanced-statefulset-crd.v1beta1.yaml

  scope: Namespaced
  names:
    plural: statefulsets
    singular: statefulset
    kind: StatefulSet
    shortNames:
    - asts
+  versions:


there are already two versions?

cofyc · 2019-12-23T06:47:35Z

/run-e2e-test

cofyc · 2019-12-23T06:54:19Z

tests/actions.go

@@ -393,15 +397,19 @@ func (oi *OperatorConfig) OperatorHelmSetString(m map[string]string) string {
 		"scheduler.kubeSchedulerImageName":           oi.SchedulerImage,
 		"controllerManager.logLevel":                 oi.LogLevel,
 		"scheduler.logLevel":                         "4",
-		"controllerManager.replicas":                 "2",
-		"scheduler.replicas":                         "2",


make it configurable, in some tests, we can use 1 replica to consumes fewer resources

cofyc · 2019-12-23T06:54:55Z

tests/actions.go

-	name := fmt.Sprintf("%s-%d", memberName, 0)
-	pod, err := kubeCli.CoreV1().Pods(namespace).Get(name, metav1.GetOptions{})
+// getMemberContainer gets member container
+func getMemberContainer(kubeCli kubernetes.Interface, namespace, tcName, component string) (*corev1.Container, bool) {


instead of fetching the -0 pod, we list the pods and use the first one.

cofyc · 2019-12-23T06:56:55Z

tests/dt.go

@@ -63,7 +63,7 @@ func (oa *operatorActions) LabelNodes() error {
 	}

 	for i, node := range nodes.Items {
-		err := wait.Poll(3*time.Second, time.Minute, func() (bool, error) {
+		err := wait.PollImmediate(3*time.Second, time.Minute, func() (bool, error) {


no need to wait at the first try, this reduces time

cofyc · 2019-12-23T08:04:47Z

/run-e2e-test

cofyc · 2019-12-23T08:07:53Z

pkg/controller/tidb_control.go

@@ -41,7 +41,7 @@ type DBInfo struct {
 // TiDBControlInterface is the interface that knows how to manage tidb peers
 type TiDBControlInterface interface {
 	// GetHealth returns tidb's health info
-	GetHealth(tc *v1alpha1.TidbCluster) map[string]bool
+	GetHealth(tc *v1alpha1.TidbCluster, ordinal int32) (bool, error)


tidb member controller will decide which member we need to request the health status

cofyc · 2019-12-23T08:19:36Z

pkg/manager/member/scaler.go

+		return nil
+	}
+	// Following cases are not supported yet.
+	// TODO support it or validate in validation phase


some desired states are not allowed because the controller has to delete and create pods. I decided to implement in the future. There are two ways to solve this issue:

avoid in validation phase

support it in multiple scaling operations (e.g. by scaling out first and then scaling in)
-replicas 5, delete slots [0] is changed to replicas 4, delete slots[1], we need to delete pod 1 and 5 and create pod 4.

cofyc · 2019-12-23T08:21:13Z

pkg/features/features.go

+	// from a string like feature1=true,feature2=false,...
+	Set(value string) error
+	// SetFromMap stores flag gates for enabled features from a map[string]bool
+	SetFromMap(m map[string]bool)


able to change features settings in unit tests

cofyc · 2019-12-23T08:24:34Z

pkg/manager/member/pd_scaler.go

+	if err != nil {
+		return err
+	}
+	resetReplicas(newSet, oldSet)


I've updated the scaling logic like this:

get desired replicas and delete slot by comparing the old and new statefulsets

reset the new statefulset

if our scaling logic succeeds, set desired replicas and delete slots, otherwise, return an error

cofyc · 2019-12-23T08:29:06Z

pkg/manager/member/scaler.go

+}
+
+// scaleOne scales actual set to desired set by deleting or creating a replica
+func scaleOne(actual *apps.StatefulSet, desired *apps.StatefulSet) (int32, int32, sets.Int, error) {


this function calculates desired replicas and delete slots from actual/desired stateful sets by allowing one pod to be deleted or created

cofyc · 2019-12-24T08:22:56Z

pkg/manager/member/pd_scaler.go

 func (psd *pdScaler) ScaleOut(tc *v1alpha1.TidbCluster, oldSet *apps.StatefulSet, newSet *apps.StatefulSet) error {
+	_, ordinal, replicas, deleteSlots := scaleOne(oldSet, newSet)


recalculating again because of less code needs to be changed (otherwise ScaleOut/ScaleIn function signatures need to change and a lot of tests must be updated)

cofyc · 2019-12-24T08:38:08Z

pkg/manager/member/tikv_scaler.go

 func (tsd *tikvScaler) ScaleOut(tc *v1alpha1.TidbCluster, oldSet *apps.StatefulSet, newSet *apps.StatefulSet) error {
+	_, ordinal, replicas, deleteSlots := scaleOne(oldSet, newSet)


recalculating again because of less code needs to be changed (otherwise ScaleOut/ScaleIn function signatures need to change and a lot of tests must be updated)

aylei · 2019-12-24T13:41:26Z

/run-e2e-test

cofyc · 2019-12-25T03:28:01Z

/run-e2e-test

aylei

Rest LGTM, the scaleOne() method is excellent

aylei · 2019-12-25T03:50:28Z

pkg/manager/member/scaler.go

+// - scaling:
+//   - 0: no scaling required
+//   - 1: scaling out
+//   - -1: scaling in


Could we define enums for this?

I don't think it's necessary because it's used internally and I actually uses > 0 and < 0 to decide whether to scale in or scale out.

aylei · 2019-12-25T03:53:08Z

pkg/manager/member/scaler_test.go

@@ -158,3 +163,188 @@ func newFakeGeneralScaler() (*generalScaler, cache.Indexer, *controller.FakePVCC
 	return &generalScaler{pvcLister: pvcInformer.Lister(), pvcControl: pvcControl},
 		pvcInformer.Informer().GetIndexer(), pvcControl
 }
+
+func TestScaleOne(t *testing.T) {


Could we add a case for "scale tidb to 0"?

fixed some corner cases and add more tests in c8eb405

aylei · 2019-12-25T03:54:33Z

pkg/manager/member/scaler.go

+	additions := desiredDesiredPodOrdinals.Difference(actualDesiredPodOrdinals)
+	deletions := actualDesiredPodOrdinals.Difference(desiredDesiredPodOrdinals)


Is the order of the diff result guaranteed? or random?

internal struct of sets is a map, List() will sort the set members in ascending order

cofyc · 2019-12-25T11:06:05Z

/run-e2e-test

aylei

LGTM

weekface

well done.

LGTM

cofyc · 2019-12-27T10:54:17Z

/merge

sre-bot · 2019-12-27T10:56:40Z

/run-all-tests

gregwebs reviewed Dec 18, 2019

View reviewed changes

cofyc force-pushed the fix1348 branch 2 times, most recently from 7c30943 to 30169a5 Compare December 19, 2019 13:15

cofyc force-pushed the fix1348 branch 6 times, most recently from 1a007b1 to cd53f6f Compare December 23, 2019 02:51

weekface reviewed Dec 23, 2019

View reviewed changes

cofyc force-pushed the fix1348 branch 3 times, most recently from 08477b9 to 4dae600 Compare December 23, 2019 06:31

cofyc changed the title ~~WIP: able to scale in/out with delete slots feature~~ able to scale in/out with delete slots feature Dec 23, 2019

cofyc force-pushed the fix1348 branch from 4dae600 to 0bb0093 Compare December 23, 2019 06:47

cofyc commented Dec 23, 2019

View reviewed changes

cofyc force-pushed the fix1348 branch 4 times, most recently from 9e60e30 to 20c79ba Compare December 23, 2019 08:03

cofyc commented Dec 23, 2019

View reviewed changes

Merge branch 'master' into fix1348

9fcd1b6

cofyc commented Dec 24, 2019

View reviewed changes

fix pd scaler

7c75bd2

cofyc commented Dec 24, 2019

View reviewed changes

fix go.sum

f970311

cofyc requested a review from Yisaer December 24, 2019 09:06

cofyc added 2 commits December 24, 2019 19:50

use helper.GetPodOrdinals

24988e8

Merge branch 'master' into fix1348

2b82a1e

Merge branch 'master' into fix1348

56b5623

aylei reviewed Dec 25, 2019

View reviewed changes

cofyc added 2 commits December 25, 2019 18:11

fix some corner cases and add more tests

c8eb405

Merge branch 'master' into fix1348

6b9b223

cofyc force-pushed the fix1348 branch 2 times, most recently from 29e6c30 to 4792854 Compare December 25, 2019 11:05

fix

acd31c7

cofyc force-pushed the fix1348 branch from 4792854 to acd31c7 Compare December 25, 2019 11:16

cofyc added 3 commits December 25, 2019 20:56

Merge branch 'master' into fix1348

7d0203f

fix getMemberContainer

1282938

Merge branch 'master' into fix1348

354318e

aylei approved these changes Dec 27, 2019

View reviewed changes

aylei added this to the v1.1.0 milestone Dec 27, 2019

weekface approved these changes Dec 27, 2019

View reviewed changes

sre-bot added the status/can-merge label Dec 27, 2019

Merge branch 'master' into fix1348

eafbfe1

sre-bot merged commit c9b1d8f into pingcap:master Dec 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

able to scale in/out with delete slots feature #1361

able to scale in/out with delete slots feature #1361

cofyc commented Dec 18, 2019 •

edited

Loading

gregwebs Dec 18, 2019

cofyc Dec 19, 2019

cofyc commented Dec 19, 2019

weekface Dec 23, 2019

cofyc commented Dec 23, 2019

cofyc Dec 23, 2019

cofyc Dec 23, 2019 •

edited

Loading

cofyc Dec 23, 2019

cofyc commented Dec 23, 2019

cofyc Dec 23, 2019

cofyc Dec 23, 2019 •

edited

Loading

cofyc Dec 23, 2019

cofyc Dec 23, 2019 •

edited

Loading

cofyc Dec 23, 2019

cofyc Dec 24, 2019

cofyc Dec 24, 2019

aylei commented Dec 24, 2019

cofyc commented Dec 25, 2019

aylei left a comment

aylei Dec 25, 2019

cofyc Dec 25, 2019

aylei Dec 25, 2019

cofyc Dec 25, 2019

aylei Dec 25, 2019

cofyc Dec 25, 2019 •

edited

Loading

cofyc commented Dec 25, 2019

aylei left a comment

weekface left a comment

cofyc commented Dec 27, 2019

sre-bot commented Dec 27, 2019

		func (psd pdScaler) ScaleOut(tc v1alpha1.TidbCluster, oldSet apps.StatefulSet, newSet apps.StatefulSet) error {
		_, ordinal, replicas, deleteSlots := scaleOne(oldSet, newSet)

		func (tsd tikvScaler) ScaleOut(tc v1alpha1.TidbCluster, oldSet apps.StatefulSet, newSet apps.StatefulSet) error {
		_, ordinal, replicas, deleteSlots := scaleOne(oldSet, newSet)

		additions := desiredDesiredPodOrdinals.Difference(actualDesiredPodOrdinals)
		deletions := actualDesiredPodOrdinals.Difference(desiredDesiredPodOrdinals)

able to scale in/out with delete slots feature #1361

able to scale in/out with delete slots feature #1361

Conversation

cofyc commented Dec 18, 2019 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Does this PR introduce a user-facing change?:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cofyc commented Dec 19, 2019

Choose a reason for hiding this comment

cofyc commented Dec 23, 2019

Choose a reason for hiding this comment

cofyc Dec 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cofyc commented Dec 23, 2019

Choose a reason for hiding this comment

cofyc Dec 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cofyc Dec 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aylei commented Dec 24, 2019

cofyc commented Dec 25, 2019

aylei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cofyc Dec 25, 2019 • edited Loading

Choose a reason for hiding this comment

cofyc commented Dec 25, 2019

aylei left a comment

Choose a reason for hiding this comment

weekface left a comment

Choose a reason for hiding this comment

cofyc commented Dec 27, 2019

sre-bot commented Dec 27, 2019

cofyc commented Dec 18, 2019 •

edited

Loading

cofyc Dec 23, 2019 •

edited

Loading

cofyc Dec 23, 2019 •

edited

Loading

cofyc Dec 23, 2019 •

edited

Loading

cofyc Dec 25, 2019 •

edited

Loading