[common] Resolve the atomic rename under the Kerberos FileSystem wrapper - #9652
[common] Resolve the atomic rename under the Kerberos FileSystem wrapper#9652LuciferYang wants to merge 2 commits into
Conversation
tryAtomicOverwriteViaRename looks up FileSystem's 3-arg rename with ReflectionUtils.getMethod, which only sees public methods. HadoopSecuredFileSystem overrides just the 2-arg rename, so with Kerberos configured the lookup failed and the method returned false for good: hint files, consumer resets, tags and _SUCCESS were all written by in-place overwrite instead, silently. Resolve and invoke on the file system under the wrapper, with the invocation inside the wrapper's doAs so the rename still runs as the login user like the temp file it renames.
JingsongLi
left a comment
There was a problem hiding this comment.
The secured atomic-rename path has concrete end-to-end value. The current implementation needs to preserve the IOException contract on the reflective failure path before it can be merged.
| .callAsLoginUser( | ||
| () -> { | ||
| renameMethod.invoke( | ||
| renameTarget, hadoopTemp, hadoopDst, renameOptions); |
There was a problem hiding this 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.
Purpose
close #9651
HadoopFileIO.tryAtomicOverwriteViaRenamelooks upFileSystem's three-argument atomic rename reflectively:ReflectionUtils.getMethodgoes throughclz.getMethods(), so it only sees public methods. With Kerberos configured,fsis aHadoopSecuredFileSystem, which overrides the two-argumentrenameand 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-placenewOutputStream(path, true): the snapshot hint files, consumer resets,TagManager.createOrReplaceTag,_SUCCESSand 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
FileSysteminstance, which does not compile.Reaching past the wrapper means taking its one responsibility with you. Every delegating method in
HadoopSecuredFileSystemruns insideugi.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 newcallAsLoginUseron the wrapper, which is the samerunSecuredWithIOExceptionthe other methods use.Tests
HadoopSecuredFileSystemTest.testAtomicRenameRunsOnTheDelegateAsTheLoginUserwraps aRawLocalFileSystemsubclass that exposes the three-argument rename as public and counts calls, secures it throughtrySecureFileSystem, installs it withHadoopFileIO.setFileSystem, and then assertstryAtomicOverwriteViaRenamereturns 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.
testUnwrapAndCallAsLoginUsercovers the two new methods directly, including thatcallAsLoginUserpropagates anIOExceptionrather than wrapping it.mvn -pl paimon-common -Dtest=HadoopSecuredFileSystemTest teston JDK 8: 6 tests, 0 failures.spotless:checkandcheckstyle:checkon 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.