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.

Sign up for GitHub

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

Jump to bottom

If we are preloading files, also preload 'stat' result #31

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open

If we are preloading files, also preload 'stat' result #31

wants to merge 1 commit into from

Conversation

jjhursey
Copy link
Contributor

  • Enabled by default, can be disabled by setting the envar:
    SPINDLE_DISABLE_PRELOAD_STAT=1

@jjhursey
If we are preloading files, also preload 'stat' result
8f74873 
 * Enabled by default, can be disabled by setting the envar:
   `SPINDLE_DISABLE_PRELOAD_STAT=1`

Signed-off-by: Joshua Hursey <[email protected]>
@jjhursey
Copy link
Contributor Author

When preloading files, we noticed that there is a stat call before the file is opened (to check permissions). This requires 2 calls to the root server to (1) cache the stat result, then later (2) to cache the file. By preloading the stat result with the file we combine those calls thus reducing the time to preload files.

mplegendre
mplegendre reviewed May 12, 2020
View reviewed changes
Copy link
Member

@mplegendre mplegendre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise looks good. I'll run some local tests before merging.

src/server/auditserver/ldcs_audit_server_handlers.c
is_stat_enabled = 0;
}
}

debug_printf2("At top of handle_preload_filelist\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation behind making this disable-able via environment variable? People won't get this if they don't preload, so it can already be avoided if someone's grossly offended by stats.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was mostly to have a way to disable it if it became a problem for those enabling preloading. That being said I don't think we ever had to use it to disable the feature.

It can be used to easily see the benefit of the optimization. But that's more of a developer consideration than a user consideration.

I'm ok if you would prefer to remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mplegendre mplegendre mplegendre left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jjhursey @mplegendre