Only read storage.conf once #1404
Conversation
Currently running a simple container runs and stats configuration storage.conf files multiple times on a simple container run. This PR cuts the opens and stats in half by caching the first read. This speeds up start by about 10-20 microseconds. If container engines want to react to storage files changing, added a new function UpdateStoreOptions to allow engines to reload options. Fixes: containers#1403 Signed-off-by: Daniel J Walsh <[email protected]>
|
Here is what I am seeing with this patch on Podman, second podman has the change. |
|
If I trace, the command, I see around half the stats and reads of the storage.conf file. |
|
This along with containers/podman#16238 should also help HPC folks. |
|
Is it expected that CRI-O will trigger the reloads? If so, that work needs to happen. |
Yes. CRI-O is already doing that for |
|
LGTM |
| storageConf, err := DefaultConfigFile(rootless && rootlessUID != 0) | ||
| if err != nil { | ||
| return defaultStoreOptions, err | ||
| } | ||
| return defaultStoreOptionsIsolated(rootless, rootlessUID, storageConf) | ||
| } | ||
|
|
||
| // UpdateOptions should be called iff container engine recieved a SIGHUP, |
There was a problem hiding this comment.
Typo on iff? (or is it just a term that I don't know?)
There was a problem hiding this comment.
"iff" is short for "if and only if"
|
for posterity, I don't see us making any reload claims about the storage fields. Even if we did have them, CRI-O triggers a reload on SIGHUP manually, so dropping this here should be fine (and if we ever add this support, we can use UpdateStoreOptions) |
Currently running a simple container runs and stats configuration storage.conf files multiple times on a simple container run.
This PR cuts the opens and stats in half by caching the first read. This speeds up start by about 10-20 microseconds.
If container engines want to react to storage files changing, added a new function UpdateStoreOptions to allow engines to reload options.
Fixes: #1403
Signed-off-by: Daniel J Walsh [email protected]