feat: Retry API calls that return a 500 error #2041

hessjcg · 2024-07-15T18:54:15Z

This updates the retry logic for API calls made by the connector:

Attempts to get an authentication token from the Google auth API are always retried 5 times.
SQL Admin API requests are retried up to 5 times if the http response status code is a 5xx. Otherwise they are fatal on the first error.

This also implements the exponential backoff logic defined used in the go connector.
See: GoogleCloudPlatform/cloud-sql-go-connector#781

jackwotherspoon

couple nits, will let @enocom review for Java readability

should we make this a PR a fix:?

jackwotherspoon · 2024-07-15T19:47:00Z

core/src/main/java/com/google/cloud/sql/core/ApiClientRetryingCallable.java

+   * response with an error code in the 500 range.
+   *
+   * @param e the exception
+   * @return false if this is a http response with a 500 status code, otherwise true.


Suggested change

* response with an error code in the 500 range.

*

* @param e the exception

* @return false if this is a http response with a 500 status code, otherwise true.

* response with an error code in the 5xx range.

*

* @param e the exception

* @return false if this is a http response with a 5xx status code, otherwise true.

jackwotherspoon · 2024-07-15T19:49:33Z

core/src/main/java/com/google/cloud/sql/core/RetryingCallable.java

+ *
+ * <p>The formula is: base * multi^(attempt + 1 + random)
+ *
+ * <p>With base = 200ms and multi = 1.1618, and random = [0.0, 1.0), the backoff values would fall


nit: value of multi should be 1.618

Suggested change

* <p>With base = 200ms and multi = 1.1618, and random = [0.0, 1.0), the backoff values would fall

* <p>With base = 200ms and multi = 1.618, and random = [0.0, 1.0), the backoff values would fall

jackwotherspoon · 2024-07-15T19:52:10Z

core/src/test/java/com/google/cloud/sql/core/ApiClientRetryingCallableTest.java

