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

Add retry logic to Anidex #11318

Merged
merged 6 commits into from Mar 15, 2021
Merged

Conversation

6cUbi57z
Copy link
Contributor

Anidex seems to have been having quite a lot of issues ongoing for a few months now. The pages are returning 500 errors but if you refresh a few times the page loads. Obviously the ideal solution would be for Anidex to resolve this but not much seems to be happening there and this is causing the indexer to go offline in Sonarr.

This change adds retry logic using Polly. The retries are configured with a delay between which increases exponentially in order to reduce strain on the server. It will also log a warning message when an error response is received so that the attempts can be tracked.

The default has been set not to retry as the delay will increase the API response times.

Apologies if this isn't meeting coding standards. My VS install is broken so I'm having to use VSCode.

There was already some retry logic there so this is making it consistent and easier to override or make user configurable.
@6cUbi57z
Copy link
Contributor Author

Updated following discussion in #11221 .

  • The duplicated retry logic has now been removed. Most of the changes in Anidex have been rolled back.
  • The retry logic in BaseWebIndexer (existing previous to this PR) have now been modified to use Polly.
    • The default is 3 retry attempts (same as previous) with the delay starting at 0.5s and increasing exponentially. This is different to previous (always 0.5s), however, there was an additional wait before (there was a delay in the last loop after the last exception). This means that the time to failure should be the same.

I have also added some properties and methods to the BaseWebIndexer to simplify the retries per indexer. To change the number of retries for a particular indexer in code, you can override the NumberOfRetryAttempts property:

protected override int NumberOfRetryAttempts => 5;

Alternatively, a user-configurable setting can be added to an indexer by calling the EnableConfigurableRetryAttempts(); method from the constructor of an indexer. The default can be changed by overriding the DefaultNumberOfRetryAttempts property.

This keeps the UI for retries consistent and reduces code duplication, however, it restricts indexers from using the retryAttempts config property as it is used by the base class. It may be possible to add some protection against this, or it can be removed if we think configuration is best handled completely in the subclasses.

Two indexers with known stability issues have had user configurable retries enabled: Anidex and RARBG (#11221).

Copy link
Contributor

@garfield69 garfield69 left a comment

Choose a reason for hiding this comment

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

Tested OK.
Mind you, as is usually the case for me, when I tested both RARBG and Anidex I could not get them to fail so I don't get to see the retry in action ;-D

src/Jackett.Common/Indexers/Anidex.cs Outdated Show resolved Hide resolved
src/Jackett.Common/Indexers/BaseIndexer.cs Outdated Show resolved Hide resolved
@garfield69 garfield69 merged commit 72ef99d into Jackett:master Mar 15, 2021
@6cUbi57z
Copy link
Contributor Author

Thanks @garfield69 !

Have a look at Mockoon (https://mockoon.com/). You may have to tweak some URLs so that the indexers point at localhost rather than the actual sites but it should let you force failure responses. Also handy for development so that you don't have to keep hitting the sites.

@ilike2burnthing
Copy link
Contributor

v0.17.710

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.

None yet

3 participants