LuciferYang opened a new issue, #9650:
URL: https://github.com/apache/paimon/issues/9650

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/paimon/issues) 
and found nothing similar.
   
   ### Paimon version
   
   master, `475be566f` (2.1-SNAPSHOT).
   
   ### Compute Engine
   
   Any engine writing a format table whose `FileIO` is a `ResolvingFileIO`, 
which is what `RESTCatalog.fileIOFromOptions` builds when `data-token.enabled` 
is off, and what `resolving-file-io.enabled` selects elsewhere.
   
   ### Minimal reproduce step
   
   Write a format table on OSS or S3 through a `ResolvingFileIO`. 
`FormatTableSingleFileWriter` asks for a two-phase output stream:
   
   ```java
   TwoPhaseOutputStream out = fileIO.newTwoPhaseOutputStream(path, false);
   ```
   
   `ResolvingFileIO` overrides `newOutputStream` and `tryToWriteAtomic` to 
forward to the resolved FileIO, but not `newTwoPhaseOutputStream`, so this one 
falls through to the interface default:
   
   ```java
   default TwoPhaseOutputStream newTwoPhaseOutputStream(Path path, boolean 
overwrite)
           throws IOException {
       return new RenamingTwoPhaseOutputStream(this, path, overwrite);
   }
   ```
   
   That writes `<dir>/_temporary/.tmp.<uuid>` and commits by renaming. 
`OSSFileIO`, `S3FileIO` and `JindoFileIO` all override the method with a native 
multipart-upload commit, and `RESTTokenFileIO` forwards to the resolved FileIO 
the way the other `ResolvingFileIO` methods do, so this is the one wrapper that 
loses it.
   
   On an object store, rename is a server-side copy plus delete: the whole file 
is copied a second time at commit, the commit is not atomic, and a `_temporary` 
directory is left behind.
   
   ### What doesn't meet your expectations?
   
   `ResolvingFileIO` exists to delegate to the FileIO that owns the scheme, and 
the sibling wrapper `RESTTokenFileIO` already forwards this method. The missing 
override quietly downgrades every format-table write on object storage.
   
   ### Anything else?
   
   Adding the forwarding on its own is not enough, which is why I am filing 
this rather than sending that patch. The native committers cast the FileIO they 
are handed at commit time:
   
   ```java
   // OSSMultiPartUploadCommitter.multiPartUploadStore
   OSSFileIO ossFileIO = (OSSFileIO) fileIO;
   ```
   
   `FormatTableCommit` commits with the FileIO it was constructed with, 
`message.getCommitter().commit(fileIO)`, which is the table's 
`ResolvingFileIO`. So forwarding the stream creation hands out an OSS-native 
committer that then receives a `ResolvingFileIO` and fails with a 
`ClassCastException` at commit, trading a slow commit for a broken one. 
`S3MultiPartUploadCommitter` and the Jindo one have the same cast.
   
   Making this work needs a decision about who resolves at commit time: the 
committers unwrapping a `ResolvingFileIO` themselves, `FormatTableCommit` 
passing an already-resolved FileIO, or `BaseMultiPartUploadCommitter` resolving 
before it casts. I did not want to pick one of those on your behalf inside a 
bug fix. Happy to implement whichever you prefer.
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!
   


-- 
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]

Reply via email to