tests/br: fix bug in issue #1158 #1160

Leavrth · 2021-05-31T04:17:58Z

What problem does this PR solve?

fix bug in issue #1158

What is changed and how it works?

for #1158 add sleep 1 to ensure tikv start up completely before restore

Check List

Tests

Unit test
Integration test

Release note

-No release note

Signed-off-by: Leavrth <Legenull@gmail.com>

Leavrth · 2021-05-31T05:16:42Z

/run-all-tests

Leavrth · 2021-05-31T05:18:07Z

/run-build

Leavrth · 2021-05-31T05:27:11Z

/build

Leavrth · 2021-05-31T06:28:01Z

/run-integration-test

Leavrth · 2021-05-31T08:16:19Z

/run-integration-test

YuJuncen · 2021-05-31T13:43:41Z

Maybe the utility function ensure_tikv would be suitable for this? (Eh, maybe needing some modify, say, ensure every TiKV nodes are up and connected via interfaces of PD.)

br/tests/_utils/run_services

Lines 144 to 154 in 701909b

    
           ensure_tikv() { 
        
               echo "Waiting initializing TiKV..." 
        
               while ! run_curl "https://$PD_ADDR/pd/api/v1/cluster/status" | grep '"is_initialized": true'; do 
        
                  i=$((i+1)) 
        
                  if [ "$i" -gt 20 ]; then 
        
                     echo 'Failed to initialize TiKV cluster' 
        
                     return 1 
        
                  fi 
        
                  sleep 5 
        
               done 
        
           }

~~BTW, according to the log provided in #1158, seems TiKV 3 started after 5 seconds since restoration starting. I guess maybe 1s of sleeping isn't enough?~~ Well, seems there is clock drifting between the host and the container.

Leavrth · 2021-06-01T02:05:23Z

Maybe the utility function ensure_tikv would be suitable for this? (Eh, maybe needing some modify, say, ensure every TiKV nodes are up and connected via interfaces of PD.)

Got it, I will modify the code and have a test.

Well, seems there is clock drifting between the host and the container.

Yes, there seems 2 different clocks in bash and in BR

Leavrth · 2021-06-01T05:00:08Z

/run-integration-test

Leavrth · 2021-06-01T09:34:57Z

/run-integration-test

3pointer · 2021-06-01T10:05:13Z

/lgtm

ti-chi-bot · 2021-06-01T10:05:14Z

@3pointer: Please use GitHub review feature instead of /lgtm [cancel] when you want to submit review to the pull request.
For how to use GitHub review feature, see also this document provided by GitHub.

For the reason we drop support to the commands, see also this page.
This reply is being used as a temporary reply during the migration of review process and will be removed on July 1st.

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.

Leavrth · 2021-06-01T10:10:44Z

/merge

ti-chi-bot · 2021-06-01T10:10:46Z

@Leavrth: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

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.

glorv · 2021-06-01T12:15:41Z

tests/br_tikv_outage/run.sh

@@ -71,6 +71,8 @@ github.com/pingcap/br/pkg/conn/hint-get-backup-client=1*return(\"$hint_get_backu
    backup_pid=$!
    single_point_fault $failure
    wait $backup_pid
+    # let tikv start up completely if backup is finished before tikv restarts
+    ensure_tikv


shall we move this line into the case block to make it clearer which failures depend on this

I add the line into kv_outage function where tikv restart. It seems better.

Signed-off-by: Leavrth <Legenull@gmail.com>

ti-chi-bot · 2021-06-02T03:00:14Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

3pointer
glorv

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.

glorv · 2021-06-02T03:00:56Z

/hold

3pointer · 2021-06-02T05:05:06Z

/merge

ti-chi-bot · 2021-06-02T05:05:15Z

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

Commit hash: 0c47e51

3pointer · 2021-06-02T05:07:50Z

/unhold

backup/checksum: add cluster index support for ranges (pingcap#1120) lightning/restore: support ingset multi ssts for same range (pingcap#1089) * ingest ssts for the same range in a batch * make ingest compatible with old tikv tests: add row count check for br clustered index tests (pingcap#1151) storage: mkdirAll for local storage even when SkipCheckPath is true (pingcap#1156) tests/br: fix bug in issue pingcap#1158 (pingcap#1160) tests/br/compatibility: fix failed for manifest unknown (pingcap#1161) *: upgrade go version from 1.13/1.15 to 1.16 (pingcap#1159) action trigger: fix compatibility trigger bug on push (pingcap#1170) address comment fix test address comment address comment fix build add ut for ignore columns go.mod: update tidb to the new version (pingcap#1153) lightning: support restore data into tables that contains data (pingcap#784) fix conflict after merge master

ensure tikv start up completely before restore

aa7eae2

Signed-off-by: Leavrth <Legenull@gmail.com>

ti-chi-bot requested review from kennytm and overvenus May 31, 2021 04:18

ti-chi-bot added the size/XS label May 31, 2021

Leavrth changed the title ~~tests: fix bug in issue(#1158 and #1050)~~ tests/br: fix bug in issue(#1158 and #1050) May 31, 2021

Leavrth linked an issue May 31, 2021 that may be closed by this pull request

tests: br_tikv_outage sometimes fail #1158

Closed

Leavrth added the do-not-merge/work-in-progress status/WIP label May 31, 2021

use ensure_tikv instead

80ca77c

Leavrth changed the title ~~tests/br: fix bug in issue(#1158 and #1050)~~ tests/br: fix bug in issue #1158 Jun 1, 2021

ti-chi-bot removed the do-not-merge/work-in-progress status/WIP label Jun 1, 2021

3pointer approved these changes Jun 1, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 LGTM1 label Jun 1, 2021

glorv reviewed Jun 1, 2021

View reviewed changes

add ensure_tikv into kv_outage

0c47e51

Signed-off-by: Leavrth <Legenull@gmail.com>

glorv approved these changes Jun 2, 2021

View reviewed changes

ti-chi-bot removed the status/LGT1 LGTM1 label Jun 2, 2021

ti-chi-bot added the status/LGT2 LGTM2 label Jun 2, 2021

ti-chi-bot added the do-not-merge/hold label Jun 2, 2021

3pointer approved these changes Jun 2, 2021

View reviewed changes

ti-chi-bot added the status/can-merge label Jun 2, 2021

ti-chi-bot removed the do-not-merge/hold label Jun 2, 2021

ti-chi-bot merged commit 293cafe into pingcap:master Jun 2, 2021

3pointer pushed a commit to 3pointer/br that referenced this pull request Jun 4, 2021

tests/br: fix bug in issue pingcap#1158 (pingcap#1160)

a7b7754

Leavrth deleted the br_tikv_outage_fix branch June 11, 2021 04:20

Leavrth mentioned this pull request Jun 25, 2021

tests: fix br_tikv_outage learner bug #1256

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests/br: fix bug in issue #1158 #1160

tests/br: fix bug in issue #1158 #1160

Leavrth commented May 31, 2021 •

edited

Loading

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

YuJuncen commented May 31, 2021 •

edited

Loading

Leavrth commented Jun 1, 2021

Leavrth commented Jun 1, 2021

Leavrth commented Jun 1, 2021

3pointer commented Jun 1, 2021

ti-chi-bot commented Jun 1, 2021

Leavrth commented Jun 1, 2021

ti-chi-bot commented Jun 1, 2021

glorv Jun 1, 2021

Leavrth Jun 2, 2021 •

edited

Loading

ti-chi-bot commented Jun 2, 2021

glorv commented Jun 2, 2021

3pointer commented Jun 2, 2021

ti-chi-bot commented Jun 2, 2021

3pointer commented Jun 2, 2021

tests/br: fix bug in issue #1158 #1160

tests/br: fix bug in issue #1158 #1160

Conversation

Leavrth commented May 31, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

Leavrth commented May 31, 2021

YuJuncen commented May 31, 2021 • edited Loading

Leavrth commented Jun 1, 2021

Leavrth commented Jun 1, 2021

Leavrth commented Jun 1, 2021

3pointer commented Jun 1, 2021

ti-chi-bot commented Jun 1, 2021

Leavrth commented Jun 1, 2021

ti-chi-bot commented Jun 1, 2021

glorv Jun 1, 2021

Choose a reason for hiding this comment

Leavrth Jun 2, 2021 • edited Loading

Choose a reason for hiding this comment

ti-chi-bot commented Jun 2, 2021

glorv commented Jun 2, 2021

3pointer commented Jun 2, 2021

ti-chi-bot commented Jun 2, 2021

3pointer commented Jun 2, 2021

Leavrth commented May 31, 2021 •

edited

Loading

YuJuncen commented May 31, 2021 •

edited

Loading

Leavrth Jun 2, 2021 •

edited

Loading