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.

Sign up for GitHub

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

Jump to bottom

Sync TiCDC after TiDB #4171

Merged
merged 10 commits into from
Sep 3, 2021
Merged

Sync TiCDC after TiDB #4171

merged 10 commits into from
Sep 3, 2021

Conversation

KanShiori
Copy link
Collaborator

@KanShiori KanShiori commented Sep 1, 2021 •
edited
Loading

What problem does this PR solve?

Close #4167

What is changed and how does it work?

  • Sync TiCDC after TiDB
  • Update TiCDC's status when TiDBCluster is paused

Code changes

  • Has Go code change
  • Has CI related scripts change

Tests

  • Unit test
  • E2E test
  • Manual test

EKS 1.21.2

  • Deploy TiDBCluster v5.0.3, then upgrade to v5.2.0.
    1. PD rolling upgrade
    2. TiFlash rolling upgrade
    3. TiKV rolling upgrade
    4. Pump rolling upgrade
    5. TiDB rolling upgrade
    6. TiCDC rolling upgrade
  • No code

Side effects

  • Breaking backward compatibility
  • Other side effects:

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Release Notes

Please refer to Release Notes Language Style Guide before writing the release note.

Sync TiCDC after TiDB to fix TiCDC v5.2.0 upgrade failure

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 1, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • DanielZhangQD
  • csuzhangxc

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

KanShiori
KanShiori commented Sep 1, 2021
View reviewed changes
pkg/manager/member/pump_member_manager.go
@@ -114,14 +114,13 @@ func (m *pumpMemberManager) syncPumpStatefulSetForTidbCluster(tc *v1alpha1.TidbC
}

// Wait for PD & TiKV upgrading done
if tc.Status.TiCDC.Phase == v1alpha1.UpgradePhase ||
tc.Status.TiFlash.Phase == v1alpha1.UpgradePhase ||
if tc.Status.TiFlash.Phase == v1alpha1.UpgradePhase ||
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we need general code to check whether a component can upgrade 🤔

@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2021 •
edited
Loading

Codecov Report

Merging #4171 (8d45649) into master (161478a) will increase coverage by 9.15%.
The diff coverage is 80.95%.

@@            Coverage Diff             @@
##           master    #4171      +/-   ##
==========================================
+ Coverage   61.27%   70.43%   +9.15%     
==========================================
  Files         178      182       +4     
  Lines       18978    21287    +2309     
==========================================
+ Hits        11629    14993    +3364     
+ Misses       6206     5130    -1076     
- Partials     1143     1164      +21
Flag Coverage Δ
e2e 57.85% <73.80%> (?)
unittest 61.28% <69.04%> (+<0.01%) ⬆️

KanShiori
KanShiori commented Sep 1, 2021
View reviewed changes
pkg/manager/member/ticdc_upgrader.go
tc.Status.TiFlash.Phase == v1alpha1.UpgradePhase ||
tc.Status.Pump.Phase == v1alpha1.UpgradePhase ||
tc.Status.TiDB.Phase == v1alpha1.UpgradePhase {
klog.Infof("TidbCluster: [%s/%s]'s pd status is %s, "+
Copy link
Collaborator Author

@KanShiori KanShiori Sep 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Why couldn't the phase of TiCDC become ScalePhase?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just because no need to care about the scale phase during upgrading, but for PD, TiKV, TiFlash we'd better avoid scaling and upgrading at the same time because it may cause unexpected behavior.

@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

DanielZhangQD
DanielZhangQD approved these changes Sep 1, 2021
View reviewed changes
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

@csuzhangxc
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 546c9f7

@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

@KanShiori KanShiori mentioned this pull request Sep 1, 2021
Closed
9 tasks
@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

1 similar comment
@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

@Mini256 Mini256 mentioned this pull request Sep 2, 2021
Closed
1 task
@KanShiori
Copy link
Collaborator Author

/test pull-e2e-kind

@KanShiori
Copy link
Collaborator Author

/test pull-e2e-kind

@DanielZhangQD
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 8d45649

@ti-chi-bot ti-chi-bot merged commit d36e6a9 into pingcap:master Sep 3, 2021
ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Sep 3, 2021
@KanShiori @ti-srebot
cherry pick pingcap#4171 to release-1.2
7dea8c3 
Signed-off-by: ti-srebot <[email protected]>
@ti-srebot ti-srebot mentioned this pull request Sep 3, 2021
Merged
10 tasks
@ti-srebot
Copy link
Contributor

cherry pick to release-1.2 in PR #4178

ti-chi-bot pushed a commit that referenced this pull request Sep 3, 2021
@ti-srebot
Sync TiCDC after TiDB (#4171) (#4178)
aac2b9e
@KanShiori KanShiori deleted the sync_ticdc_after_tidb branch September 3, 2021 06:01
ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Sep 7, 2021
@KanShiori @ti-srebot
cherry pick pingcap#4171 to release-1.1
0039259 
Signed-off-by: ti-srebot <[email protected]>
@ti-srebot ti-srebot mentioned this pull request Sep 7, 2021
Closed
10 tasks
@ti-srebot
Copy link
Contributor

cherry pick to release-1.1 in PR #4187

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csuzhangxc csuzhangxc csuzhangxc approved these changes

@DanielZhangQD DanielZhangQD DanielZhangQD approved these changes

@handlerww handlerww Awaiting requested review from handlerww

Assignees
No one assigned
Labels
needs-cherry-pick-1.1 needs-cherry-pick-1.2 status/can-merge status/LGT2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Sync TiCDC after TiDB
6 participants
@KanShiori @ti-chi-bot @codecov-commenter @DanielZhangQD @csuzhangxc @ti-srebot