YARN-11987. container-executor delete-as-user fails to rmdir private filecache directories. - #8700
Open
eubnara wants to merge 1 commit into
Open
YARN-11987. container-executor delete-as-user fails to rmdir private filecache directories.#8700eubnara wants to merge 1 commit into
eubnara wants to merge 1 commit into
Conversation
…filecache directories. delete_path() performed the final top-level rmdir only as the NM user when the relative path is empty. That works when the parent directory is owned by the NM user (usercache/<user>), but always fails with EACCES for localizer-abort cleanup of private filecache entries (usercache/<user>/filecache/<id>), whose parent is owned by the run-as user with mode 0710. Try the rmdir as the run-as user first and fall back to rmdir_as_nm() on EACCES/EPERM.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
@cnauroth @slfan1989 Could you take a look when you have a chance? This fixes a "Permission denied" on the final rmdir in container-executor's delete-as-user path: private filecache directories are created with 0710 owned by the run-as user, but the rmdir runs as the NM user, so empty directories pile up (see YARN-9157 for the inode-exhaustion symptom and YARN-11920 for the permission-model discussion). Thanks! |
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
2 participants
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.
Description of PR
See YARN-11987 for the full analysis. Summary:
When LocalizerRunner cleans up after an aborted localization, it batches the leftover dirs as baseDirs with an empty relative path, so
delete_as_user()setsneeds_tt_user = 1anddelete_path()performs the final top-level rmdir only as the NM user. That works for NM-owned parents (usercache/<user>), but always fails with EACCES for private filecache entries (usercache/<user>/filecache/<id>), whose parent is owned by the run-as user with mode 0710. Every aborted localization logs aDeleteAsUser ... exit code: 255error and leaks an empty directory that is never retried.This PR makes
delete_path()try the final rmdir as the current (run-as) user first, falling back tormdir_as_nm()on EACCES/EPERM whenneeds_tt_useris set:An alternative would be to fix the caller (one FileDeletionTask per path, with the path as subDir), but batching several absolute paths into one container-executor invocation is only possible through baseDirs, so that costs one exec per path and leaves the same trap for any other caller. Fixing
delete_path()covers all callers in one place.How was this patch tested?
Added
test_delete_dir_in_user_owned_parentto test-container-executor.c. The failure mode needs the final rmdir to run as a non-root NM user different from the directory owner, so the test only exercises the bug in the root invocation mode with two distinct users:Before the fix this fails the new test (rmdir_as_nm gets EACCES); after the fix it passes. Notably, without the fix even the existing
test_delete_userfails in this two-user root mode — its empty-subdir scenario deletes a dir under the run-as user'sappcache, which hits the same EACCES:Non-root runs (cetest in CI) skip the privileged part gracefully, same as the existing test_delete_user.
Also verified manually with the exact NM invocation against a directory laid out like a private filecache entry:
Before: exit 255,
rmdir of ... failed - Permission denied, empty dir remains. After: exit 0, dir removed. The preserved NM-owned-parent case (... 3 "" /data/01/yarn/nm/usercache/alice) still succeeds.For code changes:
declared according to the connector-specific documentation? Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required.
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
If an AI tool was used:
where is the name of the AI tool used.
Generative AI: Contains content generated by Claude Code (Anthropic
Claude). The change was human-reviewed and verified on a production
cluster, and complies with the ASF Generative Tooling Guidance
(https://www.apache.org/legal/generative-tooling.html).
https://www.apache.org/legal/generative-tooling.html