add check when initializing processing state #744
Conversation
bxue-l2
requested review from
jianoaix,
mooselumph and
ian-shim
and removed request for
jianoaix
| @@ -90,6 +92,12 @@ func (s *SharedBlobStore) StoreBlob(ctx context.Context, blob *core.Blob, reques | |||
| metadataKey.BlobHash = blobHash | |||
| metadataKey.MetadataHash = metadataHash | |||
|
|
|||
| refreshedMetadata, err := s.GetBlobMetadata(ctx, metadataKey) | |||
There was a problem hiding this comment.
what if there is a 500 error from dynamo?
There was a problem hiding this comment.
I think I should just check if refreshedMetadata is nil or not
| @@ -21,6 +21,8 @@ const ( | |||
|
|
|||
| var errProcessingToDispersing = errors.New("blob transit to dispersing from non processing") | |||
|
|
|||
| var errProcessingInitialization = errors.New("blob status initialization") | |||
There was a problem hiding this comment.
I'd call this not meeting the precondition
| refreshedMetadata, err := s.GetBlobMetadata(ctx, metadataKey) | ||
| if !errors.Is(err, disperser.ErrMetadataNotFound) { | ||
| s.logger.Error("error intentionally get blob status before creation", "err", err, "status", refreshedMetadata.BlobStatus, "metadataKey", refreshedMetadata.GetBlobKey().String()) | ||
| return metadataKey, errProcessingInitialization |
There was a problem hiding this comment.
what is the gRPC error code that will be returned to end user from here?
There was a problem hiding this comment.
this would be this
api.NewInternalError("failed to store blob, please try again later")
There was a problem hiding this comment.
It makes sense, because later users requestTime will be different
There was a problem hiding this comment.
This seems good point: How would users store the same metadataKey twice? Even if sending the same blob, it'll result in different metadataKey, so there is no overwrite.
| @@ -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") | |||
There was a problem hiding this comment.
errProcessingFailedPrecondition("blob metadata already exists....")?
| refreshedMetadata, err := s.GetBlobMetadata(ctx, metadataKey) | ||
|
|
||
| // the only safe condition: | ||
| // err is disperser.ErrMetadataNotFound && refreshedMetadata is nil |
There was a problem hiding this comment.
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
This PR adds checks in StoreBlob, called by apiserver when first receiving a blob
This PR inserts a s.GetBlobMetadata before uploading the blob on S3 and setting blobMetaData. The expected behaviors is that such metadata should not ever exists. If it does, return err