Add a defensive check on blob status #258
Conversation
| @@ -124,6 +124,10 @@ func (f *finalizer) updateBlobs(ctx context.Context, metadatas []*disperser.Blob | |||
| for _, m := range metadatas { | |||
| stageTimer := time.Now() | |||
| blobKey := m.GetBlobKey() | |||
| if m.BlobStatus != disperser.Confirmed { | |||
There was a problem hiding this comment.
So the suspicion is that confirmationMetadata.ConfirmationInfo is nil in L138, right?
What if m.BlobStatus is Confirmed, but is missing confirmationMetadata.ConfirmationInfo?
This would be an even more serious issue if that's the case
There was a problem hiding this comment.
It's failing either on confirmationMetadata being nil, or confirmationMetadata.ConfirmationInfo being nil, which are the only two pointers at L138.
The confirmationMetadata is created by https://github.com/Layr-Labs/eigenda/blob/master/disperser/common/blobstore/blob_metadata_store.go#L55, it doesn't return nil as long as err != nil (which already checked at L132). So confirmationMetadata.ConfirmationInfo being nil seems to be only case.
These blobs were created from L98 or L113 by using GetBlobMetadataByStatusWithPagination(ctx, disperser.Confirmed, ...), so it seems either 1) this function returned some blobs that are not actually Confirmed; or 2) the blob's status got changed from Confirmed to something else after that call. For 1), it's a dynamo query, should be right; for 2), do we only transition Confirmed to Finalized? I haven't tracked down further.
There was a problem hiding this comment.
Thanks for the explanation! So this logging should help us track down the root cause
| @@ -124,6 +124,10 @@ func (f *finalizer) updateBlobs(ctx context.Context, metadatas []*disperser.Blob | |||
| for _, m := range metadatas { | |||
| stageTimer := time.Now() | |||
| blobKey := m.GetBlobKey() | |||
| if m.BlobStatus != disperser.Confirmed { | |||
There was a problem hiding this comment.
Thanks for the explanation! So this logging should help us track down the root cause
Why are these changes needed?
Checks