-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Reduce file opening #11764
Reduce file opening #11764
Conversation
14640f3
to
567a057
Compare
…id multiple file openings on repeated calls
567a057
to
a10e239
Compare
static std::mutex goMutex; | ||
std::lock_guard oGuard(goMutex); | ||
|
||
// Cache last request and result to avoid repeated file accesses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the credentials cache fromosCredentialsFilename
cache populated once at process start and never again? Users could (should) update the value periodically, so a long-running GDAL processes might get access denied from AWS but it's possible new credentials were updated in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the credentials cache from osCredentialsFilename cache populated once at process start and never again?
yes
but it's possible new credentials were updated in the file.
is it really realistic that ~/.aws/config or ~/.aws/credentials content change during the lifetime of a process ? Aren't those used for 'static' credentials ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it really realistic that ~/.aws/config or ~/.aws/credentials content change during the lifetime of a process ? Aren't those used for 'static' credentials ?
The typical GDAL user would probably not rotate credentials in the lifetime of a process (such as in a single gdalwarp call). However, some companies/AWS IAM policies require users to rotate static credentials every few hours. My particular use case (long-running Mapserver) is probably an edge case where it's easy enough for me to restart a Mapserver FastCGI process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so this basically means that the whole idea is busted. I'll revert that commit and just keep the CPLGetPhysicalRAM() change
closing that one, and just keeping the CPLGetPhysicalRAM() change in #11764 |
Helps reducing the number of openat() syscalls in MapServer intensive scenarios