Skip to content

Commit

Permalink
blobfixture: tolerate non-found errors
Browse files Browse the repository at this point in the history
Previously, when the fixture GC was running, registry operations could
fail because they would list metadata blobs then attempt to read them.
Now, the registry tolerates missing blobs with the assumption they were
deleted by a concurrent GC.

Release note: none
Part of: #139159
  • Loading branch information
jeffswenson committed Feb 21, 2025
1 parent ca1026a commit c97ba77
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 10 deletions.
42 changes: 32 additions & 10 deletions pkg/roachprod/blobfixture/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,23 +179,45 @@ func (r *Registry) URI(path string) url.URL {
return copy
}

// maybeReadFile attempts to read a file and returns its contents. Returns nil bytes
// if the file does not exist. This is useful for handling cases where files may have
// been concurrently deleted by GC.
func (r *Registry) maybeReadFile(ctx context.Context, filename string) ([]byte, error) {
bytes, err := func() ([]byte, error) {
file, _, err := r.storage.ReadFile(ctx, filename, cloud.ReadOptions{})
if err != nil {
return nil, err
}
defer func() { _ = file.Close(ctx) }()

bytes, err := ioctx.ReadAll(ctx, file)
if err != nil {
return nil, err
}
return bytes, nil
}()
if errors.Is(err, cloud.ErrFileDoesNotExist) {
return nil, nil
}
return bytes, err
}

// listFixtures lists all fixtures of the given kind.
func (r *Registry) listFixtures(
ctx context.Context, kindPrefix string, l *logger.Logger,
) ([]FixtureMetadata, error) {
if l != nil {
l.Printf("listing fixtures: %s", kindPrefix)
}
result := []FixtureMetadata{}
var result []FixtureMetadata

err := r.storage.List(ctx, kindPrefix /*delimiter*/, "", func(found string) error {
file, _, err := r.storage.ReadFile(ctx, path.Join(kindPrefix, found), cloud.ReadOptions{})
json, err := r.maybeReadFile(ctx, path.Join(kindPrefix, found))
if err != nil {
return err
}
defer func() { _ = file.Close(ctx) }()

json, err := ioctx.ReadAll(ctx, file)
if err != nil {
return err
if json == nil {
return nil // Skip files that don't exist (may have been GC'd)
}

metadata := FixtureMetadata{}
Expand All @@ -204,7 +226,6 @@ func (r *Registry) listFixtures(
}

result = append(result, metadata)

return nil
})
if err != nil {
Expand Down Expand Up @@ -234,13 +255,14 @@ func (r *Registry) upsertMetadata(metadata FixtureMetadata) error {
}

func (r *Registry) deleteMetadata(metadata FixtureMetadata) error {
return errors.Wrap(r.storage.Delete(context.Background(), metadata.MetadataPath), "unable to delete fixture metadata")
return errors.Wrap(r.storage.Delete(context.Background(), metadata.MetadataPath), "failed to delete metadata")
}

func (r *Registry) deleteBlobsMatchingPrefix(prefix string) error {
return r.storage.List(context.Background(), prefix, "", func(path string) error {
err := r.storage.List(context.Background(), prefix, "", func(path string) error {
return r.storage.Delete(context.Background(), prefix+path)
})
return errors.Wrapf(err, "failed to delete blobs matching prefix %q", prefix)
}

// ScratchHandle is returned by Registry.Create and is used to mark a fixture
Expand Down
43 changes: 43 additions & 0 deletions pkg/roachprod/blobfixture/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,46 @@ func TestFixtureRegistryURI(t *testing.T) {
"nodelocal://1/roachprod/v25.1/metadata/test-kind/20240601-1223",
metaUri.String())
}

// setupTestFile creates a test file with given content in the registry
func setupTestFile(t *testing.T, registry *Registry, path string, content []byte) {
t.Helper()
ctx := context.Background()
writer, err := registry.storage.Writer(ctx, path)
require.NoError(t, err)
_, err = writer.Write(content)
require.NoError(t, err)
require.NoError(t, writer.Close())
}

// checkFileContent verifies file content matches expected or doesn't exist
func checkFileContent(t *testing.T, registry *Registry, path string, expectedContent []byte) {
t.Helper()
ctx := context.Background()
content, err := registry.maybeReadFile(ctx, path)
if expectedContent == nil {
require.Nil(t, content)
return
}
require.NoError(t, err)
require.Equal(t, expectedContent, content)
}

func TestRegistryHelpers(t *testing.T) {
defer leaktest.AfterTest(t)()

registry := newTestRegistry(t, "nodelocal://1/roachtest/v25.1")
ctx := context.Background()

testData := []byte("test content")
testPath := "test/file.txt"

// Test reading non-existent file
content, err := registry.maybeReadFile(ctx, "nonexistent")
require.NoError(t, err)
require.Nil(t, content)

// Test reading existing file
setupTestFile(t, registry, testPath, testData)
checkFileContent(t, registry, testPath, testData)
}

0 comments on commit c97ba77

Please sign in to comment.