Luke I think for [2] and [3] it would be a fair statement that may be they wanted to add a custom retry configuration. But [2] looks very specific in the sense it doesn’t allow client to be more flexible [3] is something that I feel can be moved up and made generic enough.
Jonothan Sorry for that, this was actually with regards to JdbcIO. My bad calling it S3. From: Jonothan Farr <[email protected]> Reply-To: "[email protected]" <[email protected]> Date: Thursday, April 16, 2020 at 7:07 PM To: "[email protected]" <[email protected]> Subject: Re: ** Configurable FluentBackoff for IO's ** Notice: This email is from an external sender. Maybe this is a separate conversation, but for AWS IOs specifically wouldn't it be better to use the AWS client's retry policy? Something similar to this: ``` @Override public AmazonS3ClientBuilder createBuilder(S3Options s3Options) { RetryPolicy retryPolicy = new RetryPolicy( PredefinedRetryPolicies.DEFAULT_RETRY_CONDITION, PredefinedRetryPolicies.DEFAULT_BACKOFF_STRATEGY, PredefinedRetryPolicies.DEFAULT_MAX_ERROR_RETRY, false); AmazonS3ClientBuilder builder = AmazonS3ClientBuilder.standard() .withClientConfiguration(PredefinedClientConfigurations.defaultConfig() .withRetryPolicy(retryPolicy)) .withCredentials(s3Options.getAwsCredentialsProvider()); ... ``` We had a similar discussion on https://github.com/apache/beam/pull/9765 about KinesisIO. I only bring it up because you mentioned configuring retries in S3. On Thu, Apr 16, 2020 at 1:57 PM Luke Cwik <[email protected]<mailto:[email protected]>> wrote: I was wondering why IOs went with their own retry configuration object instead of making FluentBackoff[1] public. Some examples are SnsIO[2] and SolrIO[3]. Was it because we thought that IOs would likely need specialized retry configuration that a general retry configuration class wouldn't apply? 1: https://github.com/apache/beam/blob/c3bd4854e879da65060de8cd259865a9b34742c7/sdks/java/core/src/main/java/org/apache/beam/sdk/util/FluentBackoff.java#L30 2: https://github.com/apache/beam/blob/da9e17288e8473925674a4691d9e86252e67d7d7/sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2/sns/SnsIO.java#L262 3: https://github.com/apache/beam/blob/da9e17288e8473925674a4691d9e86252e67d7d7/sdks/java/io/solr/src/main/java/org/apache/beam/sdk/io/solr/SolrIO.java#L225 On Wed, Apr 15, 2020 at 11:59 AM Akshay Iyangar <[email protected]<mailto:[email protected]>> wrote: Hi I actually wanted a way to configure FluentBackoff at the client side for S3 in that effort I created below PR. But as luke mentioned in the PR FluentBackoff is part of util and I can directly expose it to public. So a suggested alternative was to use a Configuration class that is public facing which then convert’s it to the internal beam class and have it generic enough to be used across IO’s. Just wanted to know what the community feels and if the above suggestion by luke is ok with other’s I’ll try to implement that instead. JIRA - 9742 https://github.com/apache/beam/pull/11396 Thanks Akshay I
