Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add check when initializing processing state #744

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add check when initializing processing state #744

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd3e54d
add check
Sep 4, 2024
f5dcc79
break down case by case
Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions disperser/common/blobstore/shared_storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const (

var errProcessingToDispersing = errors.New("blob transit to dispersing from non processing")

var errProcessingMeetingPrecondition = errors.New("blob not meeting precondition to processing")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errProcessingFailedPrecondition("blob metadata already exists....")?


// The shared blob store that the disperser is operating on.
// The metadata store is backed by DynamoDB and the blob store is backed by S3.
//
Expand Down Expand Up @@ -90,6 +92,24 @@ func (s *SharedBlobStore) StoreBlob(ctx context.Context, blob *core.Blob, reques
metadataKey.BlobHash = blobHash
metadataKey.MetadataHash = metadataHash

refreshedMetadata, err := s.GetBlobMetadata(ctx, metadataKey)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if there is a 500 error from dynamo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I should just check if refreshedMetadata is nil or not


// the only safe condition:
// err is disperser.ErrMetadataNotFound && refreshedMetadata is nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me there are 2 failure cases:

  • Failed to get metadata (due to errors other than Not Found): this is an INTERNAL error
  • Got the metadata: this is FAILED_PRECONDITION error

if err == nil && refreshedMetadata != nil {
s.logger.Error("error blob not meeting precondition", "status", refreshedMetadata.BlobStatus, "metadataKey", refreshedMetadata.GetBlobKey().String())
return metadataKey, errProcessingMeetingPrecondition
} else if err != nil && refreshedMetadata == nil {
if !errors.Is(err, disperser.ErrMetadataNotFound) {
s.logger.Error("error blob request or marshal error", "err", err)
return metadataKey, errProcessingMeetingPrecondition
}
// else it is expected
} else {
s.logger.Error("error get blob status abnormality ", "err", err)
return metadataKey, errProcessingMeetingPrecondition
}

err = s.s3Client.UploadObject(ctx, s.bucketName, blobObjectKey(blobHash), blob.Data)
if err != nil {
s.logger.Error("error uploading blob", "err", err)
Expand Down
Loading