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

When deleting the cache, unnecessary directories are also deleted. #75

Merged
merged 8 commits into from
Sep 20, 2024

Conversation

k1LoW
Copy link
Member

@k1LoW k1LoW commented Sep 20, 2024

If directories are not deleted, the host's inodes will be exhausted.

NOTICE: This fix does not delete existing empty cache directories.

@k1LoW k1LoW added the bug Something isn't working label Sep 20, 2024
@k1LoW k1LoW self-assigned this Sep 20, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@k1LoW k1LoW requested review from pyama86 and Drumato September 20, 2024 04:59

This comment has been minimized.

This comment has been minimized.

diskcache.go Outdated
dirs++
}
}
if dirs > 1 {
Copy link

Choose a reason for hiding this comment

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

Suggested change
if dirs > 1 {
if dirs > 0 {

じゃないのってなんでですか?

Copy link
Member Author

@k1LoW k1LoW Sep 20, 2024

Choose a reason for hiding this comment

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

自分自身が存在するはずだからです。parentを os.ReadDir して、隣にディレクトリがないかを確認しています。

dirs := 0
for _, e := range entries {
if e.IsDir() {
dirs++
Copy link

Choose a reason for hiding this comment

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

これしきい値上回ったらearly breakして良さそう?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you! ac292c0

Copy link

@pyama86 pyama86 Sep 20, 2024

Choose a reason for hiding this comment

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

dirs > 1 の部分は、parent ディレクトリ内に現在削除対象のディレクトリ以外にもディレクトリが存在するかどうかを確認するための条件です。具体的な理由を説明すると以下の通りです。
このコードの目的は、削除対象のディレクトリ(dir)を、その上位の親ディレクトリに他のディレクトリが存在しない限り、再帰的に削除していくことです。
もし親ディレクトリ(parent)に複数のディレクトリが存在する場合(つまり dirs > 1)、削除対象のディレクトリ(dir)だけを削除します。これは、親ディレクトリには他の重要なディレクトリがまだ存在するため、親ディレクトリ自体を削除しないようにするためです。
一方で、親ディレクトリに削除対象以外のディレクトリがない場合(dirs == 1)、親ディレクトリ自体を再帰的に削除できるので、recursiveRemoveDir を使って上位のディレクトリまで削除を進めます。
簡単にまとめると、dirs > 1 は「親ディレクトリに複数のディレクトリが存在するかどうか」を確認しており、複数存在する場合にはその親ディレクトリを削除せず、対象のディレクトリだけを削除するための条件です。

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

Code Metrics Report

main (cb68696) #75 (769c6b8) +/-
Coverage 76.6% 77.8% +1.1%
Code to Test Ratio 1:1.9 1:1.9 +0.0
Details
  |                    | main (cb68696) | #75 (769c6b8) |  +/-  |
  |--------------------|----------------|---------------|-------|
+ | Coverage           |          76.6% |         77.8% | +1.1% |
  |   Files            |              2 |             2 |     0 |
  |   Lines            |            296 |           316 |   +20 |
+ |   Covered          |            227 |           246 |   +19 |
+ | Code to Test Ratio |          1:1.9 |         1:1.9 |  +0.0 |
  |   Code             |            617 |           647 |   +30 |
+ |   Test             |           1192 |          1275 |   +83 |

Code coverage of files in pull request scope (76.0% → 77.5%)

Files Coverage +/-
diskcache.go 77.5% +1.4%

Reported by octocov

Copy link
Contributor

BenchmarkDiscCache1MBBody-4

main (-) #75 (769c6b8) +/-
Number of iterations 1000 1000 0
Nanoseconds per iteration 199289 ns/op 203293 ns/op 4004 ns/op
Bytes allocated per iteration 69119 B/op 69837 B/op 718 B/op
Allocs per iteration 250 allocs/op 252 allocs/op 2 allocs/op
Metadata
main (-) #75 (769c6b8)
goos linux linux
goarch amd64 amd64
pkg github.com/2manymws/rcutil github.com/2manymws/rcutil
cpu AMD EPYC 7763 64-Core Processor AMD EPYC 7763 64-Core Processor

BenchmarkEncodeDecode1MBBody-4

main (-) #75 (769c6b8) +/-
Number of iterations 1000 1000 0
Nanoseconds per iteration 2967574 ns/op 2975008 ns/op 7434 ns/op
Bytes allocated per iteration 54408 B/op 54375 B/op -33 B/op
Allocs per iteration 67 allocs/op 67 allocs/op 0 allocs/op
Metadata
main (-) #75 (769c6b8)
goos linux linux
goarch amd64 amd64
pkg github.com/2manymws/rcutil github.com/2manymws/rcutil
cpu AMD EPYC 7763 64-Core Processor AMD EPYC 7763 64-Core Processor

BenchmarkNGINXCache1MBBody-4

main (-) #75 (769c6b8) +/-
Number of iterations 1000 1000 0
Nanoseconds per iteration 364330 ns/op 362414 ns/op -1916 ns/op
Bytes allocated per iteration 14265 B/op 14260 B/op -5 B/op
Allocs per iteration 90 allocs/op 90 allocs/op 0 allocs/op
Metadata
main (-) #75 (769c6b8)
goos linux linux
goarch amd64 amd64
pkg github.com/2manymws/rcutil github.com/2manymws/rcutil
cpu AMD EPYC 7763 64-Core Processor AMD EPYC 7763 64-Core Processor

Reported by octocov

@k1LoW k1LoW merged commit 7c3851e into main Sep 20, 2024
4 checks passed
@k1LoW k1LoW deleted the removeUnuseDirs branch September 20, 2024 05:30
@github-actions github-actions bot mentioned this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants