Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use -toolexec to avoid manual preprocessing #81

Merged
merged 5 commits into from Apr 11, 2024
Merged

use -toolexec to avoid manual preprocessing #81

merged 5 commits into from Apr 11, 2024

Conversation

lance6716
Copy link
Contributor

@lance6716 lance6716 commented Apr 11, 2024

What problem does this PR solve?

don't need to use failpoint-ctl enable to manually preprocess the souce codes

What is changed and how it works?

use -toolexec

Check List

Tests

  • Manual test (add detailed scripts or steps below)

in tidb repo, this test will fail if failpoint is not enabled

# lance6716 @ lances-MacBook-Air in ~/Projects/tidb on git:master o [14:56:53]
$ go test  ./pkg/util/cpu/ -test.run TestFailpointCPUValue
--- FAIL: TestFailpointCPUValue (0.20s)
    cpu_test.go:93:
        	Error Trace:	/Users/lance6716/Projects/tidb/pkg/util/cpu/cpu_test.go:93
        	Error:      	Should be true
        	Test:       	TestFailpointCPUValue
FAIL
FAIL	github.com/pingcap/tidb/pkg/util/cpu	0.662s
FAIL

with the new failpoint-toolexec it can pass, which proves it works.

# lance6716 @ lances-MacBook-Air in ~/Projects/tidb on git:master o [14:56:19]
$ GOCACHE=/tmp/failpoint-cache go test  ./pkg/util/cpu/ -test.run TestFailpointCPUValue -toolexec /tmp/failpoint-toolexec -test.v
=== RUN   TestFailpointCPUValue
[2024/04/11 14:56:51.491 +08:00] [ERROR] [cpu.go:67] [GetCgroupCPU] [error=mockAddBatchDDLJobsErr] [errorVerbose="mockAddBatchDDLJobsErr\ngithub.com/pingcap/tidb/pkg/util/cgroup.GetCgroupCPU\n\t/var/folders/c_/dth_nd_53f5d4yw5h5tyfj2h0000gn/T/failpoint-toolexec/github.com/pingcap/tidb/pkg/util/cgroup/cgroup_cpu_unsupport.go:32\ngithub.com/pingcap/tidb/pkg/util/cpu.(*Observer).Start\n\t/var/folders/c_/dth_nd_53f5d4yw5h5tyfj2h0000gn/T/failpoint-toolexec/github.com/pingcap/tidb/pkg/util/cpu/cpu.go:64\ngithub.com/pingcap/tidb/pkg/util/cpu_test.TestFailpointCPUValue\n\t/Users/lance6716/Projects/tidb/pkg/util/cpu/cpu_test.go:89\ntesting.tRunner\n\t/opt/homebrew/Cellar/go/1.21.2/libexec/src/testing/testing.go:1595\nruntime.goexit\n\t/opt/homebrew/Cellar/go/1.21.2/libexec/src/runtime/asm_arm64.s:1197"] [stack="github.com/pingcap/tidb/pkg/util/cpu.(*Observer).Start\n\t/var/folders/c_/dth_nd_53f5d4yw5h5tyfj2h0000gn/T/failpoint-toolexec/github.com/pingcap/tidb/pkg/util/cpu/cpu.go:67\ngithub.com/pingcap/tidb/pkg/util/cpu_test.TestFailpointCPUValue\n\t/Users/lance6716/Projects/tidb/pkg/util/cpu/cpu_test.go:89\ntesting.tRunner\n\t/opt/homebrew/Cellar/go/1.21.2/libexec/src/testing/testing.go:1595"]
--- PASS: TestFailpointCPUValue (2.00s)
PASS
ok  	github.com/pingcap/tidb/pkg/util/cpu	2.337s

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

Signed-off-by: lance6716 <lance6716@gmail.com>
Signed-off-by: lance6716 <lance6716@gmail.com>
Signed-off-by: lance6716 <lance6716@gmail.com>
@lance6716 lance6716 changed the title [WIP]use -toolexec to avoid manual preprocessing use -toolexec to avoid manual preprocessing Apr 11, 2024
Signed-off-by: lance6716 <lance6716@gmail.com>
Signed-off-by: lance6716 <lance6716@gmail.com>
Copy link

codecov bot commented Apr 11, 2024

Codecov Report

Merging #81 (1fbf934) into master (2eaa328) will decrease coverage by 8.0701%.
The diff coverage is 3.1250%.

Additional details and impacted files
@@               Coverage Diff                @@
##             master        #81        +/-   ##
================================================
- Coverage   84.4318%   76.3617%   -8.0701%     
================================================
  Files             7          8         +1     
  Lines           880        973        +93     
================================================
  Hits            743        743                
- Misses           84        177        +93     
  Partials         53         53                

@lance6716 lance6716 requested a review from tisonkun April 11, 2024 08:04
Copy link
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@tisonkun tisonkun merged commit 411cc9a into master Apr 11, 2024
4 checks passed
@tisonkun tisonkun deleted the toolexec branch April 11, 2024 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants