Skip to content

DBFS API 429 retries: Wrap retry logic in a class to ensure isolation between decorated function calls.#327

Merged
bogdanghita-db merged 7 commits intodatabricks:masterfrom
bogdanghita-db:dbfs-429-retries-improvements-2
Oct 6, 2020
Merged

DBFS API 429 retries: Wrap retry logic in a class to ensure isolation between decorated function calls.#327
bogdanghita-db merged 7 commits intodatabricks:masterfrom
bogdanghita-db:dbfs-429-retries-improvements-2

Conversation

@bogdanghita-db
Copy link
Copy Markdown
Collaborator

@bogdanghita-db bogdanghita-db commented Sep 23, 2020

This PR refactors the retry_429 decorator, changing it from a function to a class, such that we can persist the time_for_last_retry separately for each decorated function, instead of using a global variable.

Manually tested and user experience is unchanged:

Received 429 REQUEST_LIMIT_EXCEEDED for attempt 1. Retrying in 0.15 seconds.
Received 429 REQUEST_LIMIT_EXCEEDED for attempt 2. Retrying in 0.48 seconds.
Received 429 REQUEST_LIMIT_EXCEEDED for attempt 3. Retrying in 2.98 seconds.
Received 429 REQUEST_LIMIT_EXCEEDED for attempt 4. Retrying in 5.03 seconds.
Received 429 REQUEST_LIMIT_EXCEEDED for attempt 5. Retrying in 8.55 seconds.
Received 429 REQUEST_LIMIT_EXCEEDED for attempt 6. Retrying in 7.08 seconds.
Received 429 REQUEST_LIMIT_EXCEEDED for attempt 7. Retrying in 5.44 seconds.
Error: RateLimitException: 429 Too Many Requests

self.client = DbfsService(api_client)

@retry_429
@Retry429
Copy link
Copy Markdown
Collaborator Author

@bogdanghita-db bogdanghita-db Sep 24, 2020

Choose a reason for hiding this comment

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

It's a class now, so name must be camel case.

@bogdanghita-db bogdanghita-db merged commit 6e2da08 into databricks:master Oct 6, 2020
bogdanghita-db added a commit to bogdanghita-db/databricks-cli that referenced this pull request Oct 16, 2020
…solation between decorated function calls. (databricks#327)"

This reverts commit 6e2da08.
bogdanghita-db added a commit that referenced this pull request Oct 20, 2020
…all databricks-cli operations (#343)

Revert retry logic on 429 responses for DBFS API requests (#319, #326, #327) and reimplement it using [urllib3.util.Retry](https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html), for all the requests made by `databricks-cli`.

* We perform a maximum number of 6 retries with exponential backoff, resulting in the following delays between them: 0.5, 1, 2, 4, 8, 16 (seconds). The `Retry-After` header is respected if present.
* There is no message logged for each retry (I could not find a way to do it using `urllib3.util.Retry`).
* If all retry attempts fail, the original 429 http response is forwarded by the retry utility.
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.

2 participants