Skip to content

Commit

Permalink
Fix: adjusting retry logic to avoid retrying successful job creation (#…
Browse files Browse the repository at this point in the history
…1879)

#1744 introduced retrying 200 responses, but the regex that we are using is too broad: [".*exceed.*rate.limit."] and in some cases it may catch a valid response.

The issue is that the current logic is scanning the whole response. In this PR the retry logic is modified to specifically check the error message from the response. For this purpose parsing logic was developed to extract error messages from responses. This logic is specifically designed for the [jobs.insert method response](https://cloud.google.com/bigquery/docs/reference/rest/v2/jobs/insert#response-body) (only case observed so far where a response with status code 200 might also return an error message).
  • Loading branch information
franklinWhaite authored Mar 3, 2022
1 parent 1bc9188 commit fd07533
Showing 1 changed file with 29 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.google.api.gax.retrying.TimedAttemptSettings;
import com.google.api.gax.retrying.TimedRetryAlgorithm;
import com.google.api.gax.retrying.TimedRetryAlgorithmWithContext;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import java.util.Iterator;
import java.util.UUID;
import java.util.concurrent.CancellationException;
Expand Down Expand Up @@ -107,9 +109,10 @@ private boolean shouldRetryBasedOnBigQueryRetryConfig(
/*
In some cases error messages may come without an exception
e.g. status code 200 with a rate limit exceeded for job create
in these cases there is now previousThrowable so we need to check previousResponse
in these cases there is no previousThrowable so we need
to check for error messages in previousResponse
*/
errorDesc = previousResponse.toString();
errorDesc = getErrorDescFromResponse(previousResponse);
}

if (errorDesc != null) {
Expand Down Expand Up @@ -212,4 +215,28 @@ private TimedAttemptSettings createNextAttemptBasedOnTiming(
}
return getTimedAlgorithm().createNextAttempt(previousSettings);
}

private String getErrorDescFromResponse(ResponseT previousResponse) {
/*
error messages may come without an exception and must be extracted from response
following logic based on response body of jobs.insert method, so far the only
known case where a response with status code 200 may contain an error message
*/
try {
JsonObject responseJson =
JsonParser.parseString(previousResponse.toString()).getAsJsonObject();
if (responseJson.has("status") && responseJson.getAsJsonObject("status").has("errorResult")) {
return responseJson
.getAsJsonObject("status")
.getAsJsonObject("errorResult")
.get("message")
.toString();
} else {
return null;
}
} catch (Exception e) {
// exceptions here implies no error message present in response, returning null
return null;
}
}
}

0 comments on commit fd07533

Please sign in to comment.