Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syncer: Optimize the logic of updating pod. #503

Merged
merged 4 commits into from
Jun 15, 2022

Conversation

runkecheng
Copy link
Collaborator

@runkecheng runkecheng commented May 27, 2022

What type of PR is this?

/bug
/enhancement

Which issue(s) this PR fixes?

Fixes #502 #310

What this PR does?

Summary:

  • bug fix: Update too many pods at the same time.

After deleting the pod, wait for the pod to restart.

  • improvement: Optimize the logic of updatepod().

Use updatedReplicas and Spec.replicas to judge if the updatepod() is needed.

  • improvement: switch leader to updated pod.

Special notes for your reviewer?

@runkecheng runkecheng added bug Something isn't working Improvement labels May 27, 2022
@runkecheng runkecheng added this to the Next milestone May 27, 2022
@runkecheng runkecheng requested review from acekingke and andyli029 May 27, 2022 08:43
@runkecheng runkecheng self-assigned this May 27, 2022
@runkecheng runkecheng force-pushed the fix_update_pod branch 2 times, most recently from 43be7d9 to dc2e03e Compare May 31, 2022 02:52
@andyli029
Copy link
Contributor

andyli029 commented May 31, 2022

Is related to: kubernetes/kubernetes#106059

@andyli029
Copy link
Contributor

  1. Do you want to split several pr

@radondb radondb deleted a comment from runkecheng May 31, 2022
@runkecheng runkecheng changed the title *: Optimize update logic and reduce invalid logs. syncer: Optimize the logic of updating pod. Jun 2, 2022
@runkecheng runkecheng force-pushed the fix_update_pod branch 5 times, most recently from 2e0c3c7 to d0c94e0 Compare June 2, 2022 03:02
@runkecheng runkecheng requested a review from zhl003 June 10, 2022 03:28
updatedRevision wiil not update with the currentRevision when using `onDelete`.
Use sfs.status.UpdatedRplicas and sfs.spec.Replicas to determine whether the update is
completed.
Copy link
Collaborator

@zhl003 zhl003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@acekingke acekingke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -298,7 +298,9 @@ func (s *StatefulSetSyncer) createOrUpdate(ctx context.Context) (controllerutil.
// updatePod update the pods, update follower nodes first.
// This can reduce the number of master-slave switching during the update process.
func (s *StatefulSetSyncer) updatePod(ctx context.Context) error {
if s.sfs.Status.UpdateRevision == s.sfs.Status.CurrentRevision {
// updatedRevision wiil not update with the currentRevision when using `onDelete`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wiil -> will

mysqlcluster/syncer/statefulset.go Outdated Show resolved Hide resolved
@zhl003 zhl003 modified the milestones: Next, v2.2.0 Jun 15, 2022
@runkecheng runkecheng merged commit 63cc34e into radondb:main Jun 15, 2022
zhl003 pushed a commit to zhl003/radondb-mysql-kubernetes that referenced this pull request Aug 17, 2022
* fix(cluster): update too many pods at the same time.

* fix(cluster): adjust the exit logic of updatePod().

updatedRevision wiil not update with the currentRevision when using `onDelete`.
Use sfs.status.UpdatedRplicas and sfs.spec.Replicas to determine whether the update is
completed.

* feat(cluster): actively switch leader to updated pod.

* fix(cluster): wait pod healthy when updating
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] Version conflicts when updating the status.
4 participants