aio-interface: cache config by mtime in ConfigurationManager to avoid redundant disk reads#7956
Draft
Copilot wants to merge 1 commit into
Draft
aio-interface: cache config by mtime in ConfigurationManager to avoid redundant disk reads#7956Copilot wants to merge 1 commit into
Copilot wants to merge 1 commit into
Conversation
Copilot created this pull request from a session on behalf of
szaimen
April 19, 2026 15:14
View session
szaimen
added
3. to review
szaimen
changed the title
szaimen
force-pushed
the
copilot/cache-config-in-configurationmanager
branch
from
af77490 to
f47da6c
Compare
szaimen
force-pushed
the
copilot/cache-config-in-configurationmanager
branch
from
d59c8a2 to
8b8f0f1
Compare
… redundant disk reads Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/259ee301-d37b-409f-9a09-0cc7d87437fe fix: change configMtime default and reset value from null to 0 Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/8a9ca3ac-8713-4235-aaac-a4496863469b fix: reset config and mtime when file absent or filemtime fails Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/259ee301-d37b-409f-9a09-0cc7d87437fe fix: use null sentinel for configMtime and reset on filemtime failure Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/259ee301-d37b-409f-9a09-0cc7d87437fe fix: restore missing return statement in getConfig() to fix psalm Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/28ac9917-d57b-465f-8e98-dbc75c416ace Co-Authored-By: szaimen <42591237+szaimen@users.noreply.github.com>
szaimen
force-pushed
the
copilot/cache-config-in-configurationmanager
branch
from
8b8f0f1 to
2be6039
Compare
Collaborator
pabzm
requested changes
Apr 24, 2026
Comment on lines
+309
to
+313
| $this->config = json_decode($configContent, true, 512, JSON_THROW_ON_ERROR); | ||
| $configContent = (string)file_get_contents(DataConst::GetConfigFile()); | ||
| if ($configContent === '') { | ||
| throw new \RuntimeException("The config file " . DataConst::GetConfigFile() . " is empty. It may have been truncated due to low disk space. Please restore it from a backup."); | ||
| } |
Member
There was a problem hiding this comment.
Line 310 is useless, maybe an artefact from rebasing?
And parsing the content should happen after it was checked to be non-empty, I'd say.
And while we're at it, let's catch possible JSON parsing errors to produce helpful error messages in the log, ok?
Suggested change
| $this->config = json_decode($configContent, true, 512, JSON_THROW_ON_ERROR); | |
| $configContent = (string)file_get_contents(DataConst::GetConfigFile()); | |
| if ($configContent === '') { | |
| throw new \RuntimeException("The config file " . DataConst::GetConfigFile() . " is empty. It may have been truncated due to low disk space. Please restore it from a backup."); | |
| } | |
| if ($configContent === '') { | |
| throw new \RuntimeException("The config file " . DataConst::GetConfigFile() . " is empty. It may have been truncated due to low disk space. Please restore it from a backup."); | |
| } | |
| try { | |
| $this->config = json_decode($configContent, true, 512, JSON_THROW_ON_ERROR); | |
| } catch (JsonException $exc) { | |
| throw new \RuntimeException("The config file " . DataConst::GetConfigFile() . " could not be parsed as JSON. Please check its contents and maybe restore it from a backup. (Original error: {$exc->message})"); | |
| } | |
szaimen
reviewed
Apr 24, 2026
| throw new \RuntimeException("The config file " . DataConst::GetConfigFile() . " is empty. It may have been truncated due to low disk space. Please restore it from a backup."); | ||
| $configFile = DataConst::GetConfigFile(); | ||
| if (file_exists($configFile)) { | ||
| $mtime = filemtime($configFile); |
Collaborator
There was a problem hiding this comment.
Suggested change
| $mtime = filemtime($configFile); | |
| clearstatcache(true, DataConst::GetConfigFile()); | |
| $mtime = filemtime($configFile); |
Collaborator
|
@pabzm and me discussed we will work on finishing this later |
szaimen
added
2. developing
Collaborator
|
in the case of starttransaction it will make the io worse as it still will read the mtime every time... |
23 tasks
This was referenced May 4, 2026
19 tasks
30 tasks
31 tasks
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
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.
No description provided.