feat(general): reduce number of open file descriptors for indexes on Unix #4866

samm81 · 2025-10-04T10:04:05Z

Close file descriptors after mmap on Unix to reduce the total number of open file descriptors.

On Unix-like platforms, close the file descriptor immediately after a successful mmap.Map of .sndx index cache files. This keeps the mapping valid until Unmap (per POSIX semantics) and significantly reduces steady-state FD usage when many indexes are open, helping avoid EMFILE ("too many open files").

Split mmapOpenWithRetry into platform-specific implementations:
- committed_content_index_disk_cache_unix.go (!windows):
  - Map RDONLY, close FD immediately.
  - Return closer that only unmaps.
- committed_content_index_disk_cache_windows.go (windows):
  - Keep FD open until Unmap.
  - Return closer that unmaps and closes FD.
Remove old mmapOpenWithRetry and mmap import from repo/content/committed_content_index_disk_cache.go
Add Linux-only unit test verifying FD count does not grow proportionally: repo/content/committed_content_index_fd_linux_test.go
- Creates N small indexes, opens them all, checks /proc/self/fd delta stays low.

Notes:

Behavior unchanged on Windows due to OS semantics.
Mapping failures close the FD to avoid leaks.
Unlink semantics remain correct; mappings stay valid until Unmap.

samm81 · 2025-10-04T10:05:43Z

trying to solve my own issue #4831 . works ok on my machine locally so I figured I'd push it up if it would help anyone else.

jkowalski · 2025-10-08T01:30:19Z

please rebase and fix merge conflict, we should test this on Mac and Linux and then we can merge

julio-lopez

Thanks for proposing the changes, they are pointing in the right direction.
Please take a look at the inline comments.

Thanks again,

repo/content/committed_content_index_disk_cache_unix.go

repo/content/committed_content_index_fd_linux_test.go

julio-lopez · 2025-10-23T02:14:57Z

repo/content/committed_content_index_fd_linux_test.go

+	// Do not run in parallel to avoid fd count noise from other tests.
+	ctx := testlogging.Context(t)
+	ft := faketime.NewClockTimeWithOffset(0)
+	cache := &diskCommittedContentIndexCache{testutil.TempDirectory(t), ft.NowFunc(), func() int { return 3 }, testlogging.Printf(t.Logf, ""), DefaultIndexCacheSweepAge}


@samm81 This failed to compile in a previous CI job and it is going to fail again due to a type mismatch for the "log" type.

See https://github.com/kopia/kopia/actions/runs/18611420883/job/53293937511?pr=4866

julio-lopez · 2025-10-23T02:19:52Z

repo/content/committed_content_index_fd_linux_test.go

+	if delta := after - before; delta > maxDelta {
+		t.Fatalf("fd count grew too much after opening %d indexes: before=%d after=%d delta=%d (max allowed %d)", N, before, after, delta, maxDelta)
+	}


this can be simplified as:

Suggested change

if delta := after - before; delta > maxDelta {

t.Fatalf("fd count grew too much after opening %d indexes: before=%d after=%d delta=%d (max allowed %d)", N, before, after, delta, maxDelta)

}

require.LessOrEqualf(t, after - before, maxDelta "fd count grew too much after opening %d indexes:", N)

The delta (that is after - before) and maxDelta will be already included in the message when the assertion fails.

julio-lopez · 2025-10-23T02:20:37Z

repo/content/committed_content_index_fd_linux_test.go

+	ft := faketime.NewClockTimeWithOffset(0)
+	cache := &diskCommittedContentIndexCache{testutil.TempDirectory(t), ft.NowFunc(), func() int { return 3 }, testlogging.Printf(t.Logf, ""), DefaultIndexCacheSweepAge}
+
+	const N = 200


nit:

Suggested change

const N = 200

const indexCount = 200

On Unix-like platforms, close the file descriptor immediately after a successful mmap.Map of .sndx index cache files. This keeps the mapping valid until Unmap (per POSIX semantics) and significantly reduces steady-state FD usage when many indexes are open, helping avoid EMFILE ("too many open files"). - Split mmapOpenWithRetry into platform-specific implementations: - committed_content_index_disk_cache_unix.go (!windows): - Map RDONLY, close FD immediately. - Return closer that only unmaps. - committed_content_index_disk_cache_windows.go (windows): - Keep FD open until Unmap. - Return closer that unmaps and closes FD. - Remove old mmapOpenWithRetry and mmap import from repo/content/committed_content_index_disk_cache.go. - Add Linux-only unit test verifying FD count does not grow proportionally: repo/content/committed_content_index_fd_linux_test.go - Creates N small indexes, opens them all, checks /proc/self/fd delta stays low. Notes: - Behavior unchanged on Windows due to OS semantics. - Mapping failures close the FD to avoid leaks. - Unlink semantics remain correct; mappings stay valid until Unmap.

Co-authored-by: Julio Lopez <1953782+julio-lopez@users.noreply.github.com>

Copilot

Pull Request Overview

This PR optimizes file descriptor usage on Unix platforms by closing FDs immediately after memory-mapping index files, while preserving the existing behavior on Windows due to OS-specific requirements.

Key changes:

Splits mmapOpenWithRetry into platform-specific implementations with Unix closing FDs post-mmap and Windows retaining them
Removes the original cross-platform implementation and mmap import from the base file
Adds a Linux-specific test verifying FD count remains low when many indexes are opened

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
repo/content/committed_content_index_disk_cache.go	Removes original mmapOpenWithRetry implementation and mmap-go import
repo/content/committed_content_index_disk_cache_unix.go	New Unix implementation that closes FD immediately after successful mmap
repo/content/committed_content_index_disk_cache_windows.go	New Windows implementation preserving retry logic and keeping FD open
repo/content/committed_content_index_fd_linux_test.go	Adds test verifying FD count doesn't grow proportionally when opening many indexes

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-23T03:58:30Z

