[common] Resolve the atomic rename under the Kerberos FileSystem wrapper - #9652
Open
LuciferYang wants to merge 1 commit into
Open
[common] Resolve the atomic rename under the Kerberos FileSystem wrapper#9652LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.