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

Add more make targets: `clean/no-push-down/push-down-without-v… #115

Merged
merged 6 commits into from
Dec 30, 2019

Conversation

iosmanthus
Copy link
Member

This pull requests refactor the copr-test inorder to make reproducing a failed SQL more conveniently.

@iosmanthus iosmanthus requested review from lonng and breezewish and removed request for lonng December 27, 2019 07:35
README.md Outdated
@@ -5,8 +5,6 @@ executes TiDB push-down executors to improve performance.

## Push Down Test

`./push-down-test`
Copy link
Member

Choose a reason for hiding this comment

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

revert

README.md Outdated
@@ -36,7 +34,7 @@ TiDB side for now.
### Test Cases

We have already added some test cases generated by [randgen] in the push-down-test. Feel free to
Copy link
Member

Choose a reason for hiding this comment

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

revert

```

If you want to filter some test cases:

```
include=1_arith_1.sql make push-down-test
Copy link
Member

Choose a reason for hiding this comment

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

revert

@@ -1 +0,0 @@
/bin/
Copy link
Member

Choose a reason for hiding this comment

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

revert

Copy link
Member Author

Choose a reason for hiding this comment

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

move to push-down-test/build

@@ -0,0 +1,366 @@
# GO environment variables
Copy link
Member

Choose a reason for hiding this comment

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

revert the rename so that github diff will work

Copy link
Member

Choose a reason for hiding this comment

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

Agreed

…sh-down-with-vec

Signed-off-by: Iosmanthus Teng <myosmanthustree@gmail.com>
Signed-off-by: Iosmanthus Teng <myosmanthustree@gmail.com>
README.md Outdated
@@ -36,7 +34,7 @@ TiDB side for now.
### Test Cases

We have already added some test cases generated by [randgen] in the push-down-test. Feel free to
add new ones. Your test case should be placed in the `push-down-test/sql` directory and ends with
add new ones. Your test case should be placed in the `sql` directory and ends with
Copy link
Member

Choose a reason for hiding this comment

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

revert?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -3,7 +3,7 @@
*.swp
.DS_Store

*.log
**.log
Copy link
Member

Choose a reason for hiding this comment

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

**?

iosmanthus and others added 2 commits December 30, 2019 14:39
Signed-off-by: Iosmanthus Teng <myosmanthustree@gmail.com>

"$tikv_bin" -C "$push_down_with_batch_config_dir"/tikv.toml --log-file "$tikv_with_batch_log_file" -L ${log_level} &
tikv_with_batch_pid=$!
# Return the PID of the new PD process
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Return the PID of the new PD process
# Return the PID of the new TiKV process

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

export GO_FAILPOINTS="github.com/pingcap/tidb/expression/PushDownTestSwitcher=return(\"$push_down_func_list\");github.com/pingcap/tidb/expression/PanicIfPbCodeUnspecified=return(true)"
"$tidb_bin" -log-file "$push_down_with_batch_tidb_log_file" -config "$push_down_with_batch_config_dir"/tidb.toml -L ${log_level} &
tidb_push_down_with_batch_pid=$!
# Return the PID of the new PD process
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

}

function build_tester() {
echo
echo "+ Building Push Down Tester"
echo " - Building from ${push_down_test_bin}"
echo "+Building Push Down Tester"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "+Building Push Down Tester"
echo "+ Building Push Down Tester"

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


clean_all_proc
exit $exit_code

Copy link
Member

Choose a reason for hiding this comment

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

Remove this empty line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Iosmanthus Teng <myosmanthustree@gmail.com>
@iosmanthus
Copy link
Member Author

@lonng PTAL

@lonng
Copy link
Member

lonng commented Dec 30, 2019

@iosmanthus The prepare step has executed in the run_tests.sh scripts. It's better to remove the prepare logic in main.go

@iosmanthus iosmanthus changed the title Add more make targets: clean/no-push-down/push-down-without-vec/push-down-with-vec Add more make targets: `clean/no-push-down/push-down-without-v… Dec 30, 2019
@iosmanthus iosmanthus merged commit 0ba0954 into tikv:master Dec 30, 2019
@iosmanthus iosmanthus deleted the split-tests branch December 30, 2019 10:13
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.

3 participants