Query request retry policy may not be working

[azure]

Query request retry policy may not be working

[rnagpal]

I haven't tested this completely but

Can you try replacing:

self.retry_after_in_milliseconds = exception.retry_after_in_milliseconds

with:

self.retry_after_in_milliseconds = exception.headers[http_constants.HttpHeaders.RetryAfterInMilliseconds]

and see if that resolves the issue.

Regards,
Rajesh

[bmachin]

Hi Rajesh, first of all, thanks for your great work!

I have two things to comment here:

1. In the Execute function, notice that sleep is called with a parameter in milliseconds, but it expects it in seconds, so, it will sleep for way too long.
2. In the _CheckIfRetryNeeded function, the headers attribute of the exception is a list of tuples, in the form (key, value). So, it should either be transformed into a dict, or we should perform a search for the tuple with key http_constants.HttpHeaders.RetryAfterInMilliseconds to get the milliseconds value.

I hope that helps.

Best regards,
Benjamín.-

[xou]

Hi, i ran into the same problem, implemented the changes @machinbenjamin suggested and created a pull request (#35).

Best Regards,

Niko

Thanks for the PR. if you could add a new test for this PR we will accept and push a new version.

[xou]

Hi,

thanks for the quick response. Is there a recommended way to test this kind of error? As far as I can see, the error tests in test/crut_test.py connect to an actual documentdb instance and rely on an invalid request, such missing permissions. Testing that a request-rate exceeded error is handled correctly using an actual documentdb instance would depend a lot on timing (and the connection speed of the machine the test is running on).

So, basically, I could see two approaches:

• Spam a documentdb collection with ~50k requests as fast as possible, and hope that at least one of them throws a rate limit exceeded/retry-after. So basically, that something like for i in range(0, 50000): document_client.query("SELECT * FROM coll_0") does not throw an exception (?)
• Fake/Mock the connection to the documentdb, for example by creating something like a MockHTTPsConnection, and overwriting the HTTPsConnection class in the pydocumentdb.https_connection module for the unit test.

What would you recommend? Are there maybe similar tests in the dotnet/java documentdb clients? (I had a look, but couldn't find any....)

Regards,
Niko

[xou]

If you're interested, i've hacked together a testcase for approach 2 here:

https://gist.github.com/xou/f97547b98a38ea8e8f03

The "on continuation" testcase fails with the old version, and succeeds with the PR applied. The "fail immediately" testcase fails in both versions, unsure where the problem is exactly (can look into it later).

looks good. are you able to add the test in to the existing tests - https://github.com/Azure/azure-documentdb-python/tree/master/test

[xou]

Hi,

I've cleaned the test up a bit and added it to the PR. As mentioned above, one test currently fails and is disabled in the PR. Maybe someone with more Azure experience than me can comment on this, how does Azure respond when the rate limit is exceeded on the first request? Is x-ms-continuation set in that case?

I'm guessing that the problem might be that in query_iterable.py/fetch_items, the if not self.has_started: self._has_started = True should happen after self._fetch_function(...) has been called. self._fetch_function might throw an exception (after _has_started is set), and if the BackoffRetryUtility calls fetch_items again, it will not enter the while loop and return immediately; with an empty result.

Cheers,

Niko

awesome. thank you. will take a look now.

if you hit the 429 on the first request you will not get a continuation token. it will simply fail with the 429 and the retry-after header will be set in the response.

fixed with PR #35