I think that load_component_strings()$disablecache and $cacheresult are really quite similar sounding arguments. I understand the distinction but wonder if there is a way to make this clearer as the difference is quite subtle.

Should we retain this behaviour in DEBUG_DEVELOPER? What are the cases when we do rebuild cache?

Dan Poltawski
added a comment - 16/Apr/12 2:09 PM Hi Petr,
I'm pondering this issue in my mind
I think that load_component_strings() $disablecache and $cacheresult are really quite similar sounding arguments. I understand the distinction but wonder if there is a way to make this clearer as the difference is quite subtle.
Should we retain this behaviour in DEBUG_DEVELOPER? What are the cases when we do rebuild cache?

The difference is subtle - see linked MDL that explains the chained cache rebuild trouble, I hope this could actually eliminate it without file locking. The problem here is that there is no separate cache reset and there are hacks that rely on the current $disable cache mechanism - I had to either undo those hacks (such as the help.php) and change API or add new fully BC parameter.

Hmm, now I think I should have probable created new method for cache reset and redefine the meaning of $disable cache... reopening, I need one more week to think about this, thanks a lot!!!

Petr Skoda
added a comment - 16/Apr/12 4:23 PM - edited The difference is subtle - see linked MDL that explains the chained cache rebuild trouble, I hope this could actually eliminate it without file locking. The problem here is that there is no separate cache reset and there are hacks that rely on the current $disable cache mechanism - I had to either undo those hacks (such as the help.php) and change API or add new fully BC parameter.
Hmm, now I think I should have probable created new method for cache reset and redefine the meaning of $disable cache... reopening, I need one more week to think about this, thanks a lot!!!

Petr Skoda
added a comment - 17/Apr/12 6:02 AM Resubmitting, I have used different trick that did not require changes in API.
I have also fixed another related problem that $disablelocal==true was borking cache contents and might incorrectly use normal cached results...

Note: this patch expects that admins do not add strings to 'en' lang packs on production sites and that plugin developers bump up version numbers after introduction of new lang strings. In the worst case scenario perf might be lower on pages that use these new strings until admin resets caches. On the other hand this could eliminate problems described in MDL-31904 and it improves performance on pages that are using missing strings because the cache in not rewritten on each get_string('missingstring') call.

Petr Skoda
added a comment - 17/Apr/12 6:07 AM Note: this patch expects that admins do not add strings to 'en' lang packs on production sites and that plugin developers bump up version numbers after introduction of new lang strings. In the worst case scenario perf might be lower on pages that use these new strings until admin resets caches. On the other hand this could eliminate problems described in MDL-31904 and it improves performance on pages that are using missing strings because the cache in not rewritten on each get_string('missingstring') call.

I was discussing this with Eloy and well, i'll just paste in what he commented upon:

1) Requiring upgrade on lang changes needs to be documented, because that was not necessary until now.
2) string_exists(), used here and there, always use cache, and never recalculates component strings. So it is possible for string_exists() to be returning false, but the corresponding get_string() be returning one proper string. No critical, but for your consideration.
3) Haven't looked to how the reset() happens. If it's simply deleting files, apparently np. But if it's done by calling load_component_strings() perhaps it would need some change. In any case, I think we should have better mechanism. I can see race conditions rebuilding those caches for sure.

Looking at 2) help.php seems to be particularly trying to do inteligence about reloading strings relating to that

Dan Poltawski
added a comment - 18/Apr/12 9:44 AM Hi Petr,
I was discussing this with Eloy and well, i'll just paste in what he commented upon:
1) Requiring upgrade on lang changes needs to be documented, because that was not necessary until now.
2) string_exists(), used here and there, always use cache, and never recalculates component strings. So it is possible for string_exists() to be returning false, but the corresponding get_string() be returning one proper string. No critical, but for your consideration.
3) Haven't looked to how the reset() happens. If it's simply deleting files, apparently np. But if it's done by calling load_component_strings() perhaps it would need some change. In any case, I think we should have better mechanism. I can see race conditions rebuilding those caches for sure.
Looking at 2) help.php seems to be particularly trying to do inteligence about reloading strings relating to that

1) it is necessary to bump up version.php after string change since 2.0 - I was telling that to everybody during reviews
2) yes, string_exists() has to always use cache for performance reasons, it is used too much in stable code; this is the main reason for 1)
3) unfortunately that is again related to 1) - see help.php nasty tricks

I do not know where to document the requirement for version bumps on en string change. Please note it is not just adding of new en strings, the cache is outdated when you modify any en string too, but thanks to the constant resetting ppl might not notice it.

Petr Skoda
added a comment - 18/Apr/12 3:55 PM 1) it is necessary to bump up version.php after string change since 2.0 - I was telling that to everybody during reviews
2) yes, string_exists() has to always use cache for performance reasons, it is used too much in stable code; this is the main reason for 1)
3) unfortunately that is again related to 1) - see help.php nasty tricks
I do not know where to document the requirement for version bumps on en string change. Please note it is not just adding of new en strings, the cache is outdated when you modify any en string too, but thanks to the constant resetting ppl might not notice it.
You have to bump up string if:
change db structure
change cron
change permissions
change en strings
change javascript
I am going to update http://docs.moodle.org/dev/Upgrade_API now

Andrew Davis
added a comment - 19/Apr/12 10:16 AM I just tested this and didnt get any missing strings message on the purge cache screen. Although the strings I removed from the cache were not used on the purge cache screen.

Adam Olley
added a comment - 13/Jun/12 10:59 AM - edited It's extremely unfortunate that this means that a simple string change will require an outage of the Moodle instance, which, ideally, should only be happening for database changes or upgrade code.
It'd be nice if upgrade only caused an outage if it was going to touch those types of things. But that's really a seperate issue than here.