Skip to content

gocached: skip spooling PUTs of blobs already in the hot tier - #11

Open
bradfitz wants to merge 1 commit into
mainfrom
bradfitz/putq_existing_blob
Open

bradfitz wants to merge 1 commit into
mainfrom
bradfitz/putq_existing_blob

Conversation

@bradfitz

@bradfitz bradfitz commented Oct 8, 2026

Copy link
Copy Markdown
Member

With cleanup intents and temp files sharded, ci-gocached-aws-1's put
queue no longer waits on NFS directory locks, and its 4 CPUs are now
what saturates during PUT bursts of 600 to 800 a second. A CPU profile
from one such burst put 52% of all CPU time in spoolBlob, mostly lz4
compression of each PUT's body as it was spooled. Yet 86 to 89% of
spooled PUTs are of blobs the server already stores, which the movers
discover only afterward, when the copy into the main blob directory
finds the blob there and skips it: the compressed spool file is then
just renamed into the hot tier or deleted.

So check first. Before reserving queue room, look the blob that the
PUT's outputID names up in the hot tier's in-memory index, which needs
no I/O. If it's there, read the body only through SHA-256, without
compressing or spooling it, and verify that it hashes to outputID. The
pending entry then has no bytes of its own; it takes a slot in the
inline lane, which drains only through the metadata flusher, and skips
the movers entirely. GETs while it's pending serve the stored blob.
Blobs on disk but not hot take the normal path, which installs them in
the hot tier, so their next PUT takes this one.

Eviction runs constantly on a full cache, and can remove the blob
between the PUT and its metadata commit. The flusher therefore looks up
the blob's Blobs row instead of upserting it, and drops the PUT if the
row is gone or its stored size differs: committing an action for a blob
that's gone would leave it pointing at nothing. Eviction deletes Blobs
rows and files under the sqliteWriteMu the flusher holds, so a row seen
then is a blob still on disk. A dropped PUT also drops the blob from
the hot tier, in case the hot copy outlived its Blobs row (as a crash
in the middle of an eviction can leave it), so the next PUT of the blob
stores it again rather than being dropped in turn.

If the body doesn't hash to outputID, it has been read and discarded,
so nothing is stored. That needs a client whose outputID isn't the hash
of what it sent but is the hash of a blob held here, which should be
rare. For now the PUT is logged and counted and still answered 204;
there's a TODO to make it a 500 once the counter confirms it's rare.

New counters: gocached_put_existing (PUTs taking this path),
gocached_put_existing_mismatches, and gocached_put_existing_gone. PUTs
on this path are labeled storage="existing" in the PUT duration and
blob size histograms, so fewer show up as storage="disk". Of the old
path, gocached_put_queue_copy_skips should fall to the PUTs of blobs
on disk but not in the hot tier.

Updates tailscale/corp#47471

@bradfitz
bradfitz requested a review from a team October 8, 2026 19:58
With cleanup intents and temp files sharded, ci-gocached-aws-1's put
queue no longer waits on NFS directory locks, and its 4 CPUs are now
what saturates during PUT bursts of 600 to 800 a second. A CPU profile
from one such burst put 52% of all CPU time in spoolBlob, mostly lz4
compression of each PUT's body as it was spooled. Yet 86 to 89% of
spooled PUTs are of blobs the server already stores, which the movers
discover only afterward, when the copy into the main blob directory
finds the blob there and skips it: the compressed spool file is then
just renamed into the hot tier or deleted.

So check first. Before reserving queue room, look the blob that the
PUT's outputID names up in the hot tier's in-memory index, which needs
no I/O. If it's there, read the body only through SHA-256, without
compressing or spooling it, and verify that it hashes to outputID. The
pending entry then has no bytes of its own; it takes a slot in the
inline lane, which drains only through the metadata flusher, and skips
the movers entirely. GETs while it's pending serve the stored blob.
Blobs on disk but not hot take the normal path, which installs them in
the hot tier, so their next PUT takes this one.

Eviction runs constantly on a full cache, and can remove the blob
between the PUT and its metadata commit. The flusher therefore looks up
the blob's Blobs row instead of upserting it, and drops the PUT if the
row is gone or its stored size differs: committing an action for a blob
that's gone would leave it pointing at nothing. Eviction deletes Blobs
rows and files under the sqliteWriteMu the flusher holds, so a row seen
then is a blob still on disk. A dropped PUT also drops the blob from
the hot tier, in case the hot copy outlived its Blobs row (as a crash
in the middle of an eviction can leave it), so the next PUT of the blob
stores it again rather than being dropped in turn.

If the body doesn't hash to outputID, it has been read and discarded,
so nothing is stored. That needs a client whose outputID isn't the hash
of what it sent but is the hash of a blob held here, which should be
rare. For now the PUT is logged and counted and still answered 204;
there's a TODO to make it a 500 once the counter confirms it's rare.

New counters: gocached_put_existing (PUTs taking this path),
gocached_put_existing_mismatches, and gocached_put_existing_gone. PUTs
on this path are labeled storage="existing" in the PUT duration and
blob size histograms, so fewer show up as storage="disk". Of the old
path, gocached_put_queue_copy_skips should fall to the PUTs of blobs
on disk but not in the hot tier.

Updates tailscale/corp#47471

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I1ddcfcc1d6e16da1f86fa072f20339fadf432625
@bradfitz
bradfitz force-pushed the bradfitz/putq_existing_blob branch from 0f6137c to ecc85ed Compare October 8, 2026 20:59

@runewake2 runewake2 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm, just a comment about some Prometheus config that drifts with this change.

// of what it sent and yet is the hash of a blob held here.
s.logf("put of %v: outputID names a hot blob but the body hashes to %v; not stored", outputID, sha256hex)
s.m.PutExistingMismatches.Add(1)
storage = "existing"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need to update some of the prometheus labels as a result of this, they get updated by this change I believe.

Help: "wall time of each cache put request, labeled by storage path: disk, inline, peer (forwarded to the owning LAN peer), or error; and type: put (success), dup, or error",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants