You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/03/19 13:33:28 UTC
[GitHub] [knox] moresandeep opened a new pull request #419: KNOX-2543 - Add ability to retry failed requests
moresandeep opened a new pull request #419:
URL: https://github.com/apache/knox/pull/419
## What changes were proposed in this pull request?
Add option to replay requests when the server unexpectedly closes the connection (not in case of socket timeouts where the server is unable to reach)
Proposed changes are adding optional service params
1. `retryCount` - how many times should a request be retried
2. `retryNonSafeRequest` - Should an unsafe request be retries, unsafe = POST, PUT, DELETE (!GET)
example
`
<service>
<role>WHOAMI</role>
<url>http://localhost:50071</url>
<param name="retryCount" value="5"/>
</service>
`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino commented on a change in pull request #419: KNOX-2543 - Add ability to retry failed requests
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #419:
URL: https://github.com/apache/knox/pull/419#discussion_r604079180
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
+ /* do we want to retry non-idempotent requests? default no */
+ boolean retryNonIdempotent = DEFAULT_PARAMETER_RETRY_NON_SAFE_REQUEST;
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST)
+ != null) {
+ retryNonIdempotent = Boolean.parseBoolean(
+ filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST));
+ } builder.setRetryHandler(new DefaultHttpRequestRetryHandler(Integer
Review comment:
nit: Should this be on a new line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep commented on a change in pull request #419: KNOX-2543 - Add ability to retry failed requests
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #419:
URL: https://github.com/apache/knox/pull/419#discussion_r604108668
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
+ /* do we want to retry non-idempotent requests? default no */
+ boolean retryNonIdempotent = DEFAULT_PARAMETER_RETRY_NON_SAFE_REQUEST;
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST)
+ != null) {
+ retryNonIdempotent = Boolean.parseBoolean(
+ filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST));
+ } builder.setRetryHandler(new DefaultHttpRequestRetryHandler(Integer
Review comment:
Ah right, I'll change that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep merged pull request #419: KNOX-2543 - Add ability to retry failed requests
Posted by GitBox <gi...@apache.org>.
moresandeep merged pull request #419:
URL: https://github.com/apache/knox/pull/419
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep commented on a change in pull request #419: KNOX-2543 - Add ability to retry failed requests
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #419:
URL: https://github.com/apache/knox/pull/419#discussion_r604111507
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
+ /* do we want to retry non-idempotent requests? default no */
+ boolean retryNonIdempotent = DEFAULT_PARAMETER_RETRY_NON_SAFE_REQUEST;
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST)
+ != null) {
+ retryNonIdempotent = Boolean.parseBoolean(
+ filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST));
+ } builder.setRetryHandler(new DefaultHttpRequestRetryHandler(Integer
+ .parseInt(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT)),
Review comment:
i mean we could but than I don't see a reason to create an extra variable, I'll extract it in a local variable to make it easy on the eyes :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #419: KNOX-2543 - Add ability to retry failed requests
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #419:
URL: https://github.com/apache/knox/pull/419#discussion_r604088238
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
Review comment:
nit: this logic worths a private boolean method IMO
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
+ /* do we want to retry non-idempotent requests? default no */
+ boolean retryNonIdempotent = DEFAULT_PARAMETER_RETRY_NON_SAFE_REQUEST;
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST)
+ != null) {
+ retryNonIdempotent = Boolean.parseBoolean(
+ filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST));
+ } builder.setRetryHandler(new DefaultHttpRequestRetryHandler(Integer
Review comment:
+1
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
+ /* do we want to retry non-idempotent requests? default no */
+ boolean retryNonIdempotent = DEFAULT_PARAMETER_RETRY_NON_SAFE_REQUEST;
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST)
+ != null) {
+ retryNonIdempotent = Boolean.parseBoolean(
+ filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST));
+ } builder.setRetryHandler(new DefaultHttpRequestRetryHandler(Integer
+ .parseInt(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT)),
Review comment:
`filterConfig.getInitParameter(PARAMETER_RETRY_COUNT)` could be extracted to a local variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino commented on a change in pull request #419: KNOX-2543 - Add ability to retry failed requests
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #419:
URL: https://github.com/apache/knox/pull/419#discussion_r604079180
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
+ /* do we want to retry non-idempotent requests? default no */
+ boolean retryNonIdempotent = DEFAULT_PARAMETER_RETRY_NON_SAFE_REQUEST;
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST)
+ != null) {
+ retryNonIdempotent = Boolean.parseBoolean(
+ filterConfig.getInitParameter(PARAMETER_RETRY_NON_SAFE_REQUEST));
+ } builder.setRetryHandler(new DefaultHttpRequestRetryHandler(Integer
Review comment:
nit: Should the builder invocation be on a new line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep commented on a change in pull request #419: KNOX-2543 - Add ability to retry failed requests
Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #419:
URL: https://github.com/apache/knox/pull/419#discussion_r604109408
##########
File path: gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
##########
@@ -119,6 +126,19 @@ public HttpClient createHttpClient(FilterConfig filterConfig) {
// See KNOX-1530 for details
builder.disableContentCompression();
+ if (filterConfig.getInitParameter(PARAMETER_RETRY_COUNT) != null
+ && StringUtils
+ .isNumeric(filterConfig.getInitParameter(PARAMETER_RETRY_COUNT))) {
Review comment:
yeah, I can brach this out to a private method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org