br: add chaos testing for advancer owner #58183

Tristan1900 · 2024-12-12T01:51:18Z

What problem does this PR solve?

Issue Number: close #50458

Problem Summary:

What changed and how does it work?

Add a cycle to force to be owner and retire when using log advancer cmd. In the test we can start this debugging command and it will do this cycle repeatedly until exits. It creates a chaos scenario for advancer testing.
Also all the parameters in the advancer are exposed to the cmd already which is convenient. The advancer daemon running on TiDB domain will not be affected by any of these configs since they use default and cannot be configured.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

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 · 2024-12-12T01:51:21Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tiprow · 2024-12-12T01:51:34Z

Hi @Tristan1900. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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-sigs/prow repository.

Signed-off-by: Wenqi Mou <[email protected]>

Tristan1900 · 2024-12-12T01:53:52Z

br/pkg/streamhelper/config/advancer_conf.go

@@ -11,18 +11,19 @@ import (
 const (
 	flagBackoffTime         = "backoff-time"
 	flagTickInterval        = "tick-interval"
-	flagFullScanDiffTick    = "full-scan-tick"


removed since not used

Tristan1900 · 2024-12-12T01:54:09Z

br/pkg/streamhelper/advancer.go

@@ -270,11 +270,6 @@ func (c *CheckpointAdvancer) WithCheckpoints(f func(*spans.ValueSortedFull)) {
 	f(c.checkpoints)
 }

-// only used for test
-func (c *CheckpointAdvancer) NewCheckpoints(cps *spans.ValueSortedFull) {


removed since not used

br/pkg/streamhelper/config/advancer_conf.go

Signed-off-by: Wenqi Mou <[email protected]>

Tristan1900 · 2024-12-12T20:50:31Z

br/pkg/streamhelper/daemon/owner_daemon_test.go

@@ -149,7 +149,6 @@ func TestDaemon(t *testing.T) {
 	ow.RetireOwner()
 	req.False(ow.IsOwner())
 	app.AssertNotRunning(1 * time.Second)
-	ow.CampaignOwner()


it doesn't need to campaign again to become leader since only one node

Tristan1900 · 2024-12-12T20:50:52Z

pkg/config/config.go

@@ -458,13 +457,6 @@ func (b *AtomicBool) UnmarshalText(text []byte) error {
 	return nil
 }

-// LogBackup is the config for log backup service.
-// For now, it includes the embed advancer.
-type LogBackup struct {


remove unused

Signed-off-by: Wenqi Mou <[email protected]>

codecov · 2024-12-12T21:19:06Z

Codecov Report

Attention: Patch coverage is 27.77778% with 39 lines in your changes missing coverage. Please review.

Project coverage is 74.6813%. Comparing base (d6b313f) to head (8e34b5e).
Report is 172 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #58183        +/-   ##
================================================
+ Coverage   73.1461%   74.6813%   +1.5352%     
================================================
  Files          1675       1700        +25     
  Lines        461870     485247     +23377     
================================================
+ Hits         337840     362389     +24549     
+ Misses       103283     100775      -2508     
- Partials      20747      22083      +1336

Flag	Coverage Δ
integration	`46.1031% <22.2222%> (?)`
unit	`72.4320% <27.7777%> (+0.1641%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.6910% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`59.0998% <27.7777%> (+13.0924%)`	⬆️

Tristan1900 · 2024-12-22T18:24:44Z

tested manually using testbed

Tristan1900 · 2024-12-22T18:25:39Z

@BornChanger please take another look when you get a chance, thanks!

Leavrth · 2024-12-31T01:57:43Z

br/pkg/streamhelper/config/advancer_conf.go

@@ -49,14 +57,19 @@ func DefineFlagsForCheckpointAdvancerConfig(f *pflag.FlagSet) {
 		"If the checkpoint lag is greater than how long, we would try to poll TiKV for checkpoints.")
 	f.Duration(flagCheckPointLagLimit, DefaultCheckPointLagLimit,
 		"The maximum lag could be tolerated for the checkpoint lag.")
+
+	// used for chaos testing
+	f.Duration(flagOwnershipCycleInterval, DefaultOwnershipCycleInterval,


should we hide the parameter?

f.MarkHidden(flagOwnershipCycleInterval)

ti-chi-bot · 2024-12-31T02:30:02Z

[LGTM Timeline notifier]

Timeline:

2024-12-30 06:38:40.113720139 +0000 UTC m=+332455.469724680: ☑️ agreed by BornChanger.
2024-12-31 02:30:01.296278448 +0000 UTC m=+403936.652282987: ☑️ agreed by Leavrth.

tiprow · 2024-12-31T02:32:18Z

@Tristan1900: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
fast_test_tiprow	`546eb1e`	link	true	`/test fast_test_tiprow`
tidb_parser_test	`546eb1e`	link	true	`/test tidb_parser_test`

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

Signed-off-by: Wenqi Mou <[email protected]>

ti-chi-bot · 2024-12-31T03:54:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BornChanger, Leavrth, yudongusa

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:

~~OWNERS~~ [BornChanger,Leavrth]
~~br/OWNERS~~ [BornChanger,Leavrth]
~~pkg/config/OWNERS~~ [yudongusa]

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

Tristan1900 · 2024-12-31T13:55:00Z

/retest

tiprow · 2024-12-31T13:55:27Z

@Tristan1900: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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-sigs/prow repository.

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked labels Dec 12, 2024

ti-chi-bot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 12, 2024

expose chaos testing

751f81c

Signed-off-by: Wenqi Mou <[email protected]>

Tristan1900 force-pushed the expose-config-for-test branch from 2e8a8eb to 751f81c Compare December 12, 2024 01:53

Tristan1900 commented Dec 12, 2024

View reviewed changes

BornChanger reviewed Dec 12, 2024

View reviewed changes

br/pkg/streamhelper/config/advancer_conf.go Show resolved Hide resolved

BornChanger reviewed Dec 12, 2024

View reviewed changes

br/pkg/streamhelper/config/advancer_conf.go Show resolved Hide resolved

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

add config to stream advancer command

252cbc1

Signed-off-by: Wenqi Mou <[email protected]>

Tristan1900 commented Dec 12, 2024

View reviewed changes

Tristan1900 marked this pull request as ready for review December 12, 2024 20:51

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

fix bazel

546eb1e

Signed-off-by: Wenqi Mou <[email protected]>

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label Dec 22, 2024

BornChanger approved these changes Dec 30, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 30, 2024

Leavrth approved these changes Dec 31, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 31, 2024

yudongusa approved these changes Dec 31, 2024

View reviewed changes

Tristan1900 added 2 commits December 30, 2024 22:48

mark hidden and better comments

a4e8efc

Signed-off-by: Wenqi Mou <[email protected]>

revert domain naming change

8e34b5e

Signed-off-by: Wenqi Mou <[email protected]>

ti-chi-bot bot added the approved label Dec 31, 2024

ti-chi-bot bot merged commit a6cd5e7 into pingcap:master Dec 31, 2024
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

br: add chaos testing for advancer owner #58183

br: add chaos testing for advancer owner #58183

Tristan1900 commented Dec 12, 2024 •

edited

Loading

ti-chi-bot bot commented Dec 12, 2024

tiprow bot commented Dec 12, 2024

Tristan1900 Dec 12, 2024 •

edited

Loading

Tristan1900 Dec 12, 2024

Tristan1900 Dec 12, 2024 •

edited

Loading

Tristan1900 Dec 12, 2024

codecov bot commented Dec 12, 2024 •

edited

Loading

Tristan1900 commented Dec 22, 2024 •

edited

Loading

Tristan1900 commented Dec 22, 2024

Leavrth Dec 31, 2024

ti-chi-bot bot commented Dec 31, 2024

tiprow bot commented Dec 31, 2024

ti-chi-bot bot commented Dec 31, 2024

Tristan1900 commented Dec 31, 2024

tiprow bot commented Dec 31, 2024

br: add chaos testing for advancer owner #58183

br: add chaos testing for advancer owner #58183

Conversation

Tristan1900 commented Dec 12, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Dec 12, 2024

tiprow bot commented Dec 12, 2024

Tristan1900 Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Tristan1900 Dec 12, 2024

Choose a reason for hiding this comment

Tristan1900 Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Tristan1900 Dec 12, 2024

Choose a reason for hiding this comment

codecov bot commented Dec 12, 2024 • edited Loading

Codecov Report

Tristan1900 commented Dec 22, 2024 • edited Loading

Tristan1900 commented Dec 22, 2024

Leavrth Dec 31, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Dec 31, 2024

[LGTM Timeline notifier]

tiprow bot commented Dec 31, 2024

ti-chi-bot bot commented Dec 31, 2024

Tristan1900 commented Dec 31, 2024

tiprow bot commented Dec 31, 2024

Tristan1900 commented Dec 12, 2024 •

edited

Loading

Tristan1900 Dec 12, 2024 •

edited

Loading

Tristan1900 Dec 12, 2024 •

edited

Loading

codecov bot commented Dec 12, 2024 •

edited

Loading

Tristan1900 commented Dec 22, 2024 •

edited

Loading