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

cluster: refine log_dir handling on clean (part 1) #981

Merged
merged 3 commits into from
Dec 14, 2020

Conversation

AstroProfundis
Copy link
Contributor

What problem does this PR solve?

Part of #947

What is changed and how it works?

  • Mark experimental features more visible
  • Check for paths to avoid deleting files if data dir is a sub path of log dir
  • Only delete *.log when deleting log files

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Code changes

  • Has persistent data change

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

Release notes:

cluster: adjust checking process when cleaning log files

@AstroProfundis AstroProfundis added type/enhancement Categorizes issue or PR as related to an enhancement. type/bug Categorizes issue as related to a bug. category/usability Categorizes issue or PR as a usability enhancement. category/stability Categorizes issue or PR as a stability enhancement. labels Dec 10, 2020
@AstroProfundis AstroProfundis self-assigned this Dec 10, 2020
@ti-chi-bot ti-chi-bot requested a review from july2993 December 10, 2020 10:34
@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 10, 2020
@codecov-io
Copy link

codecov-io commented Dec 14, 2020

Codecov Report

Merging #981 (f33a73d) into master (01bf92c) will decrease coverage by 25.46%.
The diff coverage is 85.71%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #981       +/-   ##
===========================================
- Coverage   55.40%   29.93%   -25.47%     
===========================================
  Files         279      265       -14     
  Lines       19709    18358     -1351     
===========================================
- Hits        10919     5495     -5424     
- Misses       7065    12014     +4949     
+ Partials     1725      849      -876     
Flag Coverage Δ
cluster ?
dm ?
integrate 21.52% <0.00%> (-28.14%) ⬇️
playground 20.28% <0.00%> (-0.01%) ⬇️
tiup 16.45% <0.00%> (-0.01%) ⬇️
unittest 23.10% <85.71%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
pkg/set/string_set.go 50.00% <0.00%> (-22.23%) ⬇️
components/cluster/command/clean.go 64.51% <100.00%> (-21.70%) ⬇️
components/cluster/command/deploy.go 32.50% <100.00%> (-35.00%) ⬇️
components/cluster/command/root.go 26.50% <100.00%> (-19.28%) ⬇️
components/cluster/command/scale_out.go 30.55% <100.00%> (-58.34%) ⬇️
components/dm/command/root.go 31.96% <100.00%> (-23.04%) ⬇️
pkg/meta/paths.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/utils/utils.go 0.00% <0.00%> (-100.00%) ⬇️
components/dm/main.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/logger/log/log.go 0.00% <0.00%> (-100.00%) ⬇️
... and 178 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01bf92c...f33a73d. Read the comment docs.

@lucklove
Copy link
Member

/lgtm

@lucklove
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

@lucklove: adding 'status/can-merge' to this PR must have 1 LGTMs

In response to this:

/merge

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 tidb-community-bots/ti-community-prow repository.

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 14, 2020
@lucklove
Copy link
Member

/lgtm

@lucklove
Copy link
Member

/merge

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 14, 2020
@ti-chi-bot
Copy link
Member

Can merge label has been added.

Git tree hash: f33a73d

@ti-chi-bot
Copy link
Member

@AstroProfundis: Your PR has out-of-dated and I have automatically updated it for you.

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 tidb-community-bots/ti-community-prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/stability Categorizes issue or PR as a stability enhancement. category/usability Categorizes issue or PR as a usability enhancement. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/bug Categorizes issue as related to a bug. type/enhancement Categorizes issue or PR as related to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants