Skip to content

Back off between import retries and report why one failed - #136

Open
chhtz wants to merge 3 commits into
masterfrom
feature/import-retry-backoff
Open

chhtz wants to merge 3 commits into
masterfrom
feature/import-retry-backoff

Conversation

@chhtz

@chhtz chhtz commented Oct 6, 2026

Copy link
Copy Markdown
Member

Before this, 10 retries fired in ~3s with no delay, i.e., any outage longer than a few seconds fails the import.
It also retried errors which are unlikely to recover, like missing repository.

Default retry_base_delay=0 does not change behavior, to change this, write e.g. this into your init.rb:

Autoproj.post_import do |pkg|
    pkg.importer.retry_base_delay = 2    # 2,4,8,16,32,60...
end

@chhtz
chhtz requested review from doudou and g-arjones October 6, 2026 15:44
Add retry_base_delay and retry_max_delay next to retry_count. The delay
doubles per attempt up to the maximum; both default to the previous
behavior of retrying immediately.

Only retry errors that can plausibly succeed later, and name the error
in the retry message so the reason is visible at normal verbosity.
@chhtz
chhtz force-pushed the feature/import-retry-backoff branch from 808aa51 to b42dd0e Compare October 6, 2026 15:47
@chhtz

chhtz commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

Force-pushed rubocup fix

Comment thread lib/autobuild/importer.rb Outdated
# Errors that may well succeed on a later attempt. Anything not matched
# here is treated as permanent: a missing repository or a rejected
# credential does not get better by asking again.
TRANSIENT_ERROR_RX = /

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of trying to exhaustively list transient errors and hoping that we have all of them (thus changing the current retry behaviour), I would have preferred to take the other route and identify non-transient errors. It would have less impact in the case of a non-transient errors that is misclassified (would take longer to fail, while in the current case of a misclassified transient, we get a failure where we had none previously)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. I inverted the behavior (and renamed it to permanent_error?) Also added some tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are also some transient errors in-between the HTTP 4xx: 408 Request Timeout, 429 Too Many Requests. Maybe just list 401, 403, 404, 410 for the most probable permanent status codes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done. I assume this is what you meant (i.e., still keep the non HTTP-code messages)

@chhtz
chhtz force-pushed the feature/import-retry-backoff branch from e725b76 to 83a21da Compare October 13, 2026 09:30

@doudou doudou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that Pierre's comment makes sense on HTTP codes ... just fix that and good to go for me.

Also, git actually prints `... error: 403` not `... error 403`.
@chhtz

chhtz commented Oct 22, 2026

Copy link
Copy Markdown
Member Author

Test actions appear to be stuck in "install subversion" phase ...

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.

3 participants