You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@bahir.apache.org by romeokienzler <gi...@git.apache.org> on 2017/08/24 09:25:53 UTC
[GitHub] bahir pull request #49: BAHIR-130
GitHub user romeokienzler opened a pull request:
https://github.com/apache/bahir/pull/49
BAHIR-130
In case number of requests per second are limited; just wait for a second and try again
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/romeokienzler/bahir master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/bahir/pull/49.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #49
----
commit 478ca23d04915fce812ee5bf98433bcf66a58cfc
Author: Romeo Kienzler <ro...@ch.ibm.com>
Date: 2017-08-24T09:24:28Z
BAHIR-130
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bahir issue #49: BAHIR-130
Posted by ApacheBahir <gi...@git.apache.org>.
Github user ApacheBahir commented on the issue:
https://github.com/apache/bahir/pull/49
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bahir issue #49: BAHIR-130
Posted by ckadner <gi...@git.apache.org>.
Github user ckadner commented on the issue:
https://github.com/apache/bahir/pull/49
@romeokienzler -- are you still working on incorporating @emlaver review comments?
---
[GitHub] bahir pull request #49: BAHIR-130
Posted by emlaver <gi...@git.apache.org>.
Github user emlaver commented on a diff in the pull request:
https://github.com/apache/bahir/pull/49#discussion_r135042956
--- Diff: sql-cloudant/src/main/scala/org/apache/bahir/cloudant/common/JsonStoreDataAccess.scala ---
@@ -125,6 +125,19 @@ class JsonStoreDataAccess (config: CloudantConfig) {
val clRequest: HttpRequest = getClRequest(url, postData)
val clResponse: HttpResponse[String] = clRequest.execute()
+
+ //BAHIR-130 start
+ //In cloudant Lite number of requests pre second are limited.
+ //Just wait and try again
+ clResponse.code match {
+ case 429 => {
+ Thread sleep 1000
--- End diff --
Also, would be good to have logging for:
- When there's too many requests and attempting a retry with x seconds
- Any problems/interrupt during the 'backoff' wait time
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bahir issue #49: BAHIR-130
Posted by ckadner <gi...@git.apache.org>.
Github user ckadner commented on the issue:
https://github.com/apache/bahir/pull/49
@romeokienzler -- any updates or progress?
---
[GitHub] bahir pull request #49: BAHIR-130
Posted by emlaver <gi...@git.apache.org>.
Github user emlaver commented on a diff in the pull request:
https://github.com/apache/bahir/pull/49#discussion_r135038265
--- Diff: sql-cloudant/src/main/scala/org/apache/bahir/cloudant/common/JsonStoreDataAccess.scala ---
@@ -125,6 +125,19 @@ class JsonStoreDataAccess (config: CloudantConfig) {
val clRequest: HttpRequest = getClRequest(url, postData)
val clResponse: HttpResponse[String] = clRequest.execute()
+
+ //BAHIR-130 start
+ //In cloudant Lite number of requests pre second are limited.
+ //Just wait and try again
+ clResponse.code match {
+ case 429 => {
+ Thread sleep 1000
--- End diff --
If the server is busy, we don't want this to loop and keep trying forever. We should have a set number of retries (our other Cloudant java library uses 3 retries) and exit with a 429 error if we reach that limit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bahir issue #49: BAHIR-130
Posted by romeokienzler <gi...@git.apache.org>.
Github user romeokienzler commented on the issue:
https://github.com/apache/bahir/pull/49
Sorry for the late reply. I'm using my patch in productions since a couple of weeks with great success. So I'll update this thread and also JIRA by incorporating my findings so that we can agree how to proceed.
---
[GitHub] bahir pull request #49: BAHIR-130
Posted by romeokienzler <gi...@git.apache.org>.
Github user romeokienzler closed the pull request at:
https://github.com/apache/bahir/pull/49
---
[GitHub] bahir issue #49: BAHIR-130
Posted by romeokienzler <gi...@git.apache.org>.
Github user romeokienzler commented on the issue:
https://github.com/apache/bahir/pull/49
Will be fixed through migration to java-cloudant (see comments in [JIRA](https://issues.apache.org/jira/browse/BAHIR-130) for details)
---
[GitHub] bahir pull request #49: BAHIR-130
Posted by emlaver <gi...@git.apache.org>.
Github user emlaver commented on a diff in the pull request:
https://github.com/apache/bahir/pull/49#discussion_r135040382
--- Diff: sql-cloudant/src/main/scala/org/apache/bahir/cloudant/common/JsonStoreDataAccess.scala ---
@@ -125,6 +125,19 @@ class JsonStoreDataAccess (config: CloudantConfig) {
val clRequest: HttpRequest = getClRequest(url, postData)
val clResponse: HttpResponse[String] = clRequest.execute()
+
+ //BAHIR-130 start
+ //In cloudant Lite number of requests pre second are limited.
+ //Just wait and try again
+ clResponse.code match {
+ case 429 => {
+ Thread sleep 1000
--- End diff --
For each retry we should consider increasing the sleep 'backoff' time. Use something like `long sleepTime = initialSleep * Math.round(Math.pow(2, attempt))` to double the sleep time based on the number of retry attempts. Our other libraries start with an initial sleep time of 250 ms. Here's the [retry method](https://github.com/cloudant/java-cloudant/blob/515cf156751cd67d714a206d1bc39a5489f8c156/cloudant-http/src/main/java/com/cloudant/http/interceptors/Replay429Interceptor.java#L83.) in our java library that may be helpful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---