+        new ApiClientRetryingCallable<>(
+            () -> {
+              int attempt = counter.incrementAndGet();
+              if (attempt < 3) {


any reason for choosing 3 instead of 5 here?

I wanted to demonstrate that it returns after a successful attempt before exhausting the total number of retries.

I added another test that always fails to show that it stops at 5 attempts and then fails.

enocom · 2024-07-16T14:55:40Z

This should be a "feature" -- it will have a noticeable usability change.

enocom · 2024-07-16T14:59:33Z

core/src/main/java/com/google/cloud/sql/core/ApiClientRetryingCallable.java

+    // Only retry if the error is an HTTP response with a 5xx error code.
+    if (e instanceof HttpResponseException) {
+      HttpResponseException re = (HttpResponseException) e;
+      return re.getStatusCode() < 500 || re.getStatusCode() > 599;


When would re.getStatusCode() > 599 apply? I think re.getStatusCode() < 500 will get the job done.

hessjcg

I also added a new test to ConnectorTest. This test the SqlAdmin client library actually responds correctly and the ApiClientRetryingCallable works as expected when the service intermittently responds with 502 errors.

hessjcg · 2024-07-17T16:16:18Z

core/src/main/java/com/google/cloud/sql/core/ApiClientRetryingCallable.java

+   * response with an error code in the 500 range.
+   *
+   * @param e the exception
+   * @return false if this is a http response with a 500 status code, otherwise true.


hessjcg · 2024-07-17T16:16:34Z

core/src/main/java/com/google/cloud/sql/core/ApiClientRetryingCallable.java

+   * response with an error code in the 500 range.
+   *
+   * @param e the exception
+   * @return false if this is a http response with a 500 status code, otherwise true.


hessjcg · 2024-07-17T16:16:42Z

core/src/main/java/com/google/cloud/sql/core/ApiClientRetryingCallable.java

+    // Only retry if the error is an HTTP response with a 5xx error code.
+    if (e instanceof HttpResponseException) {
+      HttpResponseException re = (HttpResponseException) e;
+      return re.getStatusCode() < 500 || re.getStatusCode() > 599;


hessjcg · 2024-07-17T16:17:17Z

core/src/main/java/com/google/cloud/sql/core/RetryingCallable.java

+ *
+ * <p>The formula is: base * multi^(attempt + 1 + random)
+ *
+ * <p>With base = 200ms and multi = 1.1618, and random = [0.0, 1.0), the backoff values would fall


hessjcg · 2024-07-17T16:20:36Z

core/src/test/java/com/google/cloud/sql/core/ApiClientRetryingCallableTest.java

+        new ApiClientRetryingCallable<>(
+            () -> {
+              int attempt = counter.incrementAndGet();
+              if (attempt < 3) {


I wanted to demonstrate that it returns after a successful attempt before exhausting the total number of retries.

I added another test that always fails to show that it stops at 5 attempts and then fails.

hessjcg requested a review from a team as a code owner July 15, 2024 18:54

blunderbuss-gcf bot assigned jackwotherspoon Jul 15, 2024

hessjcg requested review from jackwotherspoon, enocom and ttosta-google July 15, 2024 18:54

hessjcg force-pushed the gh-2029-retry-api-error-500 branch from ebc9d0a to 5faf4ec Compare July 15, 2024 18:55

jackwotherspoon approved these changes Jul 15, 2024

View reviewed changes

enocom changed the title ~~chore: Retry API calls that return a 500 error. Fixes #2029.~~ feat: Retry API calls that return a 500 error Jul 16, 2024

enocom reviewed Jul 16, 2024

View reviewed changes

hessjcg commented Jul 17, 2024

View reviewed changes

hessjcg force-pushed the gh-2029-retry-api-error-500 branch from 5faf4ec to 2e8c190 Compare July 17, 2024 16:50

hessjcg requested a review from jackwotherspoon July 17, 2024 16:50

hessjcg force-pushed the gh-2029-retry-api-error-500 branch from 2e8c190 to e1380fe Compare July 17, 2024 16:50

hessjcg enabled auto-merge (squash) July 17, 2024 16:52

chore: Retry API calls that return a 500 error. Fixes #2029.

cf3d027

hessjcg force-pushed the gh-2029-retry-api-error-500 branch from e1380fe to cf3d027 Compare July 17, 2024 18:09

hessjcg merged commit d76e892 into main Jul 17, 2024
17 checks passed

hessjcg deleted the gh-2029-retry-api-error-500 branch July 17, 2024 18:16

release-please bot mentioned this pull request Jul 17, 2024

chore(main): release 1.20.0 #2042

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Retry API calls that return a 500 error #2041

feat: Retry API calls that return a 500 error #2041

hessjcg commented Jul 15, 2024

jackwotherspoon left a comment

jackwotherspoon Jul 15, 2024

hessjcg Jul 17, 2024

hessjcg Jul 17, 2024

jackwotherspoon Jul 15, 2024

hessjcg Jul 17, 2024

jackwotherspoon Jul 15, 2024

hessjcg Jul 17, 2024

enocom commented Jul 16, 2024

enocom Jul 16, 2024

hessjcg Jul 17, 2024

hessjcg left a comment

hessjcg Jul 17, 2024

hessjcg Jul 17, 2024

hessjcg Jul 17, 2024

hessjcg Jul 17, 2024

hessjcg Jul 17, 2024

	* <p>With base = 200ms and multi = 1.1618, and random = [0.0, 1.0), the backoff values would fall
	* <p>With base = 200ms and multi = 1.618, and random = [0.0, 1.0), the backoff values would fall

feat: Retry API calls that return a 500 error #2041

feat: Retry API calls that return a 500 error #2041

Conversation

hessjcg commented Jul 15, 2024

jackwotherspoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enocom commented Jul 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hessjcg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment