Writing to S3 sometimes results in corrupted files

[sshkvar]

Hi we have migrated to Trino 367 and faced issue with create table as select in iceberg (Potentially all connectors which uses TrinoS3FileSystem affected)

We are doing CTAS in iceberg from other table (~482013195 records).
After when we try to query created table

select count(uuid) from iceberg.schema.just_created_table

we got exception

Query 20220119_153450_24112_cjit2 failed: Error reading from s3a://.../94a0afda-26d1-497d-9d48-362c5b36e363.parquet at position 27214944

After some investigation we found that because of some reasons we have multiple versions of parquet files, this is strange because each parquet file has uuid in name and we just created this table.
you can find screenshot in this thread in slack

Additional retries of table creation shoved us that first version of parquet file always has the same size: 16 MB which is equal to s3MultipartMinFileSize.

After it we checked all changes in io.trino.plugin.hive.s3 package and found this one 7383734.
We have reverted this change and it helped. We didn't have corrupted parquets in iceberg tables anymore.

Important note: issue is accidental, sometimes table created without issue.

[hashhar]

cc: @findepi @losipiuk @linzebing

[findepi]

Additional retries of table creation shoved us that first version of parquet file always has the same size: 16 MB which is equal to s3MultipartMinFileSize.

this is very suspicious

After it we checked all changes in io.trino.plugin.hive.s3 package and found this one 7383734.

Sounds like #9715 is related then
cc @joshthoward @electrum

We have reverted this change and it helped. We didn't have corrupted parquets in iceberg tables anymore.

good to hear that.

that also means you were testing with two builds, ie local modifications
did you also try with hive.s3.streaming.enabled off?

[losipiuk]

If revert helps I vote for reverting as initial fix and then rework original improvement

[findepi]

No longer considered a release blocker, as #10716 merged.
thank you @linzebing @losipiuk

[findepi]

@sshkvar can you please confirm the current master no longer exhibits the problem?
@linzebing any chance this is testable? or there is nothing tbd in this issue?

[sbernauer]

We had the same problem with corrupt parquet and ORC files. In our case the CTAS in iceberg failed with org.apache.iceberg.parquet.ParquetIO$ParquetInputFile@2700c43b is not a Parquet file. Expected magic number at tail, but found [49, -21, -31, 15]
The current master with reverted 7383734 fixed the issue for us 👍
Thanks @sshkvar for bringing this up!

[sshkvar]

@sshkvar can you please confirm the current master no longer exhibits the problem?

@findepi current master works without issue for us

[losipiuk]

@sshkvar I reworked the commit which caused the problem and it should be fine now. The new PR is #10729. Would that be ok for you to test it out?

[sshkvar]

@sshkvar I reworked the commit which caused the problem and it should be fine now. The new PR is #10729. Would that be ok for you to test it out?

@losipiuk looks like reworked fix also working for us, I will continue testing and let you know in case of any issue

[losipiuk]

@sshkvar I reworked the commit which caused the problem and it should be fine now. The new PR is #10729. Would that be ok for you to test it out?

@losipiuk looks like reworked fix also working for us, I will continue testing and let you know in case of any issue

Thanks @sshkvar. Very much appreciate your help here.