[to #295] Speed up cdc integrated tests #294

pingyu · 2022-11-04T15:54:42Z

Signed-off-by: pingyu [email protected]

What problem does this PR solve?

Issue Number: close #295, #296

Problem Description: Speed up integrated tests

What is changed and how does it work?

Speed up integrated tests by:
a. Disable checking TiDB status (but add checking TiKV status).
b. Shorten some during of sleep.
Try to fix unstable cdc: integrated test "cdc_hang_on" is not stable #296.
Fix integrated-test.Dockfile.
Add tests/up.sh to run integrated tests in a isolated environment.
By the way update master version as 1.1.0-master.
By the way add shellcheck. As we are using lots of shell scripts, I think it would be better to use some static lint tool to improve the codes. Use make tools/bin/shellcheck and tools/bin/shellcheck xxx to check modified scripts in our PRs. make shellcheck should be enabled in CI later.

Code changes

No code

Check List for Tests

This PR has been tested by at least one of the following methods:

Integration test

Side effects

No side effects

Related changes

No related changes

Signed-off-by: pingyu <[email protected]>

codecov · 2022-11-04T16:53:34Z

Codecov Report

Merging #294 (addf175) into main (b07ce74) will decrease coverage by 6.6615%.
The diff coverage is n/a.

Additional details and impacted files

@@               Coverage Diff                @@
##               main       #294        +/-   ##
================================================
- Coverage   61.0519%   54.3904%   -6.6616%     
================================================
  Files           240        238         -2     
  Lines         20286      20294         +8     
================================================
- Hits          12385      11038      -1347     
- Misses         6780       8379      +1599     
+ Partials       1121        877       -244

Flag	Coverage Δ		*Carryforward flag
br	`39.5359% <ø> (-20.9650%)`	⬇️	Carriedforward from 405c41e
cdc	`61.2001% <ø> (-0.1075%)`	⬇️

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
br/pkg/task/backup.go	`0.0000% <0.0000%> (-100.0000%)`	⬇️
br/pkg/storage/local_unix.go	`0.0000% <0.0000%> (-100.0000%)`	⬇️
br/pkg/task/restore_raw_config.go	`0.0000% <0.0000%> (-71.4286%)`	⬇️
br/pkg/task/backup_raw.go	`0.0000% <0.0000%> (-67.9739%)`	⬇️
br/pkg/task/restore.go	`0.0000% <0.0000%> (-67.6471%)`	⬇️
br/pkg/utils/tls_config.go	`0.0000% <0.0000%> (-65.2174%)`	⬇️
br/pkg/summary/summary.go	`0.0000% <0.0000%> (-62.5000%)`	⬇️
br/pkg/task/restore_raw.go	`0.0000% <0.0000%> (-62.2450%)`	⬇️
br/pkg/task/common.go	`18.6813% <0.0000%> (-51.2641%)`	⬇️
br/pkg/restore/client.go	`10.6976% <0.0000%> (-50.6977%)`	⬇️
... and 46 more

Signed-off-by: pingyu <[email protected]>

pingyu · 2022-11-05T05:05:35Z

Mark:
Captures seems to still be stuck after PD stopped. See https://ci.pingcap.net/blue/organizations/jenkins/common-check/detail/common-check/112055/pipeline/

And this may be the reason of "restart" argument of run_cdc_server not working ?

pingyu · 2022-11-05T07:15:48Z

Mark:
It can be observed that there are more than 2 captures. The result of "check capture count" may be not reliable.

failed to check capture count, expected: 2, got: 4, retry: 5
captures: [
  {
    "id": "313b963a-9386-4665-92ba-0b32422b9253",
    "is-owner": false,
    "address": "127.0.0.1:8601"
  },
  {
    "id": "a5a02ae1-8dc1-4eb8-85d7-68770a16779f",
    "is-owner": true,
    "address": "127.0.0.1:8602"
  },
  {
    "id": "aa44cd6d-d672-4982-99a9-f68fa7cf737a",
    "is-owner": false,
    "address": "127.0.0.1:8601"
  },
  {
    "id": "c49a38d6-7061-4a21-96e9-ec64a8825e0f",
    "is-owner": false,
    "address": "127.0.0.1:8602"
  }
]

Signed-off-by: pingyu <[email protected]>

pingyu · 2022-11-05T15:58:13Z

Mark:
Successful run: https://ci.pingcap.net/blue/organizations/jenkins/migration-verify-pipeline/detail/migration-verify-pipeline/789/pipeline

pingyu · 2022-11-05T15:58:20Z

/verify

pingyu · 2022-11-06T02:46:12Z

Mark:
Unstable case of multi_capture:

[2022-11-05T16:41:28.333Z] + sleep 1
[2022-11-05T16:41:29.946Z] + (( i++ ))
[2022-11-05T16:41:29.946Z] + (( i <= 50 ))
[2022-11-05T16:41:29.946Z] ++ curl -vsL http://127.0.0.1:8601/debug/info
[2022-11-05T16:41:29.946Z] * About to connect() to 127.0.0.1 port 8601 (#0)
[2022-11-05T16:41:29.946Z] *   Trying 127.0.0.1...
[2022-11-05T16:41:29.946Z] * Connection refused
[2022-11-05T16:41:29.946Z] * Failed connect to 127.0.0.1:8601; Connection refused
[2022-11-05T16:41:29.946Z] * Closing connection 0
[2022-11-05T16:41:29.946Z] + res=
[2022-11-05T16:41:29.946Z] + echo ''
[2022-11-05T16:41:29.946Z] + grep -q 'failed to get info:'
[2022-11-05T16:41:29.946Z] + echo ''
[2022-11-05T16:41:29.946Z] + grep -q 'etcd info'
[2022-11-05T16:41:29.946Z] + '[' 50 -eq 50 ']'
[2022-11-05T16:41:29.946Z] + echo 'Failed to start TiKV-CDC'
[2022-11-05T16:41:29.946Z] Failed to start TiKV-CDC
[2022-11-05T16:41:29.946Z] + exit 1

log: https://ci.pingcap.net/blue/organizations/jenkins/common-check/detail/common-check/112072/pipeline/

pingyu · 2022-11-06T02:46:20Z

/verify

pingyu · 2022-11-07T03:42:06Z

@haojinming @zeminzhou PTAL, thanks~

zeminzhou · 2022-11-07T03:47:13Z

Mark: Captures seems to still be stuck after PD stopped. See https://ci.pingcap.net/blue/organizations/jenkins/common-check/detail/common-check/112055/pipeline/

And this may be the reason of "restart" argument of run_cdc_server not working ?

At the end of run_cdc_server, it will check cdc status and return.

50s maybe is not enough to stop and restart cdc.

zeminzhou · 2022-11-07T03:54:44Z

cdc/deployments/tikv-cdc/docker/integration-test.Dockerfile

-RUN make integration_test_build kafka_consumer cdc
-COPY --from=downloader /root/download/bin/* ./bin/
+RUN make integration_test_build cdc
+COPY --from=downloader /root/download/bin/* ./scripts/bin/


What to copy？

The tidb-server, tikv-server, etc., which are downloaded by ./scripts/download-integration-test-binaries.sh.

Now make integration_test_build will not download these binaries, we should call make prepare_test_binaries to download these binaries.

The binaries are prepared at the beginning. See line 8 here.

zeminzhou · 2022-11-07T03:54:55Z

cdc/deployments/tikv-cdc/docker/integration-test.Dockerfile

 # We always need to clean before we build, please don't adjust its order.
 RUN make clean
-RUN make integration_test_build kafka_consumer cdc
-COPY --from=downloader /root/download/bin/* ./bin/
+RUN make integration_test_build cdc


remove cdc？

The tikv-cdc is necessary as test cases using $(tikv-cdc cli xxx) ?

I mean make integration_test_build cdc -> make integration_test_build, because make integration_test_build will build tikv-cdc.test & tikv-cdc

Oh, I see.
Fixed, PTAL~

zeminzhou · 2022-11-07T04:02:56Z

cdc/tests/up.sh

+	echo "Usage: $0 [OPTIONS]"
+	echo "OPTIONS:"
+	echo "  --help                 Display this message"
+	echo "  --tag (TAG)            Specify images tag used in br tests"


br -> cdc ?

Fixed. PTAL~

zeminzhou · 2022-11-07T04:08:42Z

cdc/tests/up.sh

+	echo "Run \"make integration_test\" to start integrated tests"
+	# shellcheck disable=SC2068
+	exec docker run -it \
+		-v "$host_tmp":/tmp/tikv_cdc_test \


Why binds mount a volume?

To write logs to host directory, for addressing issues. Otherwise after exit the container, all logs gone.

Get it, that's good idea!

Signed-off-by: pingyu <[email protected]>

haojinming

LGTM

Signed-off-by: pingyu <[email protected]>

zeminzhou · 2022-11-08T06:08:06Z

/verify

zeminzhou

LGTM～

* speed up integrated tests Signed-off-by: pingyu <[email protected]> * wip Signed-off-by: pingyu <[email protected]> * update release version; add usage comment for up.sh Signed-off-by: pingyu <[email protected]> * print more debug info Signed-off-by: pingyu <[email protected]> * fix fmt Signed-off-by: pingyu <[email protected]> * add shellcheck Signed-off-by: pingyu <[email protected]> * fix CI error Signed-off-by: pingyu <[email protected]> * remove "br" in comment Signed-off-by: pingyu <[email protected]> * remove useless cdc target Signed-off-by: pingyu <[email protected]> Signed-off-by: pingyu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

* add a integration test case for disk full Signed-off-by: zeminzhou <[email protected]> * [to #295] Speed up cdc integrated tests (#294) * speed up integrated tests Signed-off-by: pingyu <[email protected]> * wip Signed-off-by: pingyu <[email protected]> * update release version; add usage comment for up.sh Signed-off-by: pingyu <[email protected]> * print more debug info Signed-off-by: pingyu <[email protected]> * fix fmt Signed-off-by: pingyu <[email protected]> * add shellcheck Signed-off-by: pingyu <[email protected]> * fix CI error Signed-off-by: pingyu <[email protected]> * remove "br" in comment Signed-off-by: pingyu <[email protected]> * remove useless cdc target Signed-off-by: pingyu <[email protected]> Signed-off-by: pingyu <[email protected]> Signed-off-by: zeminzhou <[email protected]> * fix check Signed-off-by: zeminzhou <[email protected]> * remove unused code & remove modified it cases (#301) Signed-off-by: zeminzhou <[email protected]> Signed-off-by: zeminzhou <[email protected]> Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]> * address comment Signed-off-by: zeminzhou <[email protected]> * make check Signed-off-by: zeminzhou <[email protected]> * Update cdc/tests/integration_tests/disk_full/run.sh Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]> * fix stop_downstream Signed-off-by: zeminzhou <[email protected]> * fix check Signed-off-by: zeminzhou <[email protected]> * fix it Signed-off-by: zeminzhou <[email protected]> * fix disk full Signed-off-by: zeminzhou <[email protected]> * make check Signed-off-by: zeminzhou <[email protected]> Signed-off-by: zeminzhou <[email protected]> Signed-off-by: pingyu <[email protected]> Co-authored-by: Ping Yu <[email protected]> Co-authored-by: Ping Yu <[email protected]>

pingyu added 2 commits November 4, 2022 23:54

speed up integrated tests

57ec242

Signed-off-by: pingyu <[email protected]>

Merge branch 'main' into speed-up-it

c7d8f2b

pingyu added 2 commits November 5, 2022 01:26

wip

bc6199a

Signed-off-by: pingyu <[email protected]>

update release version; add usage comment for up.sh

93ce7eb

Signed-off-by: pingyu <[email protected]>

github-actions bot added the component/cdc label Nov 5, 2022

pingyu changed the title ~~[close #xxx][Draft] speed up integrated tests~~ [to #295] Speed up cdc integrated tests Nov 5, 2022

pingyu added 3 commits November 5, 2022 15:22

print more debug info

2090175

Signed-off-by: pingyu <[email protected]>

fix fmt

c153cd3

Signed-off-by: pingyu <[email protected]>

add shellcheck

4bab44e

Signed-off-by: pingyu <[email protected]>

pingyu mentioned this pull request Nov 5, 2022

cdc: unit test clientSuite.TestStreamSendWithError is unstable #297

Open

fix CI error

e1f3b0a

Signed-off-by: pingyu <[email protected]>

pingyu requested review from zeminzhou and haojinming November 7, 2022 03:41

zeminzhou reviewed Nov 7, 2022

View reviewed changes

pingyu added 2 commits November 7, 2022 15:19

remove "br" in comment

9ab6c93

Signed-off-by: pingyu <[email protected]>

Merge branch 'main' into speed-up-it

405c41e

haojinming approved these changes Nov 8, 2022

View reviewed changes

remove useless cdc target

addf175

Signed-off-by: pingyu <[email protected]>

zeminzhou reviewed Nov 8, 2022

View reviewed changes

zeminzhou approved these changes Nov 8, 2022

View reviewed changes

pingyu merged commit b22ec23 into tikv:main Nov 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[to #295] Speed up cdc integrated tests #294

[to #295] Speed up cdc integrated tests #294

pingyu commented Nov 4, 2022 •

edited

Loading

codecov bot commented Nov 4, 2022 •

edited

Loading

pingyu commented Nov 5, 2022 •

edited

Loading

pingyu commented Nov 5, 2022

pingyu commented Nov 5, 2022

pingyu commented Nov 5, 2022

pingyu commented Nov 6, 2022

pingyu commented Nov 6, 2022

pingyu commented Nov 7, 2022

zeminzhou commented Nov 7, 2022

zeminzhou Nov 7, 2022

pingyu Nov 7, 2022

zeminzhou Nov 8, 2022

pingyu Nov 8, 2022 •

edited

Loading

zeminzhou Nov 8, 2022

zeminzhou Nov 7, 2022

pingyu Nov 7, 2022

zeminzhou Nov 8, 2022

pingyu Nov 8, 2022

zeminzhou Nov 7, 2022

pingyu Nov 7, 2022

zeminzhou Nov 7, 2022

pingyu Nov 7, 2022

zeminzhou Nov 8, 2022

haojinming left a comment

zeminzhou commented Nov 8, 2022

zeminzhou left a comment

[to #295] Speed up cdc integrated tests #294

[to #295] Speed up cdc integrated tests #294

Conversation

pingyu commented Nov 4, 2022 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Code changes

Check List for Tests

Side effects

Related changes

codecov bot commented Nov 4, 2022 • edited Loading

Codecov Report

pingyu commented Nov 5, 2022 • edited Loading

pingyu commented Nov 5, 2022

pingyu commented Nov 5, 2022

pingyu commented Nov 5, 2022

pingyu commented Nov 6, 2022

pingyu commented Nov 6, 2022

pingyu commented Nov 7, 2022

zeminzhou commented Nov 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pingyu Nov 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haojinming left a comment

Choose a reason for hiding this comment

zeminzhou commented Nov 8, 2022

zeminzhou left a comment

Choose a reason for hiding this comment

pingyu commented Nov 4, 2022 •

edited

Loading

codecov bot commented Nov 4, 2022 •

edited

Loading

pingyu commented Nov 5, 2022 •

edited

Loading

pingyu Nov 8, 2022 •

edited

Loading