Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Always update retry_last_ts #16164

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Always update retry_last_ts #16164

merged 2 commits into from
Aug 23, 2023

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Aug 23, 2023

Follow on from #16156

I think by updating retry_last_ts we recover from the failure modes quicker (as we don't need to wait for the retry interval to ramp up?)

@erikjohnston erikjohnston marked this pull request as ready for review August 23, 2023 09:50
@erikjohnston erikjohnston requested a review from a team as a code owner August 23, 2023 09:50
Copy link
Contributor

@MatMaul MatMaul left a comment

Choose a reason for hiding this comment

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

After more thinking, is the WHERE clause really needed at all ? set_destination_retry_timings is only called in 2 places, and it seems to me that it is checking stuffs that should always be valid.

@erikjohnston
Copy link
Member Author

After more thinking, is the WHERE clause really needed at all ? set_destination_retry_timings is only called in 2 places, and it seems to me that it is checking stuffs that should always be valid.

I think the WHERE clause is mostly just there to help with the case where two workers are both trying to update the table at the same time. You're right that it might be easier to just remove it, but I'd want to investigate a little bit more before doing so.

Copy link
Contributor

@MatMaul MatMaul left a comment

Choose a reason for hiding this comment

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

Ah yes workers!

@erikjohnston erikjohnston merged commit 86ecd34 into develop Aug 23, 2023
37 checks passed
@erikjohnston erikjohnston deleted the erikj/destinations_fix branch August 23, 2023 12:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants