log-backup: add the switch for log backup #36115

3pointer · 2022-07-12T05:10:03Z

What problem does this PR solve?

Issue Number: ref #29501

Problem Summary:
we have several places need to check whether log backup is enabled.

Resolve locks to scan more locks after safepoint.
Add index to skip using lightning.
Telemetry of log backup feature usage statistics.

What is changed and how it works?

Add a switch for log backup is enabled.
remove the old logic that check log backup config from http client.
This PR also fix two unstable test(owner_test, ddl_test) to make CI passed.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

when cluster not enable log backup or don't have the config log-backup.enable.

when enable to true.

No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

ti-chi-bot · 2022-07-12T05:10:05Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

YuJuncen
joccau

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.

update

sre-bot · 2022-07-12T13:03:18Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/721d1a446acb9ba79474d9029be8013cab963618

3pointer · 2022-07-14T07:07:55Z

ddl/backfilling.go

@@ -609,6 +609,16 @@ func (w *worker) writePhysicalTableRecord(t table.PhysicalTable, bfWorkerType ba
 	}()
 	jc := w.jobContext(job)

+	// ctx, err := w.sessPool.get()


@Benjamin2037 Please help review this usage.

3pointer · 2022-07-14T07:09:01Z

ddl/util/main_test.go

@@ -25,6 +25,7 @@ func TestMain(m *testing.M) {
 	testsetup.SetupForCommonTest()
 	opts := []goleak.Option{
 		goleak.IgnoreTopFunction("github.com/golang/glog.(*loggingT).flushDaemon"),
+		goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"),


@hawkingrei Please help review this fix of unit test.

3pointer · 2022-07-14T07:13:53Z

telemetry/data_feature_usage.go

@@ -47,6 +48,7 @@ type featureUsage struct {
 	NonTransactionalUsage *m.NonTransactionalStmtCounter   `json:"nonTransactional"`
 	GlobalKill            bool                             `json:"globalKill"`
 	MultiSchemaChange     *m.MultiSchemaChangeUsageCounter `json:"multiSchemaChange"`
+	LogBackup             bool                             `json:"logBackup"`


@AstroProfundis Please help review this change.

YuJuncen

rest lgtm

br/pkg/utils/db.go

3pointer · 2022-07-14T07:56:22Z

/run-integration-br-tests

joccau

LGTM

3pointer · 2022-07-14T10:32:00Z

/hold for more reviewers

sticnarf

The gc worker part lgtm.

ti-chi-bot · 2022-07-15T05:48:52Z

@AstroProfundis: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

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 ti-community-infra/tichi repository.

3pointer · 2022-07-15T05:49:56Z

/unhold

3pointer · 2022-07-15T05:50:05Z

/merge

ti-chi-bot · 2022-07-15T05:50:08Z

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

Commit hash: 6e512cc

sre-bot · 2022-07-15T07:05:40Z

TiDB MergeCI notify

🔴 Bad News! New failing [1] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-ddl-test	🟥 failed 1, success 5, total 6	8 min 29 sec	New failing
idc-jenkins-ci-tidb/tics-test	🔴 failed 1, success 0, total 1	6 min 59 sec	Existing failure
idc-jenkins-ci/integration-cdc-test	🟢 all 36 tests passed	28 min	Existing passed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 11 tests passed	25 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 12 tests passed	15 min	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	9 min 37 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	7 min 18 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	6 min 7 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 25 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

log-backup: add switch for log backup

b0f2213

3pointer force-pushed the add_switch_for_logbackup branch from eb4b4a8 to b0f2213 Compare July 12, 2022 05:11

3pointer changed the title ~~log-backup: add switch for log backup~~ log-backup: add the switch for log backup Jul 12, 2022

ti-chi-bot removed the do-not-merge/needs-linked-issue label Jul 12, 2022

3pointer requested a review from joccau July 12, 2022 05:13

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 12, 2022

3pointer force-pushed the add_switch_for_logbackup branch from c540ca8 to d967eee Compare July 12, 2022 07:28

3pointer added 2 commits July 12, 2022 15:45

add test

b9d640e

update

fix test

b9a2395

3pointer force-pushed the add_switch_for_logbackup branch from b4c72d9 to b9a2395 Compare July 12, 2022 09:02

fix test

96c7dab

3pointer force-pushed the add_switch_for_logbackup branch from 8fcac45 to 96c7dab Compare July 12, 2022 12:11

3pointer added 3 commits July 12, 2022 20:39

fix leak test

0666771

fix gc worker test

a327e21

fix gc worker test

6039901

3pointer marked this pull request as ready for review July 13, 2022 02:23

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 13, 2022

3pointer added 5 commits July 13, 2022 20:15

adjust test

b46a99f

udpate code

c02b031

add example code for tidb

cf483af

use sql instead of http api to get config

9bd6ff6

remove useless code

74e5299

3pointer force-pushed the add_switch_for_logbackup branch from bbbea79 to 74e5299 Compare July 14, 2022 06:16

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 14, 2022

fmt

ba4fc17

3pointer commented Jul 14, 2022

View reviewed changes

adjust code

97b57ab

YuJuncen approved these changes Jul 14, 2022

View reviewed changes

br/pkg/utils/db.go Outdated Show resolved Hide resolved

br/pkg/utils/db.go Show resolved Hide resolved

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 14, 2022

3pointer added 2 commits July 14, 2022 15:41

address comment

5afb5c3

fix build

87c0041

address comment

23c4d88

joccau approved these changes Jul 14, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 14, 2022

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2022

sticnarf reviewed Jul 15, 2022

View reviewed changes

Merge branch 'master' into add_switch_for_logbackup

6e512cc

AstroProfundis approved these changes Jul 15, 2022

View reviewed changes

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 15, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 15, 2022

Merge branch 'master' into add_switch_for_logbackup

721d1a4

ti-chi-bot merged commit f47978c into pingcap:master Jul 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

log-backup: add the switch for log backup #36115

log-backup: add the switch for log backup #36115

3pointer commented Jul 12, 2022 •

edited

Loading

ti-chi-bot commented Jul 12, 2022 •

edited

Loading

sre-bot commented Jul 12, 2022 •

edited

Loading

3pointer Jul 14, 2022

3pointer Jul 14, 2022

3pointer Jul 14, 2022

YuJuncen left a comment

3pointer commented Jul 14, 2022

joccau left a comment

3pointer commented Jul 14, 2022

sticnarf left a comment

ti-chi-bot commented Jul 15, 2022

3pointer commented Jul 15, 2022

3pointer commented Jul 15, 2022

ti-chi-bot commented Jul 15, 2022

sre-bot commented Jul 15, 2022

log-backup: add the switch for log backup #36115

log-backup: add the switch for log backup #36115

Conversation

3pointer commented Jul 12, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jul 12, 2022 • edited Loading

sre-bot commented Jul 12, 2022 • edited Loading

3pointer Jul 14, 2022

Choose a reason for hiding this comment

3pointer Jul 14, 2022

Choose a reason for hiding this comment

3pointer Jul 14, 2022

Choose a reason for hiding this comment

YuJuncen left a comment

Choose a reason for hiding this comment

3pointer commented Jul 14, 2022

joccau left a comment

Choose a reason for hiding this comment

3pointer commented Jul 14, 2022

sticnarf left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Jul 15, 2022

3pointer commented Jul 15, 2022

3pointer commented Jul 15, 2022

ti-chi-bot commented Jul 15, 2022

sre-bot commented Jul 15, 2022

TiDB MergeCI notify

3pointer commented Jul 12, 2022 •

edited

Loading

ti-chi-bot commented Jul 12, 2022 •

edited

Loading

sre-bot commented Jul 12, 2022 •

edited

Loading