Repository navigation
gocached: skip the cleanup intent for blobs already in the main dir - #8
Merged
Merged
Conversation
On ci-gocached-aws-1 the put queue's copies into its S3 Files NFS mount top out at about 95 blobs/s no matter how many movers run: 8, an adaptive governor, and 128 all hit the same ceiling. The kernel's per-mount NFS statistics, sampled while the mover backlog was 14,000 to 24,000 deep, show why. Creates and removes each take about 5.3ms round trip with no client-side slot queueing, but by Little's law only 1.15 of them are in progress on average while over 100 movers are blocked in them, and goroutine profiles put every blocked mover in the cleanup intent directory. The mutations of that one directory run one at a time, and each blob costs two of them (the intent's create and its later unlink), so 1/(2 x 5.3ms) is about 95 blobs/s. The same sample shows about 20 renames/s against about 100 copies/s. Every real copy renames its temp file into place, so roughly 80% of spooled blobs are already present in the main directory, and copyToMain returns early after its stat. They still pay for the intent, which protects nothing: it exists so the sweeper can reap a blob this PUT created if the PUT never commits, and this PUT creates nothing. Whether an existing blob survives a sweep is decided by SQLite accounting and the pending-PUT check, which already cover the PUT until its metadata commits. So stat the target before writing the intent, and skip both the intent and the copy when the blob is there with the expected size. That removes both cleanup-directory mutations for most spooled blobs. The real copies that remain cost two mutations of their own (the temp file's create in the main directory's root and its rename out of it) besides the intent's two. Count the skips in gocached_put_queue_copy_skips, a subset of gocached_put_queue_copy_duration_seconds_count, so the share of already-present blobs is measured rather than inferred from NFS op counts. Updates tailscale/corp#47471 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> Change-Id: I6ff2aaf9b54aff13c8aa4c10e68a983b627fcdce
bradfitz
added a commit
that referenced
this pull request
Oct 8, 2026
On ci-gocached-aws-1's S3 Files NFS mount, the kernel serializes creates, unlinks, and renames within a directory, holding the directory's lock for each operation's server round trip of about 5ms. A benchmark measured it directly: with 64 goroutines creating and unlinking files in one directory, 62 were parked on the directory lock and about one was in an NFS RPC, for 96 creates+unlinks a second at any concurrency, while the same load over 256 directories reached 3,300 a second with 47 RPCs in flight. That one-directory ceiling is the put queue's: every copy into the main blob directory created and later unlinked a cleanup intent in the single .cleanup directory, and created its temp file in the main directory's root before renaming it out. The change in #8 stops writing intents for the most common case, blobs already present. For the blobs that are really copied, spread the remaining mutations over the 256 shards the main directory already uses: - Intents go in .cleanup/<xx>/, xx being the first two hex digits of the blob's name, rather than directly in .cleanup. - The temp file is created in the blob's own shard directory instead of the main directory's root, so its rename into place stays within that one directory and locks nothing else. Both directories are created on demand: a create is tried first and the directory made only if it fails with ENOENT, as renameCreatingDir did for the rename, which no longer needs that since the temp file now lives in the target's directory. The sweeper lists .cleanup and then each shard subdirectory in it, sweeping intents in both the new layout and, for upgrades, the old flat one. That's 257 directory listings a minute instead of one, each locking only its own subdirectory, and only briefly. Nothing else lists the main directory's shard directories (usage stats come from SQLite, and GETs and eviction use exact names), so a temp file appearing in one while a copy is in flight changes nothing. Rolling back to an older version after this one has run would leave intents in shard subdirectories unswept, and the old sweeper would log each subdirectory as a malformed intent once a minute. Updates tailscale/corp#47471 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> Change-Id: Icadb72b048c11f6e58c3332903a50438c561a283
bradfitz
added a commit
that referenced
this pull request
Oct 8, 2026
On ci-gocached-aws-1's S3 Files NFS mount, the kernel serializes creates, unlinks, and renames within a directory, holding the directory's lock for each operation's server round trip of about 5ms. A benchmark measured it directly: with 64 goroutines creating and unlinking files in one directory, 62 were parked on the directory lock and about one was in an NFS RPC, for 96 creates+unlinks a second at any concurrency, while the same load over 256 directories reached 3,300 a second with 47 RPCs in flight. That one-directory ceiling is the put queue's: every copy into the main blob directory created and later unlinked a cleanup intent in the single .cleanup directory, and created its temp file in the main directory's root before renaming it out. The change in #8 stops writing intents for the most common case, blobs already present. For the blobs that are really copied, spread the remaining mutations over the 256 shards the main directory already uses: - Intents go in .cleanup/<xx>/, xx being the first two hex digits of the blob's name, rather than directly in .cleanup. - The temp file is created in the blob's own shard directory instead of the main directory's root, so its rename into place stays within that one directory and locks nothing else. Both directories are created on demand: a create is tried first and the directory made only if it fails with ENOENT, as renameCreatingDir did for the rename, which no longer needs that since the temp file now lives in the target's directory. The sweeper lists .cleanup and then each shard subdirectory in it, sweeping intents in both the new layout and, for upgrades, the old flat one. That's 257 directory listings a minute instead of one, each locking only its own subdirectory, and only briefly. Nothing else lists the main directory's shard directories (usage stats come from SQLite, and GETs and eviction use exact names), so a temp file appearing in one while a copy is in flight changes nothing. Rolling back to an older version after this one has run would leave intents in shard subdirectories unswept, and the old sweeper would log each subdirectory as a malformed intent once a minute. Updates tailscale/corp#47471 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> Change-Id: Icadb72b048c11f6e58c3332903a50438c561a283
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On ci-gocached-aws-1 the put queue's copies into its S3 Files NFS mount
top out at about 95 blobs/s no matter how many movers run: 8, an
adaptive governor, and 128 all hit the same ceiling. The kernel's
per-mount NFS statistics, sampled while the mover backlog was 14,000 to
24,000 deep, show why. Creates and removes each take about 5.3ms
round trip with no client-side slot queueing, but by Little's law only
1.15 of them are in progress on average while over 100 movers are
blocked in them, and goroutine profiles put every blocked mover in the
cleanup intent directory. The mutations of that one directory run one
at a time, and each blob costs two of them (the intent's create and its
later unlink), so 1/(2 x 5.3ms) is about 95 blobs/s.
The same sample shows about 20 renames/s against about 100 copies/s.
Every real copy renames its temp file into place, so roughly 80% of
spooled blobs are already present in the main directory, and
copyToMain returns early after its stat. They still pay for the intent,
which protects nothing: it exists so the sweeper can reap a blob this
PUT created if the PUT never commits, and this PUT creates nothing.
Whether an existing blob survives a sweep is decided by SQLite
accounting and the pending-PUT check, which already cover the PUT until
its metadata commits.
So stat the target before writing the intent, and skip both the intent
and the copy when the blob is there with the expected size. That removes
both cleanup-directory mutations for most spooled blobs. The real copies
that remain cost two mutations of their own (the temp file's create in
the main directory's root and its rename out of it) besides the intent's
two.
Count the skips in gocached_put_queue_copy_skips, a subset of
gocached_put_queue_copy_duration_seconds_count, so the share of
already-present blobs is measured rather than inferred from NFS op
counts.
Updates tailscale/corp#47471