From 42c61138e8fbeb7d4d7cefbe6389f9660eabc5e0 Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Mon, 2 Dec 2019 12:18:20 +0100 Subject: [PATCH] storage: Improve doc comments delete methods (#3591) Improve the documentation of several methods involved in the delete operation to make clear their behavior without having to inspect their logic. --- storage/filestore/dir.go | 13 ++++++++++++- storage/filestore/store.go | 5 ++++- storagenode/piecestore/endpoint.go | 8 +++++--- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/storage/filestore/dir.go b/storage/filestore/dir.go index f8186c9b2..56393b626 100644 --- a/storage/filestore/dir.go +++ b/storage/filestore/dir.go @@ -442,12 +442,23 @@ func (dir *Dir) iterateStorageFormatVersions(ctx context.Context, ref storage.Bl } // Delete deletes blobs with the specified ref (in all supported storage formats). +// +// It doesn't return an error if the blob is not found for any reason or it +// cannot be deleted at this moment and it's delayed. func (dir *Dir) Delete(ctx context.Context, ref storage.BlobRef) (err error) { defer mon.Task()(&ctx)(&err) return dir.iterateStorageFormatVersions(ctx, ref, dir.DeleteWithStorageFormat) } -// DeleteWithStorageFormat deletes the blob with the specified ref for one specific format version +// DeleteWithStorageFormat deletes the blob with the specified ref for one +// specific format version. The method tries the following strategies, in order +// of preference until one succeeds: +// +// * moves the blob to garbage dir. +// * directly deletes the blob. +// * push the blobs to queue for retrying later. +// +// It doesn't return an error if the piece isn't found for any reason. func (dir *Dir) DeleteWithStorageFormat(ctx context.Context, ref storage.BlobRef, formatVer storage.FormatVersion) (err error) { defer mon.Task()(&ctx)(&err) return dir.deleteWithStorageFormatInPath(ctx, dir.blobsdir(), ref, formatVer) diff --git a/storage/filestore/store.go b/storage/filestore/store.go index 7d3d4a6dc..8c9e927b8 100644 --- a/storage/filestore/store.go +++ b/storage/filestore/store.go @@ -89,7 +89,10 @@ func (store *blobStore) StatWithStorageFormat(ctx context.Context, ref storage.B return info, Error.Wrap(err) } -// Delete deletes blobs with the specified ref +// Delete deletes blobs with the specified ref. +// +// It doesn't return an error if the blob isn't found for any reason or it cannot +// be deleted at this moment and it's delayed. func (store *blobStore) Delete(ctx context.Context, ref storage.BlobRef) (err error) { defer mon.Task()(&ctx)(&err) err = store.dir.Delete(ctx, ref) diff --git a/storagenode/piecestore/endpoint.go b/storagenode/piecestore/endpoint.go index be37c3376..3f44ebfeb 100644 --- a/storagenode/piecestore/endpoint.go +++ b/storagenode/piecestore/endpoint.go @@ -158,10 +158,12 @@ func (endpoint *Endpoint) Delete(ctx context.Context, delete *pb.PieceDeleteRequ if err := endpoint.store.Delete(ctx, delete.Limit.SatelliteId, delete.Limit.PieceId); err != nil { // explicitly ignoring error because the errors - // TODO: add more debug info + + // TODO: https://storjlabs.atlassian.net/browse/V3-3222 + // report rpc status of internal server error or not found error, + // e.g. not found might happen when we get a deletion request after garbage + // collection has deleted it endpoint.log.Error("delete failed", zap.Stringer("Satellite ID", delete.Limit.SatelliteId), zap.Stringer("Piece ID", delete.Limit.PieceId), zap.Error(err)) - // TODO: report rpc status of internal server error or missing error, - // e.g. missing might happen when we get a deletion request after garbage collection has deleted it } else { endpoint.log.Info("deleted", zap.Stringer("Satellite ID", delete.Limit.SatelliteId), zap.Stringer("Piece ID", delete.Limit.PieceId)) }