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

mkdir for raftdb-path #2198

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

mkdir for raftdb-path #2198

wants to merge 5 commits into from

Conversation

Smityz
Copy link
Contributor

@Smityz Smityz commented May 8, 2023

  - host: x.x.x.x
    port: 23000
    status_port: 23001
    data_dir: "/mnt/disk/1/x/tikv/23000"
    ignore_exporter: true
    config:
      server.labels: { host: "x.x.x.x" }
      raftstore.raftdb-path: "/mnt/disk/0/x/tikv/23000"

If we set raftstore.raftdb-path to a location different from data_dir, we need to create a folder for raftstore.raftdb-path in TiUP.
If community wants to merge this PR, I will add some unit test

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 8, 2023

[REVIEW NOTIFICATION]

This pull request has not been approved.

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.

@ti-chi-bot ti-chi-bot bot requested review from breezewish and lucklove May 8, 2023 11:11
@ti-chi-bot ti-chi-bot bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 8, 2023
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 90.91% and project coverage change: -11.14 ⚠️

Comparison is base (035b6af) 56.40% compared to head (5b703b2) 45.26%.

❗ Current head 5b703b2 differs from pull request most recent head 542a397. Consider uploading reports for the commit 542a397 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2198       +/-   ##
===========================================
- Coverage   56.40%   45.26%   -11.14%     
===========================================
  Files         318      317        -1     
  Lines       33465    33464        -1     
===========================================
- Hits        18873    15145     -3728     
- Misses      12351    16285     +3934     
+ Partials     2241     2034      -207     
Flag Coverage Δ
cluster 37.69% <90.91%> (-7.59%) ⬇️
dm 24.41% <36.36%> (-1.67%) ⬇️
tiup 16.00% <0.00%> (-0.03%) ⬇️
unittest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/cluster/spec/tikv.go 52.92% <85.71%> (-9.17%) ⬇️
pkg/cluster/manager/deploy.go 67.69% <100.00%> (-0.91%) ⬇️
pkg/cluster/spec/instance.go 63.37% <100.00%> (-0.94%) ⬇️

... and 120 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@codecov-commenter
Copy link

codecov-commenter commented May 8, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (c53895c) 55.53% compared to head (3b0d877) 50.22%.
Report is 23 commits behind head on master.

❗ Current head 3b0d877 differs from pull request most recent head 5db2cdb. Consider uploading reports for the commit 5db2cdb to get more accurate results

Files Patch % Lines
pkg/cluster/spec/tikv.go 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2198      +/-   ##
==========================================
- Coverage   55.53%   50.22%   -5.31%     
==========================================
  Files         325      324       -1     
  Lines       34757    34757              
==========================================
- Hits        19300    17455    -1845     
- Misses      13144    15004    +1860     
+ Partials     2313     2298      -15     
Flag Coverage Δ
cluster 44.75% <92.31%> (-0.15%) ⬇️
dm 25.45% <46.15%> (-0.04%) ⬇️
playground 15.24% <0.00%> (-0.01%) ⬇️
tiup 33.48% <ø> (ø)
unittest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 13, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kaaaaaaang for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@@ -461,6 +462,15 @@ func (i *TiKVInstance) PostRestart(ctx context.Context, topo Topology, tlsCfg *t
return nil
}

func (i *TiKVInstance) ExtraDirs() []string {
configMap := reflect.Indirect(reflect.ValueOf(i.InstanceSpec)).FieldByName("Config").Interface().(map[string]interface{})
Copy link
Member

Choose a reason for hiding this comment

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

How about:

spec := i.InstanceSpec.(*TiKVSpec)
raftDir := spec.Config["raftstore.raftdb-path"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@ti-chi-bot ti-chi-bot bot added the lgtm label Dec 18, 2023
Copy link
Contributor

ti-chi-bot bot commented Dec 18, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-12-18 02:19:43.482467671 +0000 UTC m=+841074.519694596: ☑️ agreed by breezewish.

@breezewish
Copy link
Member

Thanks! @lucklove PTAL~

@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2024
Copy link
Contributor

ti-chi-bot bot commented Feb 3, 2024

PR needs rebase.

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 kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants