Skip to content

Commit 223b557

Browse files
frristclaude
andcommitted
perf(blockstore,bucket): stream body blobs instead of buffering whole in RAM
Addresses PR review (alanshaw): the spool buffered each object-body blob (up to max_blob_size = 256 MiB) whole in memory on both the write and read paths, so peak RAM was blob_size × concurrency — wrong for a memory-bound appliance, and counter to the architecture's "nothing held whole in RAM" (§5/§7.1). Body blobs now stream; the small-catalog-block path (GetBlock / PutBlock, CID-keyed, fine in RAM) is unchanged. New BlobWriter/BlobReader seams (WriteBlob / OpenBlob); ReadStore gains BlobReader. Write: SplitBody streams each blob through Spool.WriteBlob, which hashes (sha256) while copying straight to a temp file (atomic rename to the digest path) — the blob never sits whole in RAM. Replaces the bytes.Buffer + PutBlock(block.Block) per blob. Read: blobBodyReader streams via OpenBlob(digest) (io.ReadCloser) instead of GetBlock → os.ReadFile, holding at most one open blob at a time and seeking into it for a ranged read, so peak RAM per concurrent GET is a copy buffer, not a 256 MiB blob. OpenBlob is implemented by Spool (os.Open), Forge (stream the /content/retrieve body, dropping io.ReadAll — GetBlock and OpenBlob now share a `retrieve` helper), and Layered/Cached (spool → base, no LRU for streamed blobs). Validated in-process: full suite + new bucket/chunker tests (multi-blob round trip, mid-blob/boundary-spanning ranged reads, empty body) and blockstore OpenBlob tier-dispatch tests. The real-network Forge.OpenBlob path is covered by smelt's read-after-eviction e2e, which currently can't run — the published piri:main image fails to boot (fx: NewPieceAccepter missing ucan.Issuer), unrelated to this change — so that leg is pending a working piri image. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d86e432 commit 223b557

9 files changed

Lines changed: 489 additions & 112 deletions

File tree

blockstore/cache.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ package blockstore
33
import (
44
"container/list"
55
"context"
6+
"io"
67
"sync"
78

89
block "github.com/ipfs/go-block-format"
910
"github.com/ipfs/go-cid"
11+
mh "github.com/multiformats/go-multihash"
1012
)
1113

1214
// Cached wraps a BlockReader with a bounded, in-memory LRU keyed by CID. Blocks
@@ -50,7 +52,21 @@ func NewCached(base BlockReader, maxBytes int64) BlockReader {
5052
}
5153
}
5254

53-
var _ BlockReader = (*Cached)(nil)
55+
var (
56+
_ BlockReader = (*Cached)(nil)
57+
_ BlobReader = (*Cached)(nil)
58+
)
59+
60+
// OpenBlob streams a body blob straight from the base reader, bypassing the LRU.
61+
// Body blobs are large and streamed; caching one whole would defeat streaming
62+
// (and a single blob can exceed the whole budget). Small catalog blocks still
63+
// cache through GetBlock.
64+
func (c *Cached) OpenBlob(ctx context.Context, digest mh.Multihash) (io.ReadCloser, error) {
65+
if br, ok := c.base.(BlobReader); ok {
66+
return br.OpenBlob(ctx, digest)
67+
}
68+
return nil, ErrNotFound
69+
}
5470

5571
// GetBlock returns the cached block if present, otherwise fetches it from the
5672
// base reader and caches it (subject to the byte budget).

blockstore/forge.go

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/fil-forge/ucantone/ucan/invocation"
1919
block "github.com/ipfs/go-block-format"
2020
"github.com/ipfs/go-cid"
21+
mh "github.com/multiformats/go-multihash"
2122
"go.uber.org/zap"
2223

2324
"github.com/fil-forge/ingot/blockstore/locator"
@@ -47,7 +48,10 @@ type Forge struct {
4748
logger *zap.Logger
4849
}
4950

50-
var _ BlockReader = (*Forge)(nil)
51+
var (
52+
_ BlockReader = (*Forge)(nil)
53+
_ BlobReader = (*Forge)(nil)
54+
)
5155

5256
// ForgeConfig wires a read-only Forge block reader.
5357
type ForgeConfig struct {
@@ -131,26 +135,29 @@ func NewForge(cfg ForgeConfig) (*Forge, error) {
131135
}, nil
132136
}
133137

134-
// GetBlock resolves the CID through the indexer and retrieves the underlying
135-
// bytes from piri via a UCAN-authorized /content/retrieve invocation, scoped to
136-
// the inner block's byte range within the containing CAR shard.
137-
func (f *Forge) GetBlock(ctx context.Context, c cid.Cid) (block.Block, error) {
138+
// retrieve resolves c through the locator and issues a UCAN-authorized
139+
// /content/retrieve to the piri node that holds it, returning the response body
140+
// stream and the expected byte length (the location Range is inclusive, so
141+
// End-Start+1). The caller owns the returned reader and must Close it. Shared by
142+
// GetBlock (which buffers small catalog blocks) and OpenBlob (which streams large
143+
// body blobs straight through).
144+
func (f *Forge) retrieve(ctx context.Context, c cid.Cid) (io.ReadCloser, int64, error) {
138145
locations, err := f.locator.Locate(ctx, f.spaces, c.Hash())
139146
if err != nil {
140147
var nf locator.NotFoundError
141148
if errors.As(err, &nf) {
142-
return nil, ErrNotFound
149+
return nil, 0, ErrNotFound
143150
}
144-
return nil, fmt.Errorf("forge: locate %s: %w", c, err)
151+
return nil, 0, fmt.Errorf("forge: locate %s: %w", c, err)
145152
}
146153
if len(locations) == 0 {
147-
return nil, ErrNotFound
154+
return nil, 0, ErrNotFound
148155
}
149156

150157
loc := locations[0]
151158
cm := loc.Commitment
152159
if len(cm.Location) == 0 {
153-
return nil, fmt.Errorf("forge: empty location URL set for %s", c)
160+
return nil, 0, fmt.Errorf("forge: empty location URL set for %s", c)
154161
}
155162
target := cm.Location[0]
156163

@@ -173,7 +180,7 @@ func (f *Forge) GetBlock(ctx context.Context, c cid.Cid) (block.Block, error) {
173180
delegation.WithExpiration(ucan.Now()+retrievalAuthTTL),
174181
)
175182
if err != nil {
176-
return nil, fmt.Errorf("forge: build retrieval proof: %w", err)
183+
return nil, 0, fmt.Errorf("forge: build retrieval proof: %w", err)
177184
}
178185

179186
inv, err := contentcmds.Retrieve.Invoke(
@@ -187,41 +194,59 @@ func (f *Forge) GetBlock(ctx context.Context, c cid.Cid) (block.Block, error) {
187194
invocation.WithProofs(retrievalProof.Link()),
188195
)
189196
if err != nil {
190-
return nil, fmt.Errorf("forge: build retrieve invocation: %w", err)
197+
return nil, 0, fmt.Errorf("forge: build retrieve invocation: %w", err)
191198
}
192199

193200
rclient, err := retrieval.NewClient(target.URL(), retrieval.WithHTTPClient(f.httpClient))
194201
if err != nil {
195-
return nil, fmt.Errorf("forge: build retrieval client: %w", err)
202+
return nil, 0, fmt.Errorf("forge: build retrieval client: %w", err)
196203
}
197204

198205
_, _, meta, err := ucanexec.Execute[*contentcmds.RetrieveOK](
199206
ctx, rclient, inv,
200207
execution.WithDelegations(retrievalProof),
201208
)
202209
if err != nil {
203-
return nil, fmt.Errorf("forge: retrieve %s: %w", c, err)
210+
return nil, 0, fmt.Errorf("forge: retrieve %s: %w", c, err)
204211
}
205212

206213
hcRes, ok := meta.(*retrieval.HTTPHeaderResponseContainer)
207214
if !ok {
208-
return nil, fmt.Errorf("forge: unexpected retrieval metadata type %T", meta)
215+
return nil, 0, fmt.Errorf("forge: unexpected retrieval metadata type %T", meta)
216+
}
217+
// Range is inclusive, so the expected length is End - Start + 1.
218+
return hcRes.Body, loc.Range.End - loc.Range.Start + 1, nil
219+
}
220+
221+
// GetBlock resolves the CID through the locator and retrieves the bytes from piri
222+
// via a UCAN-authorized /content/retrieve. It buffers the whole block, so it is
223+
// for small catalog blocks; object-body blobs use the streaming OpenBlob.
224+
func (f *Forge) GetBlock(ctx context.Context, c cid.Cid) (block.Block, error) {
225+
rc, wantLen, err := f.retrieve(ctx, c)
226+
if err != nil {
227+
return nil, err
209228
}
210-
defer hcRes.Body.Close()
229+
defer rc.Close()
211230

212-
body, err := io.ReadAll(hcRes.Body)
231+
body, err := io.ReadAll(rc)
213232
if err != nil {
214233
return nil, fmt.Errorf("forge: read retrieve body for %s: %w", c, err)
215234
}
216-
// Range is inclusive, so the expected length is End - Start + 1.
217-
wantLen := loc.Range.End - loc.Range.Start + 1
218235
if int64(len(body)) != wantLen {
219236
return nil, fmt.Errorf("forge: %s short read: got %d bytes, want %d", c, len(body), wantLen)
220237
}
221-
222238
return block.NewBlockWithCid(body, c)
223239
}
224240

241+
// OpenBlob streams an object-body blob from piri by digest, without buffering it
242+
// in memory — the network counterpart of Spool.OpenBlob. Bytes are served
243+
// straight off the /content/retrieve response; the caller owns the reader and
244+
// must Close it.
245+
func (f *Forge) OpenBlob(ctx context.Context, digest mh.Multihash) (io.ReadCloser, error) {
246+
rc, _, err := f.retrieve(ctx, cid.NewCidV1(cid.Raw, digest))
247+
return rc, err
248+
}
249+
225250
// newAuthorizeRetrieval returns the AuthorizeRetrievalFunc the IndexLocator
226251
// calls before each indexer query. The space signer (root authority) directly
227252
// authorizes the indexer to retrieve any blob in the space — the proof chain is

blockstore/layered.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ package blockstore
33
import (
44
"context"
55
"errors"
6+
"io"
67

78
block "github.com/ipfs/go-block-format"
89
"github.com/ipfs/go-cid"
10+
mh "github.com/multiformats/go-multihash"
911
)
1012

1113
// Layered is the production ReadStore: a read-only seam that consults the local
@@ -76,6 +78,27 @@ func (l *Layered) GetBlock(ctx context.Context, c cid.Cid) (blk block.Block, ret
7678
return l.base.GetBlock(ctx, c)
7779
}
7880

81+
// OpenBlob streams an object-body blob by digest: the local spool first (the
82+
// read-after-write floor), then the network base (after eviction). The log is
83+
// skipped — it only ever holds catalog blocks, never body blobs. Tiers that
84+
// don't implement BlobReader are treated as a miss. Returns ErrNotFound if no
85+
// tier has the blob.
86+
func (l *Layered) OpenBlob(ctx context.Context, digest mh.Multihash) (io.ReadCloser, error) {
87+
if br, ok := l.spool.(BlobReader); ok {
88+
rc, err := br.OpenBlob(ctx, digest)
89+
if err == nil {
90+
return rc, nil
91+
}
92+
if !errors.Is(err, ErrNotFound) {
93+
return nil, err
94+
}
95+
}
96+
if br, ok := l.base.(BlobReader); ok {
97+
return br.OpenBlob(ctx, digest)
98+
}
99+
return nil, ErrNotFound
100+
}
101+
79102
// layeredAsBlockstore lifts Layered into a BaseStore for the
80103
// CborStore wrapper. Internal-only — exists so the CBOR decoder
81104
// reuses Layered's cache + fallthrough order rather than going

blockstore/openblob_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package blockstore
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"errors"
7+
"io"
8+
"testing"
9+
10+
block "github.com/ipfs/go-block-format"
11+
"github.com/ipfs/go-cid"
12+
mh "github.com/multiformats/go-multihash"
13+
)
14+
15+
// fakeBlobTier is a read tier that implements BlockReader (so it fits the
16+
// Layered/Cached constructors) and BlobReader. data == nil means a miss.
17+
type fakeBlobTier struct{ data []byte }
18+
19+
func (f fakeBlobTier) GetBlock(context.Context, cid.Cid) (block.Block, error) {
20+
return nil, ErrNotFound
21+
}
22+
23+
func (f fakeBlobTier) OpenBlob(context.Context, mh.Multihash) (io.ReadCloser, error) {
24+
if f.data == nil {
25+
return nil, ErrNotFound
26+
}
27+
return io.NopCloser(bytes.NewReader(f.data)), nil
28+
}
29+
30+
func readOpenBlob(t *testing.T, r BlobReader, digest mh.Multihash) (string, error) {
31+
t.Helper()
32+
rc, err := r.OpenBlob(context.Background(), digest)
33+
if err != nil {
34+
return "", err
35+
}
36+
defer rc.Close()
37+
b, err := io.ReadAll(rc)
38+
return string(b), err
39+
}
40+
41+
// TestLayered_OpenBlob_Tiering covers the body-blob read fallthrough: spool first
42+
// (read-after-write), then the network base after eviction, then ErrNotFound. The
43+
// log is never consulted (it holds no body blobs).
44+
func TestLayered_OpenBlob_Tiering(t *testing.T) {
45+
digest, _ := mh.Sum([]byte("digest"), mh.SHA2_256, -1)
46+
47+
// spool hit — base is not consulted.
48+
l := NewLayered(fakeBlobTier{data: []byte("from-spool")}, nil, fakeBlobTier{data: []byte("from-base")})
49+
if got, err := readOpenBlob(t, l, digest); err != nil || got != "from-spool" {
50+
t.Fatalf("spool hit: got %q err %v, want from-spool", got, err)
51+
}
52+
53+
// spool miss → base hit (the after-eviction path).
54+
l = NewLayered(fakeBlobTier{data: nil}, nil, fakeBlobTier{data: []byte("from-base")})
55+
if got, err := readOpenBlob(t, l, digest); err != nil || got != "from-base" {
56+
t.Fatalf("spool miss → base: got %q err %v, want from-base", got, err)
57+
}
58+
59+
// both miss → ErrNotFound.
60+
l = NewLayered(fakeBlobTier{data: nil}, nil, fakeBlobTier{data: nil})
61+
if _, err := l.OpenBlob(context.Background(), digest); !errors.Is(err, ErrNotFound) {
62+
t.Fatalf("both miss: err = %v, want ErrNotFound", err)
63+
}
64+
65+
// nil spool is skipped, not panicked.
66+
l = NewLayered(nil, nil, fakeBlobTier{data: []byte("from-base")})
67+
if got, err := readOpenBlob(t, l, digest); err != nil || got != "from-base" {
68+
t.Fatalf("nil spool → base: got %q err %v, want from-base", got, err)
69+
}
70+
}
71+
72+
// TestCached_OpenBlob_BypassesCache confirms OpenBlob streams straight from the
73+
// base reader (no LRU) — a large body blob must not be buffered into the cache.
74+
func TestCached_OpenBlob_BypassesCache(t *testing.T) {
75+
digest, _ := mh.Sum([]byte("digest"), mh.SHA2_256, -1)
76+
c := NewCached(fakeBlobTier{data: []byte("streamed")}, 1<<20).(*Cached)
77+
if got, err := readOpenBlob(t, c, digest); err != nil || got != "streamed" {
78+
t.Fatalf("cached OpenBlob: got %q err %v, want streamed", got, err)
79+
}
80+
}

blockstore/spool.go

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ package blockstore
22

33
import (
44
"context"
5+
"crypto/sha256"
56
"encoding/hex"
67
"errors"
78
"fmt"
9+
"io"
810
"os"
911
"path/filepath"
1012

@@ -72,6 +74,60 @@ func (s *Spool) PutBlock(_ context.Context, blk block.Block) error {
7274
return nil
7375
}
7476

77+
// WriteBlob streams r to the spool, computing its sha256 digest as it writes so
78+
// the blob is never held whole in memory (object-body blobs run up to
79+
// max_blob_size = 256 MiB; buffering them would put that × concurrency in RAM).
80+
// The write is atomic (temp file → rename to the digest path), so a crash leaves
81+
// no partial blob readable under its digest. An empty r writes nothing and
82+
// returns a nil digest with n == 0 (a zero-byte object has no blob). Re-writing
83+
// an identical blob is idempotent (same digest, rename overwrites in place).
84+
func (s *Spool) WriteBlob(_ context.Context, r io.Reader) (mh.Multihash, int64, error) {
85+
tmp, err := os.CreateTemp(s.dir, ".tmp-*")
86+
if err != nil {
87+
return nil, 0, fmt.Errorf("blockstore: spool tempfile: %w", err)
88+
}
89+
tmpName := tmp.Name()
90+
hasher := sha256.New()
91+
n, copyErr := io.Copy(io.MultiWriter(tmp, hasher), r)
92+
if closeErr := tmp.Close(); closeErr != nil && copyErr == nil {
93+
copyErr = closeErr
94+
}
95+
if copyErr != nil {
96+
_ = os.Remove(tmpName)
97+
return nil, n, fmt.Errorf("blockstore: spool write: %w", copyErr)
98+
}
99+
if n == 0 {
100+
_ = os.Remove(tmpName)
101+
return nil, 0, nil
102+
}
103+
digest, err := mh.Encode(hasher.Sum(nil), mh.SHA2_256)
104+
if err != nil {
105+
_ = os.Remove(tmpName)
106+
return nil, n, fmt.Errorf("blockstore: spool digest: %w", err)
107+
}
108+
if err := os.Rename(tmpName, s.Path(digest)); err != nil {
109+
_ = os.Remove(tmpName)
110+
return nil, n, fmt.Errorf("blockstore: spool rename: %w", err)
111+
}
112+
return digest, n, nil
113+
}
114+
115+
// OpenBlob returns a streaming reader over the spooled blob with the given
116+
// digest, or ErrNotFound. Unlike GetBlock it does not read the blob into memory —
117+
// the body read path serves bytes straight off disk. The caller owns the reader
118+
// and must Close it. The returned *os.File is seekable, which the body reader
119+
// uses to start a ranged read mid-blob without reading-and-discarding.
120+
func (s *Spool) OpenBlob(_ context.Context, digest mh.Multihash) (io.ReadCloser, error) {
121+
f, err := os.Open(s.Path(digest))
122+
if errors.Is(err, os.ErrNotExist) {
123+
return nil, ErrNotFound
124+
}
125+
if err != nil {
126+
return nil, fmt.Errorf("blockstore: spool open %s: %w", digest.B58String(), err)
127+
}
128+
return f, nil
129+
}
130+
75131
// GetBlock returns the blob stored under c's multihash, or ErrNotFound. A miss
76132
// is expected and cheap: it lets the layered read path fall through to the log
77133
// (for catalog blocks, which are never spooled) or the network tier (for a body
@@ -102,8 +158,11 @@ func (s *Spool) Remove(digest mh.Multihash) error {
102158
return nil
103159
}
104160

105-
// Compile-time assertions: Spool is a read+write block tier.
161+
// Compile-time assertions: Spool is a read+write block tier, and the streaming
162+
// blob tier for object bodies.
106163
var (
107164
_ BlockReader = (*Spool)(nil)
108165
_ BlockWriter = (*Spool)(nil)
166+
_ BlobReader = (*Spool)(nil)
167+
_ BlobWriter = (*Spool)(nil)
109168
)

0 commit comments

Comments
 (0)