Add AutoScalerRef in TidbCluster Status #2791
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Yisaer
commented
Jun 22, 2020
Comment on lines
-120
to
-131
| // AnnTiDBConsecutiveScaleOutCount describes the least consecutive count to scale-out for tidb | ||
| AnnTiDBConsecutiveScaleOutCount = "tidb.tidb.pingcap.com/consecutive-scale-out-count" | ||
| // AnnTiDBConsecutiveScaleInCount describes the least consecutive count to scale-in for tidb | ||
| AnnTiDBConsecutiveScaleInCount = "tidb.tidb.pingcap.com/consecutive-scale-in-count" | ||
| // AnnTiKVConsecutiveScaleOutCount describes the least consecutive count to scale-out for tikv | ||
| AnnTiKVConsecutiveScaleOutCount = "tikv.tidb.pingcap.com/consecutive-scale-out-count" | ||
| // AnnTiKVConsecutiveScaleInCount describes the least consecutive count to scale-in for tikv | ||
| AnnTiKVConsecutiveScaleInCount = "tikv.tidb.pingcap.com/consecutive-scale-in-count" | ||
| // AnnAutoScalingTargetName describes the target TidbCluster Ref Name for the TidbCluserAutoScaler | ||
| AnnAutoScalingTargetName = "auto-scaling.tidb.pingcap.com/target-name" | ||
| // AnnAutoScalingTargetNamespace describes the target TidbCluster Ref Namespace for the TidbCluserAutoScaler | ||
| AnnAutoScalingTargetNamespace = "auto-scaling.tidb.pingcap.com/target-namespace" |
Comment on lines
-138
to
-152
|
|
||
| // ScaleOutThreshold describe the consecutive threshold for the auto-scaling, | ||
| // if the consecutive counts of the scale-out result in auto-scaling reach this number, | ||
| // the auto-scaling would be performed. | ||
| // If not set, the default value is 3. | ||
| // +optional | ||
| ScaleOutThreshold *int32 `json:"scaleOutThreshold,omitempty"` | ||
|
|
||
| // ScaleInThreshold describe the consecutive threshold for the auto-scaling, | ||
| // if the consecutive counts of the scale-in result in auto-scaling reach this number, | ||
| // the auto-scaling would be performed. | ||
| // If not set, the default value is 5. | ||
| // +optional | ||
| ScaleInThreshold *int32 `json:"scaleInThreshold,omitempty"` | ||
|
|
zjj2wry
reviewed
Jun 22, 2020
| } | ||
| tacNamespace := tc.Status.AutoScaler.Namespace | ||
| tacName := tc.Status.AutoScaler.Name | ||
| _, err := tcsm.cli.PingcapV1alpha1().TidbClusterAutoScalers(tacNamespace).Get(tacName, metav1.GetOptions{}) |
There was a problem hiding this comment.
+1, ever raised the same comment for TidbMonitor here
There was a problem hiding this comment.
updated, and also add the unit test
fix lint fix unit test use lister fix lint use patch instead update fix unit test debug ci fix ci debug ci add log fix ci revise util add default TAC fix ci
Yisaer
force-pushed
the
add_auto_scaler_ref
branch
from
493e3b2
to
dc07bb5
Compare
Yisaer
commented
Jun 23, 2020
Comment on lines
-62
to
-66
| func checkStsAutoScaling(tac *v1alpha1.TidbClusterAutoScaler, thresholdSeconds, intervalSeconds int32, memberType v1alpha1.MemberType) (bool, error) { | ||
| realClock := clockwork.NewRealClock() | ||
| if tac.Annotations == nil { | ||
| tac.Annotations = map[string]string{} | ||
| } |
There was a problem hiding this comment.
This function only used for tikv, move to the checkTiKVStsAutoScaling
Yisaer
commented
Jun 23, 2020
Comment on lines
-254
to
-276
| func resetAutoScalingAnn(tac *v1alpha1.TidbClusterAutoScaler) { | ||
| tac.Annotations[label.AnnAutoScalingTargetNamespace] = tac.Spec.Cluster.Namespace | ||
| tac.Annotations[label.AnnAutoScalingTargetName] = tac.Spec.Cluster.Name | ||
| } | ||
|
|
||
| // checkAndUpdateTacRef would compare the target tidbcluster ref stored in the annotations | ||
| // and in the Spec. It not equal, the previous stored status would be empty and the stored Ref | ||
| // would be updated. | ||
| func checkAndUpdateTacAnn(tac *v1alpha1.TidbClusterAutoScaler) { | ||
| if tac.Annotations == nil { | ||
| tac.Annotations = map[string]string{} | ||
| resetAutoScalingAnn(tac) | ||
| return | ||
| } | ||
| name := tac.Annotations[label.AnnAutoScalingTargetName] | ||
| namespace := tac.Annotations[label.AnnAutoScalingTargetNamespace] | ||
| if name == tac.Spec.Cluster.Name && namespace == tac.Spec.Cluster.Namespace { | ||
| return | ||
| } | ||
| // If not satisfied, reset tac Ann | ||
| resetAutoScalingAnn(tac) | ||
| } | ||
|
|
There was a problem hiding this comment.
use patch reference instead of recording in annotations.
|
/merge |
|
/run-all-tests |
ti-srebot
pushed a commit
to ti-srebot/tidb-operator
that referenced
this pull request
Jun 23, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
|
cherry pick to release-1.1 in PR #2800 |
ti-srebot
added a commit
that referenced
this pull request
Jun 23, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Close #2484
Add Auto-Scaler reference in TidbCluster Status which can avoid avoiding 2 auto-scaler directly take effect to the same tidbcluster.
Related changes
Does this PR introduce a user-facing change?: