dm-operator/: support scaling a dm cluster with dm-masters and dm-workers #3186

lichunzhu · 2020-08-31T02:48:30Z

What problem does this PR solve?

#2868
Support scale in/out dmcluster.

What is changed and how does it work?

Check pvc and delete defer deleting pvcs to scale-out dm-master and dm-worker.

Check List

Tests

Unit test
E2E test
Stability test
Manual test (add detailed scripts or steps below)
Test scale in and then scale out dm-master. dm-master can start correctly and pvc will be flushed while dm-worker will not flush pvc.

dm-master-basic-dm-master-0   Bound    pvc-9e42db35-927b-4476-9e1e-691f3e286293   1Gi        RWO            standard       20m
dm-master-basic-dm-master-1   Bound    pvc-d44bd6c4-cc94-4dad-a38a-85b6d3641096   1Gi        RWO            standard       16m
dm-master-basic-dm-master-2   Bound    pvc-a587a4d7-31f1-4075-82b5-8cd9e74f88ba   1Gi        RWO            standard       15m
dm-worker-basic-dm-worker-0   Bound    pvc-d4516dcc-a604-486d-823d-452a78f4482a   1Gi        RWO            standard       19m
dm-worker-basic-dm-worker-1   Bound    pvc-9c2b06d5-4cb9-4bc9-b510-0676eeb183ea   1Gi        RWO            standard       19m
dm-worker-basic-dm-worker-2   Bound    pvc-06215f47-18ad-47f7-9953-d35a45b091ce   1Gi        RWO            standard       19m

Does this PR introduce a user-facing change?:

NONE

…ator into defineDMSpec

…o supportStartDMCluster

codecov-commenter · 2020-08-31T03:41:58Z

Codecov Report

Merging #3186 into master will decrease coverage by 0.47%.
The diff coverage is 22.30%.

@@            Coverage Diff             @@
##           master    #3186      +/-   ##
==========================================
- Coverage   40.90%   40.43%   -0.48%     
==========================================
  Files         168      170       +2     
  Lines       18507    18818     +311     
==========================================
+ Hits         7571     7609      +38     
- Misses      10283    10553     +270     
- Partials      653      656       +3

Flag	Coverage Δ
#unittest	`40.43% <22.30%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

pkg/dmapi/master_control.go

DanielZhangQD · 2020-09-01T06:43:13Z

pkg/manager/manager.go

@@ -31,5 +31,5 @@ type TiKVGroupManager interface {

 type DMManager interface {
 	// Sync implements the logic for syncing dmcluster.
-	Sync(*v1alpha1.DMCluster) error


Why add DM here? I think it's not necessary as it's a method of DMManager which already includes the DM.

Because reclaimPolicyManager uses SyncDM as interface and I want to keep the same with that. I think using Sync is also okay.

pkg/manager/member/dm_master_scaler.go

pkg/manager/member/dm_worker_member_manager.go

pkg/manager/member/pd_scaler.go

…tidb-operator into supportScaleDMCluster

DanielZhangQD · 2020-09-02T01:40:02Z

pkg/manager/member/dm_worker_scaler.go

+	return nil
+}
+
+// We need remove member from cluster before reducing statefulset replicas


where is the member removed?

It will be removed in syncing worker status. For dm-worker we can't remove its register info from dm-master when it's still alive. So we delete it later after it's keepalive lease is outdated or revoked. We can defer deleting dm-worker register info because dm-master will patch replication worker through keepalive info.

I mean the comment here does not match the code logic.

pkg/manager/member/dm_worker_member_manager.go

DanielZhangQD · 2020-09-02T06:15:38Z

pkg/manager/member/dm_worker_member_manager.go

+			ordinal, err := util.GetOrdinalFromPodName(worker.Name)
+			if err != nil {
+				klog.Errorf("invalid worker name %s, can't offline this worker automatically, err: %s", worker.Name, err)
+			} else if ordinal >= dc.WorkerStsDesiredReplicas() {


Please refer to https://github.com/pingcap/tidb-operator/blob/master/pkg/manager/member/tikv_failover.go#L38-L45 for how to check if an ordinal is not desired or not in case of advanced statefulset is enabled.

DanielZhangQD · 2020-09-02T06:26:39Z

pkg/manager/member/dm_worker_scaler.go

+	return nil
+}
+
+// We need remove member from cluster before reducing statefulset replicas


I mean the comment here does not match the code logic.

DanielZhangQD · 2020-09-02T07:14:55Z

pkg/manager/member/dm_worker_member_manager.go

@@ -549,3 +546,13 @@ func getWorkerConfigMap(dc *v1alpha1.DMCluster) (*corev1.ConfigMap, error) {
 	}
 	return cm, nil
 }
+
+func isWorkerPodDesired(dc *v1alpha1.DMCluster, podName string) bool {
+	ordinals := dc.WorkerStsDesiredOrdinals(true)


Should be false here?

…tidb-operator into supportScaleDMCluster

DanielZhangQD

LGTM

lichunzhu added 30 commits August 5, 2020 14:36

add dmclusters CRD

08a8ae4

resolve conflicts

5cb82ac

Merge branch 'master' into defineDMSpec

bcf58ea

address comment

55a6365

Merge branch 'master' into defineDMSpec

0aae8c0

address comments

f146be1

Merge branch 'defineDMSpec' of https://github.com/lichunzhu/tidb-oper…

94c427b

…ator into defineDMSpec

delete monitor ref

4f10a51

generate dmcluster client

a020492

address comments

3edfaa2

Merge branch 'master' into defineDMSpec

74aca39

address comment

2c1bec5

tmp commit

fbe26f3

resolve conflict

d85a9fc

merge master

a9da15f

remove dm package

ba0f518

fix bugs

7a51c07

fix bug

3ec6c86

support start dm-master and dm-worker in cluster

8122c71

fix some bugs

a38bb0e

merge master branch and resolve conflicts

1e7efd8

fix ut

850035c

fix dm-master start

cd3f5b4

fix dm-worker start bug

f4a641d

Merge branch 'master' of https://github.com/pingcap/tidb-operator int…

5a51cbe

…o supportStartDMCluster

add more column info

be4fd5e

Merge branch 'master' of https://github.com/pingcap/tidb-operator int…

1e4f2ac

…o supportStartDMCluster

Merge branch 'master' into supportStartDMCluster

daf81f7

fix ut

f7c70bd

fix verify

08f4695

lichunzhu added 2 commits August 31, 2020 10:56

let scaling take precedence over upgrading

bf0db90

fix ut

f6e5a55

lichunzhu requested review from DanielZhangQD and cofyc August 31, 2020 03:34

lichunzhu added the status/PTAL PR needs to be reviewed label Aug 31, 2020

Merge branch 'master' into supportScaleDMCluster

e9e6032

DanielZhangQD reviewed Sep 1, 2020

View reviewed changes

pkg/manager/member/pd_scaler.go Show resolved Hide resolved

lichunzhu added 3 commits September 1, 2020 15:24

Merge branch 'master' into supportScaleDMCluster

38db6b3

address comments

57803d5

Merge branch 'supportScaleDMCluster' of https://github.com/lichunzhu/…

aa4903b

…tidb-operator into supportScaleDMCluster

DanielZhangQD reviewed Sep 2, 2020

View reviewed changes

Merge branch 'master' into supportScaleDMCluster

57e925d

lichunzhu mentioned this pull request Sep 2, 2020

dm-operator/: support auto-failover for dm-masters and dm-workers #3201

Merged

DanielZhangQD reviewed Sep 2, 2020

View reviewed changes

lichunzhu added 3 commits September 2, 2020 14:44

address comment

4b2deec

address comment

979fd98

Merge branch 'master' into supportScaleDMCluster

ca4048c

DanielZhangQD reviewed Sep 2, 2020

View reviewed changes

lichunzhu added 2 commits September 2, 2020 15:19

address comment

ead903f

Merge branch 'supportScaleDMCluster' of https://github.com/lichunzhu/…

12d8cba

…tidb-operator into supportScaleDMCluster

DanielZhangQD approved these changes Sep 2, 2020

View reviewed changes

ti-srebot added the status/LGT1 label Sep 2, 2020

cofyc approved these changes Sep 2, 2020

View reviewed changes

ti-srebot removed the status/LGT1 label Sep 2, 2020

ti-srebot approved these changes Sep 2, 2020

View reviewed changes

ti-srebot added the status/LGT2 label Sep 2, 2020

lichunzhu merged commit 662b037 into pingcap:master Sep 2, 2020

lichunzhu deleted the supportScaleDMCluster branch September 2, 2020 09:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dm-operator/: support scaling a dm cluster with dm-masters and dm-workers #3186

dm-operator/: support scaling a dm cluster with dm-masters and dm-workers #3186

lichunzhu commented Aug 31, 2020 •

edited

Loading

codecov-commenter commented Aug 31, 2020 •

edited

Loading

DanielZhangQD Sep 1, 2020

lichunzhu Sep 1, 2020

DanielZhangQD Sep 2, 2020

lichunzhu Sep 2, 2020

DanielZhangQD Sep 2, 2020

DanielZhangQD Sep 2, 2020

DanielZhangQD Sep 2, 2020

DanielZhangQD Sep 2, 2020

DanielZhangQD left a comment

dm-operator/: support scaling a dm cluster with dm-masters and dm-workers #3186

dm-operator/: support scaling a dm cluster with dm-masters and dm-workers #3186

Conversation

lichunzhu commented Aug 31, 2020 • 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?:

codecov-commenter commented Aug 31, 2020 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielZhangQD left a comment

Choose a reason for hiding this comment

lichunzhu commented Aug 31, 2020 •

edited

Loading

codecov-commenter commented Aug 31, 2020 •

edited

Loading