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

benchmark: add lightning import for TPC-C #2169

Merged
merged 7 commits into from
Jan 10, 2020

Conversation

jackysp
Copy link
Member

@jackysp jackysp commented Jan 8, 2020

Signed-off-by: Shuaipeng Yu jackysp@gmail.com

What is changed, added or deleted?

Add lightning import for TPC-C.

What is the related PR or file link(s)?

Which version does your change affect?

dev, 3.0, 3.1

Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
@jackysp
Copy link
Member Author

jackysp commented Jan 8, 2020

I'll update 3.0 and 3.1 when it is ready to merge.

Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

Rest LGTM

dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
Comment on lines 229 to 232
* 在 Importer 目录下创建一个 log 文件夹
* 在 Importer 目录下执行 `scripts/start_importer.sh`,启动 Importer
* 在 Lightning 目录下创建一个 log 文件夹
* 在 Lightning 目录下执行 `scripts/start_lightning.sh`,开始导入数据
Copy link
Contributor

@kennytm kennytm Jan 8, 2020

Choose a reason for hiding this comment

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

The log folders should be created automatically by the shell scripts 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

All directorys should be created automatically. I'll check it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

great

jackysp and others added 2 commits January 8, 2020 13:12
Co-Authored-By: kennytm <kennytm@gmail.com>
Co-Authored-By: kennytm <kennytm@gmail.com>

```yaml
tikv_importer:
addr: "172.16.5.34:13323" # 这里指向 importer 的地址,需要将上面 inventory.ini 里指定的再写一次
Copy link
Member Author

Choose a reason for hiding this comment

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

Could we improve this? I mean it doesn't need to set IP:port once again. @kennytm @liubo0127

This comment was marked as off-topic.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be configured automatically.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have tested, and it can be configured automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

企业微信截图_20200110102808
企业微信截图_20200110102653

Copy link
Member Author

Choose a reason for hiding this comment

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

port does not set automatically.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, it's actually set automatically, but it doesn't match the custom port.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

great, thx

@lilin90 lilin90 added size/large Changes of a large size. translation/doing This PR’s assignee is translating this PR. labels Jan 8, 2020
Copy link
Contributor

@yikeke yikeke left a comment

Choose a reason for hiding this comment

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

Some small editing advice on improving the language. Rest LGTM.

dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
dev/benchmark/how-to-run-tpcc.md Outdated Show resolved Hide resolved
Co-Authored-By: Keke Yi <40977455+yikeke@users.noreply.github.com>
Copy link
Contributor

@yikeke yikeke left a comment

Choose a reason for hiding this comment

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

Language LGTM

Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
@jackysp
Copy link
Member Author

jackysp commented Jan 10, 2020

All comments addressed, and apply to v3.0 and v3.1, PTAL @yikeke

Copy link
Contributor

@yikeke yikeke left a comment

Choose a reason for hiding this comment

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

LGTM

@yikeke yikeke added the status/can-merge Indicates a PR has been approved by a committer. label Jan 10, 2020
@claassistantio
Copy link

claassistantio commented Jan 10, 2020

CLA assistant check
All committers have signed the CLA.

@sre-bot
Copy link
Contributor

sre-bot commented Jan 10, 2020

/run-all-tests

@jackysp
Copy link
Member Author

jackysp commented Jan 10, 2020

... the bot blocks CLA.

@yikeke
Copy link
Contributor

yikeke commented Jan 10, 2020

... the bot blocks CLA.

I asked the bot's manager @you06 to sign it. Wait a moment.

@you06
Copy link
Contributor

you06 commented Jan 10, 2020

Facing a bot-login issue, we're trying to solve this.

@sre-bot sre-bot merged commit 0b052a7 into pingcap:master Jan 10, 2020
@yikeke yikeke assigned toutdesuite and unassigned yikeke Mar 4, 2020
@yikeke yikeke added priority/P1 The issue has P1 priority. and removed priority/P1 The issue has P1 priority. labels Mar 4, 2020
@jackysp jackysp deleted the tpcc_lightning branch March 18, 2020 06:16
@toutdesuite toutdesuite added translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. and removed translation/doing This PR’s assignee is translating this PR. labels Mar 19, 2020
LS1 ansible_host=172.16.5.34 deploy_dir=/data2/ls1 tidb_lightning_pprof_port=23323 data_source_dir=/home/user/csv
```

##### 修改 conf/tidb-lightning.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

not toml? @kennytm

Copy link
Contributor

Choose a reason for hiding this comment

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

YAML is correct, Ansible uses YAML as template. 😓 @july2993

Copy link
Contributor

Choose a reason for hiding this comment

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

I methinks this is the config file lightning use directly 😓

@jackysp
Copy link
Member Author

jackysp commented Mar 24, 2020

The issue found in this PR is fixed by pingcap/tidb-ansible#1105, pingcap/tidb-ansible#1104 and pingcap/tidb-ansible#1103

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/large Changes of a large size. status/can-merge Indicates a PR has been approved by a committer. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants