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

Upgrade TiCDC before TiKV and PD when cluster is equal or greater than v5.1.0 #2253

Merged
merged 5 commits into from
Aug 21, 2023

Conversation

KanShiori
Copy link
Collaborator

@KanShiori KanShiori commented Aug 17, 2023

What problem does this PR solve?

Close #2166

What is changed and how it works?

Upgrade TiCDC before TiKV and PD when cluster is equal or greater than v5.1.0

Check List

Tests

  • Manual test (add detailed scripts or steps below)
  1. Upgrade cluster from v5.0.0 to v.5.2.0
$ ./tiup-cluster upgrade my-cluster v5.2.0
This operation will upgrade tidb v5.0.0 cluster my-cluster to v5.2.0.
...
Upgrading component pd
        Restarting instance 127.0.0.1:2379
        Restart instance 127.0.0.1:2379 success
Upgrading component tikv
        Restarting instance 127.0.0.1:20160
        Restart instance 127.0.0.1:20160 success
Upgrading component tidb
        Restarting instance 127.0.0.1:4000
        Restart instance 127.0.0.1:4000 success
Upgrading component cdc
        Restarting instance 127.0.0.1:8300
        Restart instance 127.0.0.1:8300 success
...
  1. Upgrade cluster from v5.1.0 to v5.2.0
$ ./tiup-cluster upgrade my-cluster v5.2.0
This operation will upgrade tidb v5.1.0 cluster my-cluster to v5.2.0.
...
Upgrading component cdc
        Restarting instance 127.0.0.1:8300
        Restart instance 127.0.0.1:8300 success
Upgrading component pd
        Restarting instance 127.0.0.1:2379
        Restart instance 127.0.0.1:2379 success
Upgrading component tikv
        Restarting instance 127.0.0.1:20160
        Restart instance 127.0.0.1:20160 success
Upgrading component tidb
        Restarting instance 127.0.0.1:4000
        Restart instance 127.0.0.1:4000 success
...
  1. Upgrade cluster from v5.2.0 to v5.3.0
$  ./tiup-cluster upgrade my-cluster v5.3.0
This operation will upgrade tidb v5.2.0 cluster my-cluster to v5.3.0.
...
Upgrading component cdc
        Restarting instance 127.0.0.1:8300
        Restart instance 127.0.0.1:8300 success
Upgrading component pd
        Restarting instance 127.0.0.1:2379
        Restart instance 127.0.0.1:2379 success
Upgrading component tikv
        Restarting instance 127.0.0.1:20160
        Restart instance 127.0.0.1:20160 success
Upgrading component tidb
        Restarting instance 127.0.0.1:4000
...

Release notes:

Upgrade TiCDC before TiKV and PD when cluster is equal or greater than v5.1.0

@CLAassistant
Copy link

CLAassistant commented Aug 17, 2023

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot ti-chi-bot bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 17, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (2feb74b) 55.98% compared to head (0a44220) 55.98%.

❗ Current head 0a44220 differs from pull request most recent head 9fb8404. Consider uploading reports for the commit 9fb8404 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2253   +/-   ##
=======================================
  Coverage   55.98%   55.98%           
=======================================
  Files         320      320           
  Lines       33881    33891   +10     
=======================================
+ Hits        18965    18971    +6     
- Misses      12648    12651    +3     
- Partials     2268     2269    +1     
Flag Coverage Δ
cluster 44.82% <100.00%> (+0.02%) ⬆️
dm 25.69% <17.65%> (-0.06%) ⬇️
playground 15.70% <0.00%> (-0.01%) ⬇️
tiup 15.86% <0.00%> (-0.01%) ⬇️
unittest 22.64% <0.00%> (-0.01%) ⬇️

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

Files Changed Coverage Δ
components/dm/spec/logic.go 70.00% <100.00%> (ø)
pkg/cluster/manager/check.go 69.41% <100.00%> (ø)
pkg/cluster/manager/upgrade.go 57.29% <100.00%> (ø)
pkg/cluster/operation/upgrade.go 47.14% <100.00%> (ø)
pkg/cluster/spec/spec.go 83.60% <100.00%> (+0.27%) ⬆️
pkg/tidbver/tidbver.go 90.16% <100.00%> (+0.33%) ⬆️

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@hi-rustin hi-rustin left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/tidbver/tidbver.go Outdated Show resolved Hide resolved
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 18, 2023

@hi-rustin: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@hi-rustin
Copy link
Member

/cc @sdojjy

/hold

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 18, 2023

@hi-rustin: GitHub didn't allow me to request PR reviews from the following users: sdojjy.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @sdojjy

/hold

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 18, 2023
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 18, 2023

@sdojjy: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot bot removed the lgtm label Aug 21, 2023
@ti-chi-bot ti-chi-bot bot added the lgtm label Aug 21, 2023
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hi-rustin, kaaaaaaang, sdojjy

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 21, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-08-20 18:08:47.379936525 +0000 UTC m=+1086491.928952513: ☑️ agreed by kaaaaaaang.
  • 2023-08-21 02:12:50.650689687 +0000 UTC m=+1115535.199705659: ✖️🔁 reset by KanShiori.
  • 2023-08-21 03:11:24.211567308 +0000 UTC m=+1119048.760583295: ☑️ agreed by kaaaaaaang.

@kaaaaaaang
Copy link
Collaborator

/merge

@kaaaaaaang kaaaaaaang removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 21, 2023
@ti-chi-bot ti-chi-bot bot merged commit af2aa2a into pingcap:master Aug 21, 2023
17 of 18 checks passed
@KanShiori KanShiori deleted the shiori/cdc-upgrade-compatibility branch August 21, 2023 05:46
@kaaaaaaang kaaaaaaang added this to the 1.13.0 milestone Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is it possible to customize the upgrade sequence for different versions of a component?
6 participants