Add additional configuration properties for FileSystemExchange

[arhimondr]

Description

Is this change a fix, improvement, new feature, refactoring, or other?

Improvement

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

File system exchange

How would you describe this change to a non-technical end user or system administrator?

Additional configuration properties needed for experimentation. In the future the default values might be readjusted. These properties are rather not expected to be changed by the end user.

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

[losipiuk]

No documentation is needed

I guess we should have some (I think we are documenting other properties, right?)

[losipiuk]

please do not use enum from external library directly as config parameter setter, to have full control over what we can set in configuration file.

[arhimondr]

I was also thinking about that. Here's my thoughts. I don't think they are going to remove an existing option, as it may break the compatibility. And when they add a new option we probably want it to also be available.

[losipiuk]

Yeah. But still (I think) the common case is to either have our own enum which maps to enum from an external library. Or make setSomething consume a String and do switch...case in the setter body to map to enum fields.
Adding a new option to our mapping if AWS adds a new enum value is a minimalistic effort. And we have better control over product surface, and also config values naming.

[linzebing]

As a reference trino-hive uses our own enum

trino/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3StorageClass.java

Line 20 in 4ae3121

public enum TrinoS3StorageClass

[arhimondr]

There are other places in the codebase when the library enum is used directly:

trino/plugin/trino-cassandra/src/main/java/io/trino/plugin/cassandra/CassandraClientConfig.java

Line 50 in 193db65

private ConsistencyLevel consistencyLevel = ConsistencyLevel.ONE;

trino/plugin/trino-kafka/src/main/java/io/trino/plugin/kafka/KafkaSecurityConfig.java

Line 30 in c647c31

private SecurityProtocol securityProtocol = PLAINTEXT;

The TrinoS3StorageClass might not be the best example as it does two things:

1. Translates available values from lower case to upper case
2. Limits the available options

The new version of the AWS libraries already uses values in upper case. I'm not sure if there's a value of limiting the available options. While it is rather unlikely that we will ever try to use GLACIER, we may still want to play around with reduced concurrency and reduced availability options. So I don't want to limit the available options just yet. Also using the library enum directly will make any new options available without any modifications and it's rather unlikely for the existing options to be removed as I'm pretty sure AWS developers won't be willing to break compatibility.

Thus I'm not sure if there's a significant practical reason of adding a proxy.

[losipiuk]

feels arbitrary

[arhimondr]

This is the default from the library. It feels like there shouldn't be a real reason why to set it to something lower.

[losipiuk]

Idk if there is the reason or not :) Just feels arbitrary. I would put @Min(1) here. But I do not feel strongly.

[arhimondr]

Actually yeah, while it doesn't make sense to set it to something lower than 50 it is still a legal value. Let me change it.

[losipiuk]

maybe just exchage.s3.async-client-concurrency?