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

feat: add exponential backoff strategy for retry #140

Merged
merged 12 commits into from
Aug 12, 2020
Merged

feat: add exponential backoff strategy for retry #140

merged 12 commits into from
Aug 12, 2020

Conversation

bednar
Copy link
Contributor

@bednar bednar commented Aug 6, 2020

Closes #144

Proposed Changes

  1. added exponential backoff strategy for batch writes
    1. max_retries: the number of max retries when write fails
    2. max_retry_delay: the maximum delay when retrying write in milliseconds
  2. other http request doesn't have retry strategy
  3. possibility to configure retry for client by:
    from urllib3 import Retry
    
    from influxdb_client import InfluxDBClient
    
    retries = Retry(connect=5, read=2, redirect=5)
    client = InfluxDBClient(url="http://localhost:9999", token="my-token", org="my-org", retries=retries)

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • pytest tests completes successfully
  • Commit messages are in semantic format
  • Sign CLA (if not already signed)

@codecov-commenter
Copy link

codecov-commenter commented Aug 6, 2020

Codecov Report

Merging #140 into master will increase coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
+ Coverage   88.94%   89.14%   +0.19%     
==========================================
  Files          24       25       +1     
  Lines        1882     1916      +34     
==========================================
+ Hits         1674     1708      +34     
  Misses        208      208              
Impacted Files Coverage Δ
influxdb_client/api_client.py 67.66% <100.00%> (+0.12%) ⬆️
influxdb_client/client/influxdb_client.py 98.37% <100.00%> (ø)
influxdb_client/client/write/retry.py 100.00% <100.00%> (ø)
influxdb_client/client/write_api.py 97.53% <100.00%> (-0.05%) ⬇️
influxdb_client/rest.py 80.45% <100.00%> (+0.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfa0ac4...5f94acf. Read the comment docs.

@bednar bednar requested review from rhajek and rolincova August 6, 2020 09:46
:param int max_retry_delay: maximum delay when retrying write
"""

WRITES_RETRY_AFTER_STATUS_CODES = frozenset([429, 503])

Choose a reason for hiding this comment

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

I think we should include more codes, such as to handle the case in #144, where connection could not even be made, due to the backend service being down:

The batch item wasn't processed successfully because: HTTPSConnectionPool(host='us-central1-1.gcp.cloud2.influxdata.com', port=443): Max retries exceeded with url: /api/v2/write?org=<REDACTED>&bucket=<REDACTED>+Bucket&precision=ns (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7fa44a34b6d8>: Failed to establish a new connection: [Errno 110] Connection timed out'))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The all network errors are also handled. The status codes are only used for http responses with status codes.

@bednar bednar merged commit 31a3f1c into master Aug 12, 2020
@bednar bednar deleted the feat/retry branch August 12, 2020 07:45
@bednar bednar added this to the 1.10.0 milestone Aug 12, 2020
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.

Improve retry when influxdb backend is down
5 participants