Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acl fix performance #197

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alfonso-escribano
Copy link

SUMMARY

This PR fix the acl module performance problem.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

acl

ADDITIONAL INFORMATION

When you have more than 7 million files and you have to apply recursive acl, the ansible acl module uses tens of GB of memory.

This is due to the acl module, first try to "apply" acls with the "--test" parameter and analyze the output for any files that change, then apply the setfacl command, and in the last step, run another getfacl command to get the resulting acls, but when you use the recursive option, it makes no sense to get the resulting acls.

With this PR I try to improve performance by making those changes:

First, when you run setfacl with --test, I don't want to get the standard output of the command, the module does not use it, I just want to know if there are any files that change, once I found a file that changes, I terminate the command and do not store the stdout output.
In the last step if I am using the recursive option, I don't want to know the resulting acl, the output only makes sense if there is only one file, not for the recursive one.

With those changes it reduces the memory usage to nothing, before the changes, sometimes, applying it to a file system with more than 7 million files, it used more than 20GB of memory.
It also reduces the execution time, when there is no file that changes, the first step takes the same time, but if there are files that change, the first step is shortened and the last step is not executed.

Command with --check before this PR it also uses gigabytes of ram and the output only shows acls, don't show which file has what acls, so it has no possible use.

(venv) alfons@alfonso-escribano ~/dev-ansible $ time ansible test-host -m acl -a 'path="/path/with/files" entity="test" etype="user" permissions="rX"  recursive=true  state="present"' -C 

test-host | CHANGED => {
    "acl": [
        "user::rwx", 
(....)
(....)
(....)

        "default:other::---", 
], 
    "changed": true, 
    "msg": "user:test:rX is present recursively"
}

real    5m0,547s
user    2m37,924s
sys     0m13,495s

Command with --check after this PR:

(venv) alfons@alfonso-escribano ~/dev-ansible $ time ansible test-host -m acl -a 'path="/path/with/files" entity="test" etype="user" permissions="rX"  recursive=true  state="present"' -C 
 [WARNING]: Not showing resulting acls in the recursive mode

test-host | CHANGED => {
    "acl": [], 
    "changed": true, 
    "msg": "user:test:rX is present recursively"
}

real    0m3,165s
user    0m1,514s
sys     0m0,192s

@maxamillion
Copy link
Collaborator

closing and re-opening to kick CI

@maxamillion maxamillion closed this Jan 8, 2024
@maxamillion maxamillion reopened this Jan 8, 2024
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants