feat(updater): enhance scaling and update logic for TiCDC instances#6488
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances TiCDC scaling and update logic by implementing maxSurge support similar to TiDB, allowing for more efficient rolling updates. The change introduces intelligent update strategies that can perform rolling updates with zero downtime when configuration changes are reloadable, or fall back to restart-based updates when necessary.
Key changes:
- Added precheck logic to determine if TiCDC instances need updates and whether they require restarts
- Implemented dynamic maxSurge/maxUnavailable configuration based on update type
- Added comprehensive E2E tests for TiCDC scaling and simultaneous scale/update operations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/controllers/ticdcgroup/tasks/updater.go | Enhanced updater logic with precheck functionality and dynamic surge/unavailable configuration |
| tests/e2e/ticdc/ticdc.go | Added new E2E test cases for TiCDC scaling and simultaneous operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| maxSurge, maxUnavailable := 0, 1 | ||
| noUpdate := false | ||
| if needRestart { | ||
| maxSurge, maxUnavailable = 1, 0 | ||
| noUpdate = true | ||
| } |
There was a problem hiding this comment.
[nitpick] Consider extracting the surge/unavailable configuration logic into a separate function like calculateUpdateStrategy(needRestart bool) (maxSurge, maxUnavailable int, noUpdate bool) to improve readability and make the strategy more explicit.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/v2 #6488 +/- ##
==============================================
+ Coverage 43.12% 43.15% +0.02%
==============================================
Files 345 345
Lines 23855 23878 +23
==============================================
+ Hits 10287 10304 +17
- Misses 13568 13574 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
/lgtm |
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liubog2008 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
What problem does this PR solve?
Similar to TiDB, set maxSurge by default for TiCDC.
What is changed and how does it work?
Code changes
Tests
Side effects
Related changes
Release Notes
Please refer to Release Notes Language Style Guide before writing the release note.