You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "hlteoh37 (via GitHub)" <gi...@apache.org> on 2024/09/24 08:26:52 UTC
[PR] [FLINK-36357][Connectors/Kinesis] Set up AWS SDK default retry strate… [flink-connector-aws]
hlteoh37 opened a new pull request, #170:
URL: https://github.com/apache/flink-connector-aws/pull/170
…gy configurations
## Purpose of the change
Configure SDK retry default configurations. This helps with classification of retryable errors out of the box.
## Verifying this change
- Manually run connector to verify that the application doesn't restart as frequently on connection loss!
## Significant changes
*(Please check any boxes [x] if the answer is "yes". You can first publish the PR and check them afterwards, for convenience.)*
- [ ] Dependencies have been added or upgraded
- [ ] Public API has been changed (Public API is any class annotated with `@Public(Evolving)`)
- [ ] Serializers have been changed
- [ ] New feature has been introduced
- If yes, how is this documented? (not applicable / docs / JavaDocs / not documented)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36357][Connectors/Kinesis] Set up AWS SDK default retry strate… [flink-connector-aws]
Posted by "karubian (via GitHub)" <gi...@apache.org>.
karubian commented on code in PR #170:
URL: https://github.com/apache/flink-connector-aws/pull/170#discussion_r1772935727
##########
flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/source/KinesisStreamsSource.java:
##########
@@ -254,31 +252,10 @@ private RetryStrategy createExpBackoffRetryStrategy(
final BackoffStrategy backoffStrategy =
BackoffStrategy.exponentialDelayHalfJitter(initialDelay, maxDelay);
- return StandardRetryStrategy.builder()
+ return SdkDefaultRetryStrategy.standardRetryStrategyBuilder()
.backoffStrategy(backoffStrategy)
.throttlingBackoffStrategy(backoffStrategy)
.maxAttempts(maxAttempts)
Review Comment:
Circuit breaker is enabled in this strategy by default. Is this intended?
##########
flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/source/KinesisStreamsSource.java:
##########
@@ -254,31 +252,10 @@ private RetryStrategy createExpBackoffRetryStrategy(
final BackoffStrategy backoffStrategy =
BackoffStrategy.exponentialDelayHalfJitter(initialDelay, maxDelay);
- return StandardRetryStrategy.builder()
+ return SdkDefaultRetryStrategy.standardRetryStrategyBuilder()
Review Comment:
Do we have any thoughts on how this would work with a user defined retryable exceptions improvement like the one in the old connector?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36357][Connectors/Kinesis] Set up AWS SDK default retry strate… [flink-connector-aws]
Posted by "hlteoh37 (via GitHub)" <gi...@apache.org>.
hlteoh37 merged PR #170:
URL: https://github.com/apache/flink-connector-aws/pull/170
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36357][Connectors/Kinesis] Set up AWS SDK default retry strate… [flink-connector-aws]
Posted by "hlteoh37 (via GitHub)" <gi...@apache.org>.
hlteoh37 commented on code in PR #170:
URL: https://github.com/apache/flink-connector-aws/pull/170#discussion_r1772943120
##########
flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/source/KinesisStreamsSource.java:
##########
@@ -254,31 +252,10 @@ private RetryStrategy createExpBackoffRetryStrategy(
final BackoffStrategy backoffStrategy =
BackoffStrategy.exponentialDelayHalfJitter(initialDelay, maxDelay);
- return StandardRetryStrategy.builder()
+ return SdkDefaultRetryStrategy.standardRetryStrategyBuilder()
.backoffStrategy(backoffStrategy)
.throttlingBackoffStrategy(backoffStrategy)
.maxAttempts(maxAttempts)
Review Comment:
I was wondering that - is there a reason we really don't want to enable circuit breaker for our setup?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36357][Connectors/Kinesis] Set up AWS SDK default retry strate… [flink-connector-aws]
Posted by "hlteoh37 (via GitHub)" <gi...@apache.org>.
hlteoh37 commented on code in PR #170:
URL: https://github.com/apache/flink-connector-aws/pull/170#discussion_r1772950453
##########
flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/source/KinesisStreamsSource.java:
##########
@@ -254,31 +252,10 @@ private RetryStrategy createExpBackoffRetryStrategy(
final BackoffStrategy backoffStrategy =
BackoffStrategy.exponentialDelayHalfJitter(initialDelay, maxDelay);
- return StandardRetryStrategy.builder()
+ return SdkDefaultRetryStrategy.standardRetryStrategyBuilder()
Review Comment:
> Do we have any thoughts on how this would work with a user defined retryable exceptions improvement like the one in the old connector?
Good qn - we can add more predicates by calling `retryOnException()`, but we won't be able to override the ones already marked as non-retryable in the SDK defaults, because I think it does eager checks for non-retryable errors.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36357][Connectors/Kinesis] Set up AWS SDK default retry strate… [flink-connector-aws]
Posted by "karubian (via GitHub)" <gi...@apache.org>.
karubian commented on code in PR #170:
URL: https://github.com/apache/flink-connector-aws/pull/170#discussion_r1772955157
##########
flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/source/KinesisStreamsSource.java:
##########
@@ -254,31 +252,10 @@ private RetryStrategy createExpBackoffRetryStrategy(
final BackoffStrategy backoffStrategy =
BackoffStrategy.exponentialDelayHalfJitter(initialDelay, maxDelay);
- return StandardRetryStrategy.builder()
+ return SdkDefaultRetryStrategy.standardRetryStrategyBuilder()
.backoffStrategy(backoffStrategy)
.throttlingBackoffStrategy(backoffStrategy)
.maxAttempts(maxAttempts)
Review Comment:
I don't think so. I believe it will be useful, we just need to document this as a new change as it might be surprising for users.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org