jfz commented on pull request #4454:
URL: https://github.com/apache/iceberg/pull/4454#issuecomment-1084987170
>
> I have a few questions about this change: Should a FileIO object be
shareable between multiple threads? I don't see any thread safety guarantees
documented. If it cannot be shared, then ideally the fix should be in the usage
of S3FileIO objects where we make sure a new instance is used within each
thread.
>
> If the S3FileIO is meant to be shareable between threads, then there are
more thread safety concerns. For example, the below needs to be made thread
safe too.
>
> ```
> @Override
> public void initialize(Map<String, String> properties) {
> this.awsProperties = new AwsProperties(properties);
>
> // Do not override s3 client if it was provided
> if (s3 == null) {
> this.s3 = AwsClientFactories.from(properties)::s3;
> }
> ```
I think the intend of shareable is to avoid creating a new S3Client every
time and reduce over the wire transportation of the object. I do see some of
the other stateful methods(executorService/close) are guarded already.
You're right about initialize not guarded but I'd assume in most real cases
that will be called once early at the lifecycle of this object.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
