Skip to content

Fix download cache bloat and stale disk capacity reporting causing Diego cell disk full #1163

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.

Sign up for GitHub

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

Merged
ameowlia merged 3 commits into from
Jul 2, 2026
Merged

Fix download cache bloat and stale disk capacity reporting causing Diego cell disk full #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97442e3
fix: extend makeRoom() to evict based on real partition free space
kart2bc Jun 26, 2026
5c840dc
fix: move live Statfs to RemainingResources; use *uint64 for min-free…
kart2bc Jun 26, 2026
a9ee371
fix gh actions based on the new repo modules
kart2bc Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions .github/workflows/tests-workflow.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,6 @@ jobs:
- name: build binaries
run: |
repo/.github/helpers/build.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: bbs-mysql
env:
DIR: src/code.cloudfoundry.org/bbs
DB: mysql
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: auctioneer-mysql
env:
DIR: src/code.cloudfoundry.org/auctioneer
Expand All @@ -162,12 +156,6 @@ jobs:
DB: mysql
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: locket-mysql
env:
DIR: src/code.cloudfoundry.org/locket
DB: mysql
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: cfdot-mysql
env:
DIR: src/code.cloudfoundry.org/cfdot
Expand Down Expand Up @@ -239,12 +227,6 @@ jobs:
- name: build binaries
run: |
repo/.github/helpers/build.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: bbs-postgres
env:
DIR: src/code.cloudfoundry.org/bbs
DB: postgres
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: auctioneer-postgres
env:
DIR: src/code.cloudfoundry.org/auctioneer
Expand All @@ -263,12 +245,6 @@ jobs:
DB: postgres
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: locket-postgres
env:
DIR: src/code.cloudfoundry.org/locket
DB: postgres
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: cfdot-postgres
env:
DIR: src/code.cloudfoundry.org/cfdot
Expand Down Expand Up @@ -296,12 +272,6 @@ jobs:
- name: build binaries
run: |
repo/.github/helpers/build.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: bbs-mysql
env:
DIR: src/code.cloudfoundry.org/bbs
DB: mysql
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: auctioneer-mysql
env:
DIR: src/code.cloudfoundry.org/auctioneer
Expand All @@ -320,12 +290,6 @@ jobs:
DB: mysql
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: locket-mysql
env:
DIR: src/code.cloudfoundry.org/locket
DB: mysql
run: |
repo/.github/helpers/test.bash ${{ github.workspace }} "$BUILD_IMAGE"
- name: cfdot-mysql
env:
DIR: src/code.cloudfoundry.org/cfdot
Expand Down
3 changes: 3 additions & 0 deletions jobs/rep/spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ properties:
diego.executor.max_cache_size_in_bytes:
description: "maximum size of the cache in bytes - this should leave a healthy overhead for temporary items, etc."
default: 10000000000
diego.executor.min_cache_partition_free_bytes:
Comment thread
kart2bc marked this conversation as resolved.
description: "Minimum free space in bytes that must remain on the disk where the download cache lives. When actual disk free space falls below this threshold, the cache deletes old downloaded files to recover space. Set to 0 to disable this behavior."
default: 5368709120
diego.executor.use_schedulable_disk_size:
description: "Use total space available to containers reported by Garden. If false the total size of image plugin store minus max_cache_size_in_bytes is used."
default: false
Expand Down
2 changes: 2 additions & 0 deletions jobs/rep/templates/rep.json.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@
raise "additional_memory_allocation_mb cannot be negative"
end

config[:min_cache_partition_free_bytes] = p("diego.executor.min_cache_partition_free_bytes")

if_p("diego.executor.volman.driver_paths") do |value|
config[:volman_driver_paths] = value
end
Expand Down
3 changes: 3 additions & 0 deletions jobs/rep_windows/spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ properties:
diego.executor.max_cache_size_in_bytes:
description: "maximum size of the cache in bytes - this should leave a healthy overhead for temporary items, etc."
default: 10000000000
diego.executor.min_cache_partition_free_bytes:
Comment thread
kart2bc marked this conversation as resolved.
description: "EXPERIMENTAL: Additional memory allocated to each container for the envoy proxy. This must not be negative. Currently doesn't work on windows cells but left here for compatability with the linux Rep."
default: 5368709120
diego.executor.use_schedulable_disk_size:
Comment thread
kart2bc marked this conversation as resolved.
description: "Use total space available to containers reported by Garden. If false the total size of image plugin store minus max_cache_size_in_bytes is used."
default: false
Expand Down
2 changes: 2 additions & 0 deletions jobs/rep_windows/templates/rep.json.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@
raise "additional_memory_allocation_mb cannot be negative"
end

config[:min_cache_partition_free_bytes] = p("diego.executor.min_cache_partition_free_bytes")

if_p("diego.executor.volman.driver_paths") do |value|
config[:volman_driver_paths] = value
end
Expand Down
11 changes: 11 additions & 0 deletions spec/rep_template_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,17 @@
expect(JSON.parse(rendered_template)['sidecar_root_fs']).to eq('cflinuxfs4')
end
end

context 'min_cache_partition_free_bytes' do
it 'defaults to 5368709120 (5GB)' do
expect(JSON.parse(rendered_template)['min_cache_partition_free_bytes']).to eq(5_368_709_120)
end

it 'is configurable' do
deployment_manifest_fragment['diego']['executor']['min_cache_partition_free_bytes'] = 1_073_741_824
expect(JSON.parse(rendered_template)['min_cache_partition_free_bytes']).to eq(1_073_741_824)
end
end
end

describe 'setup_mounted_data_dirs.erb' do
Expand Down
11 changes: 11 additions & 0 deletions spec/rep_windows_template_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,5 +123,16 @@
expect(JSON.parse(rendered_template)['sidecar_root_fs']).to eq('cflinuxfs4')
end
end

context 'min_cache_partition_free_bytes' do
it 'defaults to 5368709120 (5GB)' do
expect(JSON.parse(rendered_template)['min_cache_partition_free_bytes']).to eq(5_368_709_120)
end

it 'is configurable' do
deployment_manifest_fragment['diego']['executor']['min_cache_partition_free_bytes'] = 1_073_741_824
expect(JSON.parse(rendered_template)['min_cache_partition_free_bytes']).to eq(1_073_741_824)
end
end
end
end
8 changes: 4 additions & 4 deletions src/code.cloudfoundry.org/cacheddownloader/cached_downloader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var _ = Describe("File cache", func() {

transformer = cacheddownloader.NoopTransform

cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes)
cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes, 0)
downloader = cacheddownloader.NewDownloader(1*time.Second, MAX_CONCURRENT_DOWNLOADS, nil)
cachedDownloader, err = cacheddownloader.New(downloader, cache, transformer)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -89,7 +89,7 @@ var _ = Describe("File cache", func() {
Describe("When a new CachedDownloader fails to create a temp directory for the cache", func() {
It("returns an error", func() {
cachedPath = ""
cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes)
cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes, 0)
downloader = cacheddownloader.NewDownloader(1*time.Second, MAX_CONCURRENT_DOWNLOADS, nil)
cachedDownloader, err = cacheddownloader.New(downloader, cache, transformer)
Expect(err).To(HaveOccurred())
Expand Down Expand Up @@ -1048,7 +1048,7 @@ var _ = Describe("File cache", func() {
ghttp.RespondWith(http.StatusOK, string(downloadContent), returnedHeader),
))

cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes)
cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes, 0)
cachedDownloader, err = cacheddownloader.New(downloader, cache, cacheddownloader.TarTransform)
Expect(err).ToNot(HaveOccurred())

Expand Down Expand Up @@ -1321,7 +1321,7 @@ var _ = Describe("File cache", func() {
Expect(cachedDownloader.SaveState(logger)).To(Succeed())

maxSizeInBytes = 32
cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes)
cache = cacheddownloader.NewCache(cachedPath, maxSizeInBytes, 0)
})

It("should evict old entries from the cache", func() {
Expand Down
39 changes: 21 additions & 18 deletions src/code.cloudfoundry.org/cacheddownloader/file_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ var (
)

type FileCache struct {
CachedPath string
maxSizeInBytes int64
minFreeBytes int64
Entries map[string]*FileCacheEntry
OldEntries map[string]*FileCacheEntry
Seq uint64
CachedPath string
maxSizeInBytes int64
minimumPartitionFreeBytes int64
FreeSpaceFunc func(path string) int64 `json:"-"`
Entries map[string]*FileCacheEntry
OldEntries map[string]*FileCacheEntry
Seq uint64
}

type FileCacheEntry struct {
Expand All @@ -41,18 +42,15 @@ type FileCacheEntry struct {
fileInUseCount int
}

func NewCache(dir string, maxSizeInBytes int64, minFreeBytes ...int64) *FileCache {
var minFree int64
if len(minFreeBytes) > 0 {
minFree = minFreeBytes[0]
}
func NewCache(dir string, maxSizeInBytes, minimumPartitionFreeBytes int64) *FileCache {
return &FileCache{
CachedPath: dir,
maxSizeInBytes: maxSizeInBytes,
minFreeBytes: minFree,
Entries: map[string]*FileCacheEntry{},
OldEntries: map[string]*FileCacheEntry{},
Seq: 0,
CachedPath: dir,
maxSizeInBytes: maxSizeInBytes,
minimumPartitionFreeBytes: minimumPartitionFreeBytes,
FreeSpaceFunc: defaultFreeSpaceOnPartition,
Entries: map[string]*FileCacheEntry{},
OldEntries: map[string]*FileCacheEntry{},
Seq: 0,
}
}

Expand Down Expand Up @@ -193,6 +191,11 @@ func (e *FileCacheEntry) expandedDirectory() (string, error) {
err = os.RemoveAll(e.FilePath)
if err != nil {
Comment thread
kart2bc marked this conversation as resolved.
fmt.Fprintln(os.Stderr, "Unable to delete the cached file", err)
} else {
// GetDirectory doubled Size before calling here; tar is now gone so halve it
// back to reflect only the .tar.d on disk. Mirrors the symmetric halving in
// decrementFileInUseCount() which fires when the tar is deleted later.
e.Size = e.Size / 2
}
}
}
Expand Down Expand Up @@ -384,7 +387,7 @@ func (c *FileCache) updateOldEntries(logger lager.Logger, cacheKey string, entry

func (c *FileCache) makeRoom(logger lager.Logger, size int64, excludedCacheKey string) {
usedSpace := c.usedSpace(logger)
for c.maxSizeInBytes < usedSpace+size {
for c.maxSizeInBytes < usedSpace+size || (c.minimumPartitionFreeBytes > 0 && c.FreeSpaceFunc(c.CachedPath) < c.minimumPartitionFreeBytes) {
var oldestEntry *FileCacheEntry
oldestAccessTime, oldestCacheKey := maxTime(), ""
for ck, f := range c.Entries {
Expand Down
Loading
Loading