[to #67] fix rawkv backup failure #77
Conversation
Issue Number: tikv#67 Signed-off-by: pingyu <yuping@pingcap.com>
Signed-off-by: pingyu <yuping@pingcap.com>
pingyu
requested review from
zeminzhou and
zz-jason
and removed request for
zeminzhou
Signed-off-by: pingyu <yuping@pingcap.com>
br/Makefile
Outdated
| @@ -118,3 +118,22 @@ clean: | |||
| rm -rf *.out | |||
| rm -rf bin | |||
| rm -rf tools/bin | |||
|
|
|||
| br_integration_test: br_bins build_br build_for_br_integration_test | |||
| @cd br && tests/run.sh | |||
There was a problem hiding this comment.
- would it raise an error if we build the target
br_integration_test? - why name it with
br_prefix? - how about adding this integration test to .github/workflow/ci-br.yml?
Signed-off-by: pingyu <yuping@pingcap.com>
Signed-off-by: pingyu <yuping@pingcap.com>
Signed-off-by: pingyu <yuping@pingcap.com>
Signed-off-by: pingyu <yuping@pingcap.com>
br/tests/br_rawkv/run.py
Outdated
|
|
||
| def parse_args(): | ||
| parser = argparse.ArgumentParser(description="The backup/restore integration test runner for RawKV") | ||
| parser.add_argument("--br", dest = "br", required = True, | ||
| help = "The br binary to be tested.") | ||
| help = "The normal br binary to be tested.") | ||
| parser.add_argument("--br-test", dest = "br_test", required = True, |
There was a problem hiding this comment.
can we only test the br with code coverage and failpoint enabled?
There was a problem hiding this comment.
En... I think yes. I worried overmuch.
br/tests/br_rawkv/run.py
Outdated
| self.pd = global_args.pd | ||
| self.br = global_args.br | ||
| self.br = br | ||
| self.br_test = global_args.br_test |
There was a problem hiding this comment.
seems like it hasn't been used?
br/tests/br_rawkv/run.py
Outdated
| tester.test_rawkv() | ||
|
|
||
| tester_fp = rawkvTester(args.br_test, args, storage_path_suffix="fp", failpoints=FAILPOINT_TIKV_REGION_ERROR) |
There was a problem hiding this comment.
can we set this failpoint in each test case directly? so we don't need to maintain global failpoint test variables.
There was a problem hiding this comment.
I will fix it later.
| cd br | ||
| make test/integration | ||
| br-integration-test-5X: | ||
| name: br-integration-test-5X-${{ matrix.tikv_version }} |
There was a problem hiding this comment.
could you add some comments about why separating integrations for v6.0 and nightly apart from it?
Signed-off-by: pingyu <yuping@pingcap.com>
|
@zz-jason All comments are addressed. PTAL~ |
br/tests/br_rawkv/run.py
Outdated
| # construct command and arguments | ||
| cmd_list = [cmd] | ||
| cmd_list = cmd.split() # `cmd` may contain arguments, so split() to meet requirement of `subprocess.check_output`. |
There was a problem hiding this comment.
cmdmay contain arguments
it breaks the assumption that cmd is used to point to the binary to be executed and args represents all the arguments to the command. Is there other method to maintain this assumption?
There was a problem hiding this comment.
I will try to fix it later.
zz-jason
changed the title
Signed-off-by: pingyu <yuping@pingcap.com>
|
we can merge this PR right now and fix the broken test in the future. |
Signed-off-by: pingyu yuping@pingcap.com
What problem does this PR solve?
Issue Number: to #67
Problem Description: Cherry pick pingcap/tidb#32612 to fix rawkv backup failure
What is changed and how does it work?
Pass
isRawKvto necessary functions.Migrate integration codes to new framework (from
run.shtorun.py).Effectiveness of fail points is verified by log as following:
This line is printed when fail point is trigger.
This line is printed because the fail point cause backup of one of the region failed.
Code changes
No.
No.
No.
No.
No.
Check List for Tests
This PR has been tested by at least one of the following methods:
Side effects
Related changes