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.

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

Mark failed deal data transfer as failed and stop restarting it #304

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

en0ma
Copy link
Contributor

@en0ma en0ma commented Jun 29, 2022

Mark failed deal data transfer as failed so it won't be picked up perpetually for retry.

closes #305

@en0ma en0ma requested a review from alvin-reyes June 29, 2022 16:47
@en0ma en0ma changed the title update failed and stop retrying Mark failed deal data transfer as failed Jun 29, 2022
@en0ma en0ma changed the title Mark failed deal data transfer as failed Mark failed deal data transfer as failed and stop restarting it Jun 29, 2022
if loc == "local" {
st, err := cm.FilClient.TransferStatus(ctx, &chanid)
if err != nil {
return err
}

if util.TransferTerminated(st) {
return fmt.Errorf("deal in database as being in progress, but data transfer is terminated: %d", st.Status)
if err := cm.DB.Model(contentDeal{}).Where("id = ?", dealID).UpdateColumns(map[string]interface{}{
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to store the status if we're going to restart the transfer anyway (line 847)? Just concern that we have a DB call here to update it to failed only to do another restart transfer which technically will be "in-progress".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

look closely, if TransferTerminated (meaning is it useless to restart), we mark it as failed and return the deal in database is in progress, but data transfer is terminated error (line 847 wont get called). The marking is useful in that, whenever estuary is restarted or restart is trigger, deals marked as failed will not be picked from db - improved db calls and less error logs

Copy link
Contributor

@alvin-reyes alvin-reyes left a comment

Choose a reason for hiding this comment

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

LGTM

@en0ma en0ma merged commit c5707ea into master Jul 7, 2022
@en0ma en0ma deleted the update-failed-deal branch July 7, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed deal data transfer are always restarted
2 participants