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

transient_error_retry - Retry transient errors when attempting to insert_tasks, and re-raise if the reinsert fails. #155

Merged
merged 1 commit into from
Oct 6, 2014

Conversation

markshaule-wf
Copy link
Contributor

Instead of silently failing on a TransientError on task inserts, we check the options to see if we should retry the failure (after a delay). If the retry fails, we re-raise.

Currently, the default option is to retry. I can't think of a scenario that the called would not want to have the errors retry, or at least trigger an error.
As a side effect, this may break custom implementations of _insert_tasks since we have added a parameter.

@beaulyddon-wf @robertkluin-wf @johnlockwood-wf @tannermiller-wf @rosshendrickson-wf @macleodbroad-wf @tylertreat-wf

FYI: @jasonaguilon-wf @erikpetersen-wf

…ert_tasks, and re-raise if the reinsert fails.
@erikpetersen-wf
Copy link
Member

Agree, why even have a kwarg to indicate raising an error, that should be a default?

@erikpetersen-wf
Copy link
Member

Why is there a backoff on performing the retry?

@tylertreat-wf
Copy link
Contributor

@erikpetersen-wf TransientError is typically a result of the queue being unavailable or contended for some reason. Google recommends a small backoff when you're seeing these types of errors.

@tylertreat-wf
Copy link
Contributor

To clarify, the current behavior for how TransientError is handled is not to fail silently but to split the batch into two and re-insert. Is this mainly to handle the case where len(tasks) == 1? Also I suppose TransientErrors tend to be mitigated with a backoff, so this probably makes sense.

@markshaule-wf
Copy link
Contributor Author

@erikpetersen-wf - Yes we do have kwarg argument for the retry, and the default is 'True' at this point.

@markshaule-wf
Copy link
Contributor Author

@tylertreat - Yes the current behaviour is to split and retry on TransientError. The issue is when we get down to 1 task and hit a Transient error, we fail silently. The other exception types (BadTaskStateError, TaskAlreadyExistsError, TombstonedTaskError), as I understand, indicate that the task has already been inserted, and we don't need to re-raise the exception.

But yes - that was the main fix - for len(tasks)==1 and a TransientError

@tylertreat-wf
Copy link
Contributor

+1

3 similar comments
@tannermiller-wf
Copy link
Contributor

+1

@jasonaguilon-wf
Copy link
Contributor

+1

@beaulyddon-wf
Copy link
Contributor

+1

beaulyddon-wf added a commit that referenced this pull request Oct 6, 2014
transient_error_retry - Retry transient errors when attempting to insert_tasks, and re-raise if the reinsert fails.
@beaulyddon-wf beaulyddon-wf merged commit 22967e5 into Workiva:master Oct 6, 2014
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.

None yet

6 participants