Skip to content

Commit e0818dd

Browse files
srinathshresthaRaduBerinde
authored andcommitted
objstorage: cache local object paths
Local object operations repeatedly reconstructed the same filesystem path through base.MakeFilepath because object metadata stored only the file number, file type, and storage tier rather than the complete path. In a focused benchmark, this path construction took approximately 247–256 ns/op and allocated 112 bytes across four allocations per call. Store the eagerly computed local path in ObjectMetadata.Local.Path. Populate the path when existing hot- and cold-tier objects are discovered at startup, when a new local object is created, and when an object is created through LinkOrCopyFromLocal. Local path lookup, opening, sizing, and removal now reuse the stored path. Provider.Path retains a fallback for metadata without a populated path. A focused post-change benchmark runs at approximately 4.13–4.27 ns/op with 0 B/op and 0 allocs/op. Tests cover newly created hot- and cold-tier objects, startup-discovered objects, reuse of the cached path across local operations, and the fallback path. Testing: - go test ./... - go test -race ./objstorage/objstorageprovider Fixes #5340.
1 parent 13596f1 commit e0818dd

4 files changed

Lines changed: 176 additions & 29 deletions

File tree

‎objstorage/objstorage.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ type ObjectMetadata struct {
142142
// The fields below are only set if the object is on local storage.
143143
Local struct {
144144
Tier base.StorageTier
145+
// Path is the object's path within the filesystem for Tier (the hot
146+
// tier FS or the cold tier FS). It is populated by the provider for all
147+
// local objects it knows about; callers should use Provider.Path.
148+
Path string
145149
}
146150

147151
// The fields below are only set if the object is on remote storage.
@@ -200,6 +204,9 @@ func (meta *ObjectMetadata) Placement() base.Placement {
200204
// AssertValid checks that the metadata is sane.
201205
func (meta *ObjectMetadata) AssertValid() {
202206
if !meta.IsRemote() {
207+
if meta.Local.Path == "" {
208+
panic(errors.AssertionFailedf("meta.Local.Path not set"))
209+
}
203210
// Verify all Remote fields are empty.
204211
if meta.Remote != (ObjectMetadata{}).Remote {
205212
panic(errors.AssertionFailedf("meta.Remote not empty: %#v", errors.Safe(meta.Remote)))
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
// Copyright 2026 The LevelDB-Go and Pebble Authors. All rights reserved. Use
2+
// of this source code is governed by a BSD-style license that can be found in
3+
// the LICENSE file.
4+
5+
package objstorageprovider
6+
7+
import (
8+
"context"
9+
"testing"
10+
11+
"github.com/cockroachdb/pebble/internal/base"
12+
"github.com/cockroachdb/pebble/objstorage"
13+
"github.com/cockroachdb/pebble/vfs"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
type pathJoinCountingFS struct {
18+
vfs.FS
19+
pathJoins int
20+
}
21+
22+
func (f *pathJoinCountingFS) PathJoin(elem ...string) string {
23+
f.pathJoins++
24+
return f.FS.PathJoin(elem...)
25+
}
26+
27+
func TestLocalObjectPath(t *testing.T) {
28+
ctx := context.Background()
29+
30+
t.Run("created", func(t *testing.T) {
31+
mem := vfs.NewMem()
32+
fs := &pathJoinCountingFS{FS: mem}
33+
require.NoError(t, fs.MkdirAll("hot", 0755))
34+
require.NoError(t, fs.MkdirAll("cold", 0755))
35+
36+
settings := DefaultSettings(fs, "hot")
37+
settings.Local.ColdTier.FS = fs
38+
settings.Local.ColdTier.FSDirName = "cold"
39+
p, err := Open(settings)
40+
require.NoError(t, err)
41+
defer func() { require.NoError(t, p.Close()) }()
42+
43+
for _, tc := range []struct {
44+
fileType base.FileType
45+
fileNum base.DiskFileNum
46+
tier base.StorageTier
47+
dir string
48+
}{
49+
{fileType: base.FileTypeTable, fileNum: 1, tier: base.HotTier, dir: "hot"},
50+
{fileType: base.FileTypeBlob, fileNum: 2, tier: base.ColdTier, dir: "cold"},
51+
} {
52+
w, meta, err := p.Create(ctx, tc.fileType, tc.fileNum, objstorage.CreateOptions{Tier: tc.tier})
53+
require.NoError(t, err)
54+
require.NoError(t, w.Finish())
55+
require.Equal(t, base.MakeFilepath(mem, tc.dir, tc.fileType, tc.fileNum), meta.Local.Path)
56+
57+
fs.pathJoins = 0
58+
require.Equal(t, meta.Local.Path, p.Path(meta))
59+
_, err = p.Size(meta)
60+
require.NoError(t, err)
61+
r, err := p.OpenForReading(ctx, tc.fileType, tc.fileNum, objstorage.OpenOptions{})
62+
require.NoError(t, err)
63+
require.NoError(t, r.Close())
64+
require.NoError(t, p.Remove(tc.fileType, tc.fileNum))
65+
require.Zero(t, fs.pathJoins)
66+
}
67+
})
68+
69+
t.Run("discovered", func(t *testing.T) {
70+
mem := vfs.NewMem()
71+
require.NoError(t, mem.MkdirAll("db", 0755))
72+
path := base.MakeFilepath(mem, "db", base.FileTypeTable, 1)
73+
file, err := mem.Create(path, vfs.WriteCategoryUnspecified)
74+
require.NoError(t, err)
75+
require.NoError(t, file.Close())
76+
77+
fs := &pathJoinCountingFS{FS: mem}
78+
p, err := Open(DefaultSettings(fs, "db"))
79+
require.NoError(t, err)
80+
defer func() { require.NoError(t, p.Close()) }()
81+
82+
meta, err := p.Lookup(base.FileTypeTable, 1)
83+
require.NoError(t, err)
84+
require.Equal(t, path, meta.Local.Path)
85+
86+
fs.pathJoins = 0
87+
require.Equal(t, path, p.Path(meta))
88+
_, err = p.Size(meta)
89+
require.NoError(t, err)
90+
r, err := p.OpenForReading(ctx, base.FileTypeTable, 1, objstorage.OpenOptions{})
91+
require.NoError(t, err)
92+
require.NoError(t, r.Close())
93+
require.NoError(t, p.Remove(base.FileTypeTable, 1))
94+
require.Zero(t, fs.pathJoins)
95+
})
96+
97+
t.Run("fallback", func(t *testing.T) {
98+
mem := vfs.NewMem()
99+
fs := &pathJoinCountingFS{FS: mem}
100+
require.NoError(t, fs.MkdirAll("db", 0755))
101+
p, err := Open(DefaultSettings(fs, "db"))
102+
require.NoError(t, err)
103+
defer func() { require.NoError(t, p.Close()) }()
104+
105+
// reportCorruption passes bare metadata when a local object is missing.
106+
meta := objstorage.ObjectMetadata{
107+
DiskFileNum: 1,
108+
FileType: base.FileTypeTable,
109+
}
110+
fs.pathJoins = 0
111+
require.Equal(t, base.MakeFilepath(mem, "db", base.FileTypeTable, 1), p.Path(meta))
112+
require.NotZero(t, fs.pathJoins)
113+
})
114+
}

‎objstorage/objstorageprovider/provider.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ func (p *provider) OpenForReading(
286286

287287
var r objstorage.Readable
288288
if !meta.IsRemote() {
289-
r, err = p.localOpenForReading(ctx, fileType, fileNum, meta.Local.Tier, opts)
289+
r, err = p.localOpenForReading(ctx, meta, opts)
290290
} else {
291291
r, err = p.remoteOpenForReading(ctx, meta, opts)
292292
if err != nil && p.isNotExistError(meta, err) {
@@ -354,7 +354,7 @@ func (p *provider) Remove(fileType base.FileType, fileNum base.DiskFileNum) erro
354354
}
355355

356356
if !meta.IsRemote() {
357-
err = p.localRemove(fileType, fileNum, meta.Local.Tier)
357+
err = p.localRemove(meta)
358358
} else {
359359
// TODO(radu): implement remote object removal (i.e. deref).
360360
err = p.sharedUnref(meta)
@@ -436,6 +436,7 @@ func (p *provider) LinkOrCopyFromLocal(
436436
FileType: dstFileType,
437437
}
438438
meta.Local.Tier = tier
439+
meta.Local.Path = dstPath
439440
p.addMetadata(meta)
440441
return meta, nil
441442
}
@@ -501,6 +502,12 @@ func (p *provider) Lookup(
501502
// Path is part of the objstorage.Provider interface.
502503
func (p *provider) Path(meta objstorage.ObjectMetadata) string {
503504
if !meta.IsRemote() {
505+
// Local.Path is set for all objects known to the provider. Callers may
506+
// pass bare metadata for an object that isn't known (e.g. reportCorruption
507+
// for a missing local file); reconstruct the path in that case.
508+
if meta.Local.Path != "" {
509+
return meta.Local.Path
510+
}
504511
_, path := p.localPath(meta.FileType, meta.DiskFileNum, meta.Local.Tier)
505512
return path
506513
}
@@ -510,7 +517,7 @@ func (p *provider) Path(meta objstorage.ObjectMetadata) string {
510517
// Size returns the size of the object.
511518
func (p *provider) Size(meta objstorage.ObjectMetadata) (int64, error) {
512519
if !meta.IsRemote() {
513-
return p.localSize(meta.FileType, meta.DiskFileNum, meta.Local.Tier)
520+
return p.localSize(meta)
514521
}
515522
return p.remoteSize(meta)
516523
}

‎objstorage/objstorageprovider/vfs.go‎

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/cockroachdb/errors"
1414
"github.com/cockroachdb/pebble/internal/base"
15+
"github.com/cockroachdb/pebble/internal/invariants"
1516
"github.com/cockroachdb/pebble/objstorage"
1617
"github.com/cockroachdb/pebble/vfs"
1718
)
@@ -73,10 +74,24 @@ func (ls *localLockedState) objChanged(meta objstorage.ObjectMetadata) {
7374
func (p *provider) localPath(
7475
fileType base.FileType, fileNum base.DiskFileNum, tier base.StorageTier,
7576
) (vfs.FS, string) {
77+
fs, dir := p.localTierFS(tier)
78+
return fs, base.MakeFilepath(fs, dir, fileType, fileNum)
79+
}
80+
81+
// localFS returns the filesystem backing the given tier. It falls back to the
82+
// hot tier when no cold tier filesystem is configured.
83+
func (p *provider) localFS(tier base.StorageTier) vfs.FS {
84+
fs, _ := p.localTierFS(tier)
85+
return fs
86+
}
87+
88+
// localTierFS returns the filesystem and directory backing the given tier. It
89+
// falls back to the hot tier when no cold tier filesystem is configured.
90+
func (p *provider) localTierFS(tier base.StorageTier) (vfs.FS, string) {
7691
if coldFS := p.st.Local.ColdTier.FS; tier == base.ColdTier && coldFS != nil {
77-
return coldFS, base.MakeFilepath(coldFS, p.st.Local.ColdTier.FSDirName, fileType, fileNum)
92+
return coldFS, p.st.Local.ColdTier.FSDirName
7893
}
79-
return p.st.Local.FS, base.MakeFilepath(p.st.Local.FS, p.st.Local.FSDirName, fileType, fileNum)
94+
return p.st.Local.FS, p.st.Local.FSDirName
8095
}
8196

8297
// metaFileType returns the file type for a file that contains only the metadata
@@ -119,17 +134,14 @@ func offsetFromMetaPath(filename string) (startOffset int64, ok bool) {
119134
}
120135

121136
func (p *provider) localOpenForReading(
122-
ctx context.Context,
123-
fileType base.FileType,
124-
fileNum base.DiskFileNum,
125-
tier base.StorageTier,
126-
opts objstorage.OpenOptions,
137+
ctx context.Context, meta objstorage.ObjectMetadata, opts objstorage.OpenOptions,
127138
) (objstorage.Readable, error) {
128-
fs, filename := p.localPath(fileType, fileNum, tier)
139+
fs := p.localFS(meta.Local.Tier)
140+
filename := meta.Local.Path
129141
file, err := fs.Open(filename, vfs.RandomReadsOption)
130142
if err != nil {
131143
if opts.MustExist && p.IsNotExistError(err) {
132-
err = base.AddDetailsToNotExistError(p.st.Local.FS, filename, err)
144+
err = base.AddDetailsToNotExistError(fs, filename, err)
133145
err = base.MarkCorruptionError(err)
134146
}
135147
return nil, err
@@ -138,9 +150,11 @@ func (p *provider) localOpenForReading(
138150
if err != nil {
139151
return nil, err
140152
}
141-
if tier == base.ColdTier {
142-
if startOffset, ok := p.getColdObjectMetaFile(fileType, fileNum); ok {
143-
metaPath := p.metaPath(fileType, fileNum, startOffset)
153+
if meta.Local.Tier == base.ColdTier {
154+
if startOffset, ok := p.getColdObjectMetaFile(meta.FileType, meta.DiskFileNum); ok {
155+
// TODO: Store this path in metaFileInfo to avoid reconstructing it on
156+
// every open and removal of a cold-tier object.
157+
metaPath := p.metaPath(meta.FileType, meta.DiskFileNum, startOffset)
144158
return newColdReadableWithHotMeta(r, p.st.Local.FS, metaPath, startOffset), nil
145159
}
146160
}
@@ -172,22 +186,21 @@ func (p *provider) vfsCreate(
172186
FileType: fileType,
173187
}
174188
meta.Local.Tier = tier
189+
meta.Local.Path = filename
175190
w := objstorage.Writable(newFileBufferedWritable(file))
176191
if tier == base.ColdTier {
177192
w = newColdWritable(p, fileType, fileNum, w, category)
178193
}
179194
return w, meta, nil
180195
}
181196

182-
func (p *provider) localRemove(
183-
fileType base.FileType, fileNum base.DiskFileNum, tier base.StorageTier,
184-
) error {
185-
fs, path := p.localPath(fileType, fileNum, tier)
186-
err := p.st.Local.FSCleaner.Clean(fs, fileType, path)
187-
if tier == base.ColdTier {
188-
if startOffset, ok := p.popColdObjectMetaFile(fileType, fileNum); ok {
189-
metaPath := p.metaPath(fileType, fileNum, startOffset)
190-
metaFileType := metaFileType(fileType)
197+
func (p *provider) localRemove(meta objstorage.ObjectMetadata) error {
198+
fs := p.localFS(meta.Local.Tier)
199+
err := p.st.Local.FSCleaner.Clean(fs, meta.FileType, meta.Local.Path)
200+
if meta.Local.Tier == base.ColdTier {
201+
if startOffset, ok := p.popColdObjectMetaFile(meta.FileType, meta.DiskFileNum); ok {
202+
metaPath := p.metaPath(meta.FileType, meta.DiskFileNum, startOffset)
203+
metaFileType := metaFileType(meta.FileType)
191204
err = firstError(err, p.st.Local.FSCleaner.Clean(p.st.Local.FS, metaFileType, metaPath))
192205
}
193206
}
@@ -219,6 +232,10 @@ func (p *provider) localInit() error {
219232
DiskFileNum: fileNum,
220233
}
221234
o.Local.Tier = base.HotTier
235+
_, o.Local.Path = p.localPath(o.FileType, o.DiskFileNum, o.Local.Tier)
236+
if invariants.Enabled {
237+
o.AssertValid()
238+
}
222239
p.mu.knownObjects[o.DiskFileNum] = o
223240
}
224241
}
@@ -246,6 +263,10 @@ func (p *provider) localInit() error {
246263
DiskFileNum: fileNum,
247264
}
248265
o.Local.Tier = base.ColdTier
266+
_, o.Local.Path = p.localPath(o.FileType, o.DiskFileNum, o.Local.Tier)
267+
if invariants.Enabled {
268+
o.AssertValid()
269+
}
249270
if _, exists := p.mu.knownObjects[o.DiskFileNum]; exists {
250271
p.st.Logger.Errorf("object %s exists on both tiers; using hot tier version", o.DiskFileNum)
251272
} else {
@@ -327,11 +348,9 @@ func (p *provider) localSync() error {
327348
return nil
328349
}
329350

330-
func (p *provider) localSize(
331-
fileType base.FileType, fileNum base.DiskFileNum, tier base.StorageTier,
332-
) (int64, error) {
333-
fs, filename := p.localPath(fileType, fileNum, tier)
334-
stat, err := fs.Stat(filename)
351+
func (p *provider) localSize(meta objstorage.ObjectMetadata) (int64, error) {
352+
fs := p.localFS(meta.Local.Tier)
353+
stat, err := fs.Stat(meta.Local.Path)
335354
if err != nil {
336355
return 0, err
337356
}

0 commit comments

Comments
 (0)