You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2026/09/05 09:08:22 UTC
[PR] [common] Resolve the atomic rename under the Kerberos FileSystem wrapper [paimon]
LuciferYang opened a new pull request, #9652:
URL: https://github.com/apache/paimon/pull/9652
### Purpose
close #9651
`HadoopFileIO.tryAtomicOverwriteViaRename` looks up `FileSystem`'s three-argument atomic rename reflectively:
```java
method = ReflectionUtils.getMethod(fs.getClass(), "rename", 3);
```
`ReflectionUtils.getMethod` goes through `clz.getMethods()`, so it only sees public methods. With Kerberos configured, `fs` is a `HadoopSecuredFileSystem`, which overrides the two-argument `rename` and leaves the three-argument one protected on the base class. The lookup fails, the cached method stays null, and the method returns false for good, so every caller silently falls back to an in-place `newOutputStream(path, true)`: the snapshot hint files, consumer resets, `TagManager.createOrReplaceTag`, `_SUCCESS` and the service files all stop being written atomically on a secured cluster, with nothing logged.
The lookup now resolves against the file system underneath the wrapper, since the wrapper genuinely cannot override that method: it would have to call a protected member on a different `FileSystem` instance, which does not compile.
Reaching past the wrapper means taking its one responsibility with you. Every delegating method in `HadoopSecuredFileSystem` runs inside `ugi.doAs`, and invoking the rename directly on the unwrapped file system would run it as whatever the calling thread happens to be, while the temporary file being renamed was created as the login user. The invocation therefore goes through a new `callAsLoginUser` on the wrapper, which is the same `runSecuredWithIOException` the other methods use.
### Tests
`HadoopSecuredFileSystemTest.testAtomicRenameRunsOnTheDelegateAsTheLoginUser` wraps a `RawLocalFileSystem` subclass that exposes the three-argument rename as public and counts calls, secures it through `trySecureFileSystem`, installs it with `HadoopFileIO.setFileSystem`, and then asserts `tryAtomicOverwriteViaRename` returns true, the delegate's atomic rename ran exactly once, it ran as the login user, and the content landed.
Against the unfixed code that test fails on the first assertion: the method returns false, which is the silent fallback.
`testUnwrapAndCallAsLoginUser` covers the two new methods directly, including that `callAsLoginUser` propagates an `IOException` rather than wrapping it.
`mvn -pl paimon-common -Dtest=HadoopSecuredFileSystemTest test` on JDK 8: 6 tests, 0 failures. `spotless:check` and `checkstyle:check` on paimon-common are clean.
What this does not cover is a real Kerberos cluster: the delegate here is a local file system with a public three-argument rename, so the test pins the lookup and the identity, not HDFS's rename semantics.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [common] Resolve the atomic rename under the Kerberos FileSystem wrapper [paimon]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #9652:
URL: https://github.com/apache/paimon/pull/9652#issuecomment-5631431183
Thank you @JingsongLi
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [common] Resolve the atomic rename under the Kerberos FileSystem wrapper [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9652:
URL: https://github.com/apache/paimon/pull/9652#discussion_r3975882036
##########
paimon-common/src/main/java/org/apache/paimon/fs/hadoop/HadoopFileIO.java:
##########
@@ -435,8 +444,20 @@ public boolean tryAtomicOverwriteViaRename(Path dst, String content) throws IOEx
writer.flush();
}
- renameMethod.invoke(
- fs, hadoopTemp, hadoopDst, new Options.Rename[] {Options.Rename.OVERWRITE});
+ Options.Rename[] renameOptions = new Options.Rename[] {Options.Rename.OVERWRITE};
+ if (fs instanceof HadoopSecuredFileSystem) {
+ // the call has to stay inside the wrapper's doAs, or the rename runs as
+ // whoever the current thread is rather than the login user
+ ((HadoopSecuredFileSystem) fs)
+ .callAsLoginUser(
+ () -> {
+ renameMethod.invoke(
+ renameTarget, hadoopTemp, hadoopDst, renameOptions);
Review Comment:
[P2] Convert reflective failures to IOException inside the doAs action. When the delegate's rename throws IOException, Method.invoke wraps it in InvocationTargetException. UGI.doAs converts that to UndeclaredThrowableException, and runSecuredWithIOException throws RuntimeException, bypassing the catch below and HintFileUtils.commitHint's IOException retry loop. I reproduced this with the repository's Hadoop 2.8.5 dependency: the unsecured path throws IOException, while the secured head throws RuntimeException with that exception chain. Catch and translate reflective exceptions inside this lambda, and add a regression test through tryAtomicOverwriteViaRename with an injected delegate rename failure; the direct callAsLoginUser IOException test does not exercise Method.invoke.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [common] Resolve the atomic rename under the Kerberos FileSystem wrapper [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #9652:
URL: https://github.com/apache/paimon/pull/9652
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org