Skip to content

Commit

Permalink
storagenode/blobstore: minor simplifications
Browse files Browse the repository at this point in the history
In one case, we don't need to do an extra stat() call at the top of the
function every time we trash a blob, since we os.MkdirAll() a descendant
of that same directory a little bit further down. Any os.IsNotExist
errors coming from the rename call are definitely talking about the
source file, not the destination directory (unless of course the dest
directory was deleted somehow concurrently, but that problem already
existed so we're not making it worse).

In the second case, we don't necessarily need special code to handle the
possibility of a v0PieceAccess, because this is the blob store layer,
which doesn't know anything about pieces. All blobs are blobs, to the
blob store. (Even if that weren't the case, we only put FormatV1 blobs
into the trash, so it still couldn't be a v0PieceAccess.) Add some extra
info to diagnose the situation better if it ever happens.

Change-Id: Idc2c2f79dbd2f6b9e4fdb28da19d0241d4596d47
  • Loading branch information
thepaul authored and Storj Robot committed Feb 16, 2024
1 parent 349b67d commit 759b419
Showing 1 changed file with 3 additions and 11 deletions.
14 changes: 3 additions & 11 deletions storagenode/blobstore/filestore/dir.go
Expand Up @@ -323,13 +323,6 @@ func (dir *Dir) Trash(ctx context.Context, ref blobstore.BlobRef) (err error) {

// TrashWithStorageFormat moves the blob specified by ref to the trash for the specified format version.
func (dir *Dir) TrashWithStorageFormat(ctx context.Context, ref blobstore.BlobRef, formatVer blobstore.FormatVersion) (err error) {
// Ensure trashdir exists so that we know any os.IsNotExist errors below
// are not from a missing trash dir
_, err = os.Stat(dir.trashdir())
if err != nil {
return err
}

blobsBasePath, err := dir.blobToBasePath(ref)
if err != nil {
return err
Expand Down Expand Up @@ -475,12 +468,11 @@ func (dir *Dir) EmptyTrash(ctx context.Context, namespace []byte, trashedBefore
if errors.Is(err, ErrIsDir) {
return nil
}
// it would be best if we could report the actual problematic path
if thisBlobInfo, ok := info.(*blobInfo); ok {
errorsEncountered.Add(Error.New("%s: %s", thisBlobInfo.path, err))
errorsEncountered.Add(Error.New("%s: %w", thisBlobInfo.path, err))
} else {
// this is probably a v0PieceAccess; do what we can
errorsEncountered.Add(Error.New("blobRef %+v: %s", info.BlobRef(), err))
// if this happens, it's time to extend the code to handle the other type
errorsEncountered.Add(Error.New("%+v [unexpected type %T]: %w", info, info, err))
}
return nil
}
Expand Down

0 comments on commit 759b419

Please sign in to comment.