repo/content/committed_content_index_disk_cache_unix.go

+func (c *diskCommittedContentIndexCache) mmapOpenWithRetry(_ context.Context, path string) (mmap.MMap, func() error, error) {
+	f, err := os.Open(path) //nolint:gosec
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "unable to open file despite retries")


The error message 'unable to open file despite retries' is misleading on Unix since no retry logic exists in this implementation. Consider changing to a simpler message like 'unable to open file' or 'error opening file for mmap'.

Suggested change

return nil, nil, errors.Wrap(err, "unable to open file despite retries")

return nil, nil, errors.Wrap(err, "unable to open file")

Copilot · 2025-10-23T03:58:30Z

repo/content/committed_content_index_disk_cache_windows.go

+	retryCount := 0
+	for err != nil && retryCount < maxRetries {
+		retryCount++
+		contentlog.Log2(ctx, c.log, "retry unable to mmap.Open()",


[nitpick] Corrected spacing in log message from 'retry unable to mmap.Open()' to 'retry: unable to mmap.Open()' for better readability.

Suggested change

contentlog.Log2(ctx, c.log, "retry unable to mmap.Open()",

contentlog.Log2(ctx, c.log, "retry: unable to mmap.Open()",

codecov · 2025-10-23T04:07:08Z

Codecov Report

❌ Patch coverage is 45.83333% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.71%. Comparing base (cb455c6) to head (cc34ff7).
⚠️ Report is 692 commits behind head on master.

Files with missing lines	Patch %	Lines
...content/committed_content_index_disk_cache_unix.go	45.83%	11 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4866      +/-   ##
==========================================
+ Coverage   75.86%   76.71%   +0.84%     
==========================================
  Files         470      537      +67     
  Lines       37301    40968    +3667     
==========================================
+ Hits        28299    31427    +3128     
- Misses       7071     7494     +423     
- Partials     1931     2047     +116

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

julio-lopez · 2025-10-23T04:17:23Z

@samm81 Thanks for your digging into this issue and for the corresponding improvement.

samm81 mentioned this pull request Oct 4, 2025

failed to open repository: too many open files when accessing repository (index cache size exceeds default limits) #4831

Closed

samm81 force-pushed the reduce-open-fds-unix branch from 8a2f7bd to f682b7d Compare October 9, 2025 08:09

julio-lopez requested changes Oct 13, 2025

View reviewed changes

repo/content/committed_content_index_disk_cache_unix.go Outdated Show resolved Hide resolved

repo/content/committed_content_index_disk_cache_unix.go Show resolved Hide resolved

repo/content/committed_content_index_disk_cache_unix.go Outdated Show resolved Hide resolved

julio-lopez reviewed Oct 21, 2025

View reviewed changes

repo/content/committed_content_index_fd_linux_test.go Outdated Show resolved Hide resolved

julio-lopez reviewed Oct 23, 2025

View reviewed changes

samm81 and others added 10 commits October 22, 2025 19:41

fix: moves comment

c6d1955

remove windows only retry logic under unix

c00381b

Co-authored-by: Julio Lopez <1953782+julio-lopez@users.noreply.github.com>

style: prefer NoError

24feb57

Co-authored-by: Julio Lopez <1953782+julio-lopez@users.noreply.github.com>

remove unused imports

5c25126

remove unused constant

2a7b99d

unused context parameter

be2b713

fix test compilation error

0dbab2b

rename const indexCount

1da45ca

require.LessOrEqual

6d5ae8b

julio-lopez force-pushed the reduce-open-fds-unix branch from 4999af2 to 6d5ae8b Compare October 23, 2025 03:49

remove stale comment

cc34ff7

julio-lopez requested a review from Copilot October 23, 2025 03:57

Copilot AI reviewed Oct 23, 2025

View reviewed changes

julio-lopez changed the title ~~feat(content): close FD after mmap on Unix to reduce open FDs~~ feat(general): reduce number of open file descriptors used for indexes on Unix Oct 23, 2025

julio-lopez changed the title ~~feat(general): reduce number of open file descriptors used for indexes on Unix~~ feat(general): reduce number of open file descriptors for indexes on Unix Oct 23, 2025

julio-lopez approved these changes Oct 23, 2025

View reviewed changes

julio-lopez merged commit 11765d6 into kopia:master Oct 23, 2025
29 of 31 checks passed

Lyndon-Li mentioned this pull request Nov 14, 2025

Velero backups fail due to failing to connect to kopia repo: mmap error: cannot allocate memory vmware-tanzu/velero#8502

Open

	return nil, nil, errors.Wrap(err, "unable to open file despite retries")
	return nil, nil, errors.Wrap(err, "unable to open file")

	contentlog.Log2(ctx, c.log, "retry unable to mmap.Open()",
	contentlog.Log2(ctx, c.log, "retry: unable to mmap.Open()",

feat(general): reduce number of open file descriptors for indexes on Unix #4866

feat(general): reduce number of open file descriptors for indexes on Unix #4866

Uh oh!

Conversation

samm81 commented Oct 4, 2025 • edited by julio-lopez Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samm81 commented Oct 4, 2025

Uh oh!

jkowalski commented Oct 8, 2025

Uh oh!

julio-lopez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julio-lopez Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

julio-lopez Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

julio-lopez Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

julio-lopez commented Oct 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samm81 commented Oct 4, 2025 •

edited by julio-lopez

Loading

codecov bot commented Oct 23, 2025 •

edited

Loading