Moodle
  1. Moodle
  2. MDL-39430

We should discourage/drop support for eaccelerator

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.6
    • Component/s: Installation
    • Labels:
    • Rank:
      50079

      Description

      Discovered from MDL-36060, eaccelerator doesn't support closures - a critical php 5.3 feature. Its also the way the php api is moving (if you could ever suggest a direction for php..)

      https://github.com/eaccelerator/eaccelerator/issues/12

      That feature is nearly 4 years old and that bug has been dead like that for nearly 10 months, which highly suggests eaccerator development is dead.

      Unfortunately, eaccerator used to be much better than APC in the opcode cache world and so I imagine there are a lot of moodle sites using it.

      I think that we should start with a warning and then also potentially drop support of eaccelerator (if we can detect it, not certain of that). If they aren't supporting anything as critical as closures, who knows what else could break.

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          Adding some VIPs here.

          Show
          Dan Poltawski added a comment - Adding some VIPs here.
          Hide
          Dan Poltawski added a comment -

          (tentatively added it for 2.5, because if we can warn about it, the sooner the better IMO)

          Show
          Dan Poltawski added a comment - (tentatively added it for 2.5, because if we can warn about it, the sooner the better IMO)
          Hide
          Sam Hemelryk added a comment -

          Gets a +1 from me to add a warning if possible when eAccelerator is being used.

          Just to provide a bit of background info to save others time looking at this perhaps.
          Closures weren't introduced until PHP 5.3, they're reasonably new and we've only a limited number within Moodle.
          MDL-36060 is cleaning up over half of the closures we have.
          I did a quick bit of testing myself just now to see whether this affected lambda style anonymous functions and the good news is that it doesn't.
          Strictly closure style anonymous functions are affected.
          Good news as we have dozens of lambda style anonymous functions being used within Moodle.

          I feel given we've not really got a precedence for the use of closures within Moodle it'd be worth establishing one and tidying up our code.
          My personal preference is that we should not permit the use of closures and instead request a declared function or in trivial cases (think sorting) accept lamdba style anonymous functions.
          Although this bug directly pertains to their use I feel they are often easy to look over, and can be a challenge to debug.

          They are however used by significant frameworks, and while we've no sign of them that I could find within the frameworks our standard installation runs I did find uses within both symfony and behat.
          Certainly the warning is justified.

          Show
          Sam Hemelryk added a comment - Gets a +1 from me to add a warning if possible when eAccelerator is being used. Just to provide a bit of background info to save others time looking at this perhaps. Closures weren't introduced until PHP 5.3, they're reasonably new and we've only a limited number within Moodle. MDL-36060 is cleaning up over half of the closures we have. I did a quick bit of testing myself just now to see whether this affected lambda style anonymous functions and the good news is that it doesn't. Strictly closure style anonymous functions are affected. Good news as we have dozens of lambda style anonymous functions being used within Moodle. I feel given we've not really got a precedence for the use of closures within Moodle it'd be worth establishing one and tidying up our code. My personal preference is that we should not permit the use of closures and instead request a declared function or in trivial cases (think sorting) accept lamdba style anonymous functions. Although this bug directly pertains to their use I feel they are often easy to look over, and can be a challenge to debug. They are however used by significant frameworks, and while we've no sign of them that I could find within the frameworks our standard installation runs I did find uses within both symfony and behat. Certainly the warning is justified.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Just to note that there are some more closures along core codebase... that surely would need to be fixed, like the course renderer and the lti above... pasting the quick list I've found (not verified):

          grep -r ' function(' * | grep '.php:' | grep -v vendor | grep -v Y
          lib/behat/behat_base.php:            function($context, $args) {
          mod/lti/locallib.php:    $bestmatch = array_reduce($tools, function($value, $tool) {
          mod/workshop/allocation/random/lib.php:        $getvars = function($obj) { return get_object_vars($obj); };
          

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Just to note that there are some more closures along core codebase... that surely would need to be fixed, like the course renderer and the lti above... pasting the quick list I've found (not verified): grep -r ' function(' * | grep '.php:' | grep -v vendor | grep -v Y lib/behat/behat_base.php: function($context, $args) { mod/lti/locallib.php: $bestmatch = array_reduce($tools, function($value, $tool) { mod/workshop/allocation/random/lib.php: $getvars = function($obj) { return get_object_vars($obj); }; Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          personally I'd say

          "-1 to use closures unless strictly necessary, betting they aren't strictly necessary ever!"

          Show
          Eloy Lafuente (stronk7) added a comment - personally I'd say "-1 to use closures unless strictly necessary, betting they aren't strictly necessary ever!"
          Hide
          Dan Poltawski added a comment -

          I thought that some php core array functions were moving to closure-only alternatives (which I do actually think are much more readable in a lot of cases) - but seemingly I am dreaming. Too much objective-c blocks in the weekends I think

          Show
          Dan Poltawski added a comment - I thought that some php core array functions were moving to closure-only alternatives (which I do actually think are much more readable in a lot of cases) - but seemingly I am dreaming. Too much objective-c blocks in the weekends I think
          Hide
          Dan Poltawski added a comment -

          In any case, I still don't think this is a good sign for eaccelerator - i've posted on the hardware and performance forum about it here: https://moodle.org/mod/forum/discuss.php?d=227631

          Show
          Dan Poltawski added a comment - In any case, I still don't think this is a good sign for eaccelerator - i've posted on the hardware and performance forum about it here: https://moodle.org/mod/forum/discuss.php?d=227631
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Seems that there are some more:

          grep -re '[, (=]function *(.*)[^()]*{' * | grep '\.php:' | \ 
              grep -vP 'vendor|var |Y|fixtures|codechecker|setTimeout|click|onchange|onload|jscode'
          
          admin/tool/behat/renderer.php:                function ($matches) {
          admin/tool/behat/renderer.php:                function ($matches) {
          lib/behat/behat_base.php:            function($context, $args) {
          lib/coursecatlib.php:        uasort($records, function ($a, $b) use ($sortfields) {
          lib/coursecatlib.php:            $searchterms = array_filter($searchterms, function ($v) { return strlen($v) > 1; } );
          lib/coursecatlib.php:                $files = array_filter($files, function ($file) use ($acceptedtypes) {
          lib/tests/behat/behat_transformations.php:            function ($matches) {
          mod/lti/locallib.php:    $bestmatch = array_reduce($tools, function($value, $tool) {
          mod/lti/locallib.php:    $values = array_map(function($item) {
          mod/workshop/allocation/random/lib.php:        $getvars = function($obj) { return get_object_vars($obj); };
          question/type/calculated/db/upgradelib.php:                function ($matches) use ($vs, $format, $length) {
          question/type/calculated/question.php:                function ($matches) use ($vs, $format, $length) {
          question/type/calculatedmulti/db/upgradelib.php:                function ($matches) use ($vs, $format, $length) {
          

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Seems that there are some more: grep -re '[, (=]function *(.*)[^()]*{' * | grep '\.php:' | \ grep -vP 'vendor| var |Y|fixtures|codechecker|setTimeout|click|onchange|onload|jscode' admin/tool/behat/renderer.php: function ($matches) { admin/tool/behat/renderer.php: function ($matches) { lib/behat/behat_base.php: function($context, $args) { lib/coursecatlib.php: uasort($records, function ($a, $b) use ($sortfields) { lib/coursecatlib.php: $searchterms = array_filter($searchterms, function ($v) { return strlen($v) > 1; } ); lib/coursecatlib.php: $files = array_filter($files, function ($file) use ($acceptedtypes) { lib/tests/behat/behat_transformations.php: function ($matches) { mod/lti/locallib.php: $bestmatch = array_reduce($tools, function($value, $tool) { mod/lti/locallib.php: $values = array_map(function($item) { mod/workshop/allocation/random/lib.php: $getvars = function($obj) { return get_object_vars($obj); }; question/type/calculated/db/upgradelib.php: function ($matches) use ($vs, $format, $length) { question/type/calculated/question.php: function ($matches) use ($vs, $format, $length) { question/type/calculatedmulti/db/upgradelib.php: function ($matches) use ($vs, $format, $length) { Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note: the 3 lib/coursecatlib.php ones are being fixed @ MDL-39432

          Show
          Eloy Lafuente (stronk7) added a comment - Note: the 3 lib/coursecatlib.php ones are being fixed @ MDL-39432
          Hide
          Marina Glancy added a comment -

          MDL-39432 fixes course/renderer.php but not lib/coursecatlib.php. I did not know about this problem so I'll fix those

          Show
          Marina Glancy added a comment - MDL-39432 fixes course/renderer.php but not lib/coursecatlib.php. I did not know about this problem so I'll fix those
          Hide
          Martin Dougiamas added a comment -

          +1 to fix things for now assuming that people will use eaccelerator. But we can put a warning for now somewhere during upgrade and on any relevant reports.

          Show
          Martin Dougiamas added a comment - +1 to fix things for now assuming that people will use eaccelerator. But we can put a warning for now somewhere during upgrade and on any relevant reports.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Marina, MDL-39432 fixes coursecats too, there is an extra commit.

          Show
          Eloy Lafuente (stronk7) added a comment - Marina, MDL-39432 fixes coursecats too, there is an extra commit.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Updated list (against integration.git):

          grep -re '[, (=]function *(.*)[^()]*{' * | grep '\.php:' | \
              grep -vP 'vendor|var |Y|fixtures|codechecker|setTimeout|click|onchange|onload|jscode'
          
          admin/tool/behat/renderer.php:                function ($matches) {
          admin/tool/behat/renderer.php:                function ($matches) {
          lib/behat/behat_base.php:            function($context, $args) {
          lib/tests/behat/behat_transformations.php:            function ($matches) {
          mod/lti/locallib.php:    $bestmatch = array_reduce($tools, function($value, $tool) {
          mod/lti/locallib.php:    $values = array_map(function($item) {
          mod/workshop/allocation/random/lib.php:        $getvars = function($obj) { return get_object_vars($obj); };
          question/type/calculated/db/upgradelib.php:                function ($matches) use ($vs, $format, $length) {
          question/type/calculated/question.php:                function ($matches) use ($vs, $format, $length) {
          question/type/calculatedmulti/db/upgradelib.php:                function ($matches) use ($vs, $format, $length) {
          
          Show
          Eloy Lafuente (stronk7) added a comment - Updated list (against integration.git): grep -re '[, (=]function *(.*)[^()]*{' * | grep '\.php:' | \ grep -vP 'vendor| var |Y|fixtures|codechecker|setTimeout|click|onchange|onload|jscode' admin/tool/behat/renderer.php: function ($matches) { admin/tool/behat/renderer.php: function ($matches) { lib/behat/behat_base.php: function($context, $args) { lib/tests/behat/behat_transformations.php: function ($matches) { mod/lti/locallib.php: $bestmatch = array_reduce($tools, function($value, $tool) { mod/lti/locallib.php: $values = array_map(function($item) { mod/workshop/allocation/random/lib.php: $getvars = function($obj) { return get_object_vars($obj); }; question/type/calculated/db/upgradelib.php: function ($matches) use ($vs, $format, $length) { question/type/calculated/question.php: function ($matches) use ($vs, $format, $length) { question/type/calculatedmulti/db/upgradelib.php: function ($matches) use ($vs, $format, $length) {
          Hide
          Michael de Raadt added a comment -

          I haven't been following this issue up to now, so I don't know how hard it will be to resolve the remaining uses of closures. If I've read things correctly we have three options:

          1. fix the remaining closure uses,
          2. add a warning for eaccelerator users (perhaps that's not going to work), or
          3. drop support for eaccelerator.

          While this is related to the change to PHP 5.3 in Moodle 2.5, should this block 2.5 release? Is it easier to drop support or fix the closures issue? And most importantly, who can work on this?

          Show
          Michael de Raadt added a comment - I haven't been following this issue up to now, so I don't know how hard it will be to resolve the remaining uses of closures. If I've read things correctly we have three options: fix the remaining closure uses, add a warning for eaccelerator users (perhaps that's not going to work), or drop support for eaccelerator. While this is related to the change to PHP 5.3 in Moodle 2.5, should this block 2.5 release? Is it easier to drop support or fix the closures issue? And most importantly, who can work on this?
          Hide
          Dan Poltawski added a comment -

          I think we can do all 1 and 2 and still consider 3.

          I don't consider dropping support for it with no warning a good way forward, but if we agree to warn against it, a release (2.5) would be a good opportunity to start.

          Show
          Dan Poltawski added a comment - I think we can do all 1 and 2 and still consider 3. I don't consider dropping support for it with no warning a good way forward, but if we agree to warn against it, a release (2.5) would be a good opportunity to start.
          Hide
          Dan Poltawski added a comment -

          I was pushing the must fix status in the hope someone could take it, but its not happened and i've warned on the forums, so time to focus on the real important stuff.

          Show
          Dan Poltawski added a comment - I was pushing the must fix status in the hope someone could take it, but its not happened and i've warned on the forums, so time to focus on the real important stuff.
          Hide
          Sam Hemelryk added a comment -

          Hi Dan, I'll still look at getting a notice into admin/notifications and cleaning up the last closures I think.
          Should be pretty quick and I told Michael I'd look at it yesterday.

          Show
          Sam Hemelryk added a comment - Hi Dan, I'll still look at getting a notice into admin/notifications and cleaning up the last closures I think. Should be pretty quick and I told Michael I'd look at it yesterday.
          Hide
          Sam Hemelryk added a comment - - edited

          I've a patch up for integration now.

          It does two things:

          1. It adds a warning to the admin notifications page if the eaccelerator extension has been loaded.
          2. It converts two simple closures into lambda style functions (1 in lti, 1 in workshop).

          There are a handful of closures still within Moodle.

          grep -rn '[^a-zA-Z_$]function \?( \?[$}]' . | grep '.php:' | grep -v '/vendor/'
          ./question/type/calculatedmulti/db/upgradelib.php:314:                function ($matches) use ($vs, $format, $length) {
          ./question/type/calculated/db/upgradelib.php:290:                function ($matches) use ($vs, $format, $length) {
          ./question/type/calculated/question.php:393:                function ($matches) use ($vs, $format, $length) {
          ./admin/tool/behat/renderer.php:93:                function ($matches) {
          ./admin/tool/behat/renderer.php:101:                function ($matches) {
          ./lib/tests/behat/behat_transformations.php:113:            function ($matches) {
          ./lib/behat/behat_base.php:127:            function($context, $args) {
          

          The three in question I think are too risky to convert this close to release (mostly because time isn't that free presently)
          The behat closures... do we care? not sure.
          Either way we could easily create two new issues to fix those up explicitly if we want.

          Could someone please peer-review this, close attention to the new string would be good.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - - edited I've a patch up for integration now. It does two things: It adds a warning to the admin notifications page if the eaccelerator extension has been loaded. It converts two simple closures into lambda style functions (1 in lti, 1 in workshop). There are a handful of closures still within Moodle. grep -rn '[^a-zA-Z_$]function \?( \?[$}]' . | grep '.php:' | grep -v '/vendor/' ./question/type/calculatedmulti/db/upgradelib.php:314: function ($matches) use ($vs, $format, $length) { ./question/type/calculated/db/upgradelib.php:290: function ($matches) use ($vs, $format, $length) { ./question/type/calculated/question.php:393: function ($matches) use ($vs, $format, $length) { ./admin/tool/behat/renderer.php:93: function ($matches) { ./admin/tool/behat/renderer.php:101: function ($matches) { ./lib/tests/behat/behat_transformations.php:113: function ($matches) { ./lib/behat/behat_base.php:127: function($context, $args) { The three in question I think are too risky to convert this close to release (mostly because time isn't that free presently) The behat closures... do we care? not sure. Either way we could easily create two new issues to fix those up explicitly if we want. Could someone please peer-review this, close attention to the new string would be good. Many thanks Sam
          Hide
          Petr Škoda added a comment -

          +1

          Show
          Petr Škoda added a comment - +1
          Hide
          Martin Dougiamas added a comment -

          "We have detected that eAccelerator has been enabled for this site. Please be aware that it has some known bugs and may cause some problems with Moodle. We recommend you switch to more supported opcode cache solutions such as APC or Zend Cache."

          Show
          Martin Dougiamas added a comment - "We have detected that eAccelerator has been enabled for this site. Please be aware that it has some known bugs and may cause some problems with Moodle. We recommend you switch to more supported opcode cache solutions such as APC or Zend Cache."
          Hide
          Marina Glancy added a comment -

          Hi Sam, just had a quick look at commit with eaccelerator warning. You convinced me that new parameter is better than putting logic into renderer. But probably it's worth mentioning it in theme/upgrade.txt in case somebody for some reason decided to overwrite this function

          Show
          Marina Glancy added a comment - Hi Sam, just had a quick look at commit with eaccelerator warning. You convinced me that new parameter is better than putting logic into renderer. But probably it's worth mentioning it in theme/upgrade.txt in case somebody for some reason decided to overwrite this function
          Hide
          Damyon Wiese added a comment -

          Thanks Sam,

          All the code looks fine and I think you made a good call as to what is achievable and low risk.

          Suggestions for the string:

          (Yours):
          eAccelerator has been enabled for this site. Please be aware that we don\'t offically support eAccelerator. It is recommended to consider other opcode cache solutions to replace eAccelerator.

          (My suggestion):
          eAccelerator has been enabled for this site. eAccelerator is not officially supported by Moodle and this may cause problems with your site. Please ask your server administrator to install an alternative.

          (Helens Suggestion):
          Something better? (Added as a watcher).

          Thanks Sam - feel free to send for integration - when the string is decided.

          Show
          Damyon Wiese added a comment - Thanks Sam, All the code looks fine and I think you made a good call as to what is achievable and low risk. Suggestions for the string: (Yours): eAccelerator has been enabled for this site. Please be aware that we don\'t offically support eAccelerator. It is recommended to consider other opcode cache solutions to replace eAccelerator. (My suggestion): eAccelerator has been enabled for this site. eAccelerator is not officially supported by Moodle and this may cause problems with your site. Please ask your server administrator to install an alternative. (Helens Suggestion): Something better? (Added as a watcher). Thanks Sam - feel free to send for integration - when the string is decided.
          Hide
          Tim Hunt added a comment -

          My offering:

          The site appears to be using the eAccelerator PHP optimiser. Please be aware that it has limitations which cause problems for some parts of Moodle. We recommend switching to another PHP optimiser such as APC or Zend Cache.

          I would also be inclined to make the key words links to

          Having tried to find the link, I am not sure that 'Zend Cache' is the right name.

          Show
          Tim Hunt added a comment - My offering: The site appears to be using the eAccelerator PHP optimiser. Please be aware that it has limitations which cause problems for some parts of Moodle. We recommend switching to another PHP optimiser such as APC or Zend Cache. I would also be inclined to make the key words links to http://eaccelerator.net http://pecl.php.net/package/apc http://pecl.php.net/package/ZendOpcache Having tried to find the link, I am not sure that 'Zend Cache' is the right name.
          Hide
          Helen Foster added a comment -

          Wow, lots of good string suggestions, so no need for me to come up with one!

          Show
          Helen Foster added a comment - Wow, lots of good string suggestions, so no need for me to come up with one!
          Hide
          Matteo Scaramuccia added a comment -

          Hi All,
          may I ask to add another opcode cacher?

          1. XCache, http://xcache.lighttpd.net/

          Matteo

          Show
          Matteo Scaramuccia added a comment - Hi All, may I ask to add another opcode cacher? XCache, http://xcache.lighttpd.net/ Matteo
          Hide
          Petr Škoda added a comment -

          Sorry for the stealing, I am going to add opcache (standard in PHP 5.5.0) as the only recommended op code cache extension to tge environment test for 2.6 explaining that anything else is not supported.

          Show
          Petr Škoda added a comment - Sorry for the stealing, I am going to add opcache (standard in PHP 5.5.0) as the only recommended op code cache extension to tge environment test for 2.6 explaining that anything else is not supported.
          Hide
          Tim Hunt added a comment -

          Petr, who else have you consulted about this proposed change? Why do you think everyone should use opcache? I guess all the partners have a lot of experience of hosting Moodle, what do they think about this?

          Basically, suddenly an unilaterally putting this up for peer review without any consultation seems very weird to me, especially since this bug seemed to have been heading for a consensus that was something different.

          Show
          Tim Hunt added a comment - Petr, who else have you consulted about this proposed change? Why do you think everyone should use opcache? I guess all the partners have a lot of experience of hosting Moodle, what do they think about this? Basically, suddenly an unilaterally putting this up for peer review without any consultation seems very weird to me, especially since this bug seemed to have been heading for a consensus that was something different.
          Hide
          Petr Škoda added a comment -

          1/ clearly only OPcache has any future because it is now bundled with PHP 5.5.x, all other op caching extensions are going to die soon
          2/ main PHP developers support OPcahe and fix bugs there, did you see PHP 5.5.1 release notes?
          3/ HQ developers can not test and support everything
          4/ APC is already dead, did you see APCu? https://github.com/krakjoe/apcu
          5/ and finally this was already discussed as part of MDL-40415

          Show
          Petr Škoda added a comment - 1/ clearly only OPcache has any future because it is now bundled with PHP 5.5.x, all other op caching extensions are going to die soon 2/ main PHP developers support OPcahe and fix bugs there, did you see PHP 5.5.1 release notes? 3/ HQ developers can not test and support everything 4/ APC is already dead, did you see APCu? https://github.com/krakjoe/apcu 5/ and finally this was already discussed as part of MDL-40415
          Hide
          Ryan Smith added a comment -

          +1 to making OPcache as the only recommended op code cache extension.

          OPCache is officially supported on PHP 5.3.x and 5.4.x as well:

          https://github.com/zendtech/ZendOptimizerPlus/

          As Petr says, APC is dead, eAccelerator is buggy, Wincache is essentially dead (RARELY updated by Microsoft, also see http://blog.syntaxc4.net/ ). Before OPcache was voted to be included from PHP 5.5.x and on, we did test Xcache and it did seem to work well.

          Show
          Ryan Smith added a comment - +1 to making OPcache as the only recommended op code cache extension. OPCache is officially supported on PHP 5.3.x and 5.4.x as well: https://github.com/zendtech/ZendOptimizerPlus/ As Petr says, APC is dead, eAccelerator is buggy, Wincache is essentially dead (RARELY updated by Microsoft, also see http://blog.syntaxc4.net/ ). Before OPcache was voted to be included from PHP 5.5.x and on, we did test Xcache and it did seem to work well.
          Hide
          Martin Dougiamas added a comment -

          Well, let's not scare everyone.

          People on web hosts are going to be stuck with other solutions for some time.

          +1 to start recommending people move to opcache soon but we simply cannot just say we won't support anything else. We need to continue fixing any bugs that come up there (not that I'm expecting many).

          Show
          Martin Dougiamas added a comment - Well, let's not scare everyone. People on web hosts are going to be stuck with other solutions for some time. +1 to start recommending people move to opcache soon but we simply cannot just say we won't support anything else. We need to continue fixing any bugs that come up there (not that I'm expecting many).
          Hide
          Martin Dougiamas added a comment -

          I've notified Moodle Partners about this issue.

          Show
          Martin Dougiamas added a comment - I've notified Moodle Partners about this issue.
          Hide
          Tim Hunt added a comment -

          Adding a link to MDL-40415 then.

          Would it be sensible for someone from HQ to post about this in the hardware and performance forum?

          Show
          Tim Hunt added a comment - Adding a link to MDL-40415 then. Would it be sensible for someone from HQ to post about this in the hardware and performance forum?
          Hide
          Petr Škoda added a comment -

          To integrators: the stable branches should get the same environment.xml file, but I guess we do not need to include them in the Fix version field.

          Show
          Petr Škoda added a comment - To integrators: the stable branches should get the same environment.xml file, but I guess we do not need to include them in the Fix version field.
          Hide
          Dan Poltawski added a comment - - edited

          -1 to the term 'it is the only officially supported op code cache' and -1 for the lack of discussion which made you arrive at sending this to integration. Like Tim says, it should be discussed more widely.

          You can't keep changing the goalposts like that Petr. Up until two weeks ago it wasn't even supported! That sort of strong statement has a significant impact on people who host Moodle, and in my opinion, its not necessary - I can't recall the last time I heard about an APC bug (or ever).

          I also object to the way you are summing up the situation:

          1/ clearly only OPcache has any future because it is now bundled with PHP 5.5.x, all other op caching extensions are going to die soon

          Yet, the link to the RFC you've put on this issue says something quite different:

          The integration proposed for PHP 5.5.0 is mostly 'soft' integration. That means that there'll be no tight coupling between Optimizer+ and PHP; Those who wish to use another opcode cache will be able to do so, by not loading Optimizer+ and loading another opcode cache instead. As per the Suggested Roadmap above, we might want to review this decision in the future; There might be room for further performance or functionality gains from tighter integration; None are known at this point, and they're beyond the scope of this RFC.

          Finally, we don't just support php 5.5.x and actually our 'officialally supported HQ experience' of this is that it was non-trivial to install with 5.3.x on a standard debian based install. It looks like it conflicts with the suoshin patch and would probably end up requiring people to roll their own php packages.

          In fact, we didn't even get that resolved.

          Reopening.

          Show
          Dan Poltawski added a comment - - edited -1 to the term 'it is the only officially supported op code cache' and -1 for the lack of discussion which made you arrive at sending this to integration. Like Tim says, it should be discussed more widely. You can't keep changing the goalposts like that Petr. Up until two weeks ago it wasn't even supported! That sort of strong statement has a significant impact on people who host Moodle, and in my opinion, its not necessary - I can't recall the last time I heard about an APC bug (or ever). I also object to the way you are summing up the situation: 1/ clearly only OPcache has any future because it is now bundled with PHP 5.5.x, all other op caching extensions are going to die soon Yet, the link to the RFC you've put on this issue says something quite different: The integration proposed for PHP 5.5.0 is mostly 'soft' integration. That means that there'll be no tight coupling between Optimizer+ and PHP; Those who wish to use another opcode cache will be able to do so, by not loading Optimizer+ and loading another opcode cache instead. As per the Suggested Roadmap above, we might want to review this decision in the future; There might be room for further performance or functionality gains from tighter integration; None are known at this point, and they're beyond the scope of this RFC. Finally, we don't just support php 5.5.x and actually our 'officialally supported HQ experience' of this is that it was non-trivial to install with 5.3.x on a standard debian based install. It looks like it conflicts with the suoshin patch and would probably end up requiring people to roll their own php packages. In fact, we didn't even get that resolved. Reopening.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Petr Škoda added a comment -

          Now that you mention it Suhosin does not seem to be supported either, I had to disable it a few times to make Moodle work and that was just a default install without any extra restrictions.

          PHP 5.5.x is supported, maybe not from you but I do care if Moodle runs fine with it since it was in beta. You seem to be stuck in the past, Moodle 2.6 is about the future. What PHP versions and op caches will be our admins using next year?

          Anyway please report bugs to Debian.

          Show
          Petr Škoda added a comment - Now that you mention it Suhosin does not seem to be supported either, I had to disable it a few times to make Moodle work and that was just a default install without any extra restrictions. PHP 5.5.x is supported, maybe not from you but I do care if Moodle runs fine with it since it was in beta. You seem to be stuck in the past, Moodle 2.6 is about the future. What PHP versions and op caches will be our admins using next year? Anyway please report bugs to Debian.
          Hide
          Dan Poltawski added a comment -

          I've no problem with encouraging a move in the opcache direction, that is different from saying what people are currently doing is unsupported. Unfortunately, Moodle 2.6 will probably be out of Moodle support before php 5.5 is the dominant php version of our users.

          Show
          Dan Poltawski added a comment - I've no problem with encouraging a move in the opcache direction, that is different from saying what people are currently doing is unsupported. Unfortunately, Moodle 2.6 will probably be out of Moodle support before php 5.5 is the dominant php version of our users.
          Hide
          Petr Škoda added a comment -

          PHP developers decided that opcache is the solution for 5.5.x forward and they also support is via PECL in older branches. OPcache is not new, it was a proven caching solution for a very long time. The problem with Debian is that they ignored it because it was closed, it is fully open sourced now. I suppose commercial distros like Redhat will have fewer problems.

          We should recommend some opcache to our admins - because we need better performance and it also lowers the memory requirements significantly. Anything we recommend should work with any random caching settings defined by admins or shared hosting companies - MDL-40415 makes sure admins do not hit random problems if they enable op caching.

          Historically the admins with most problems are Windows users especially if this is their first web app installation - there are no security backports on Windows, they should always stick with the latest and greatest PHP release - next year it will be 5.5.x. Shared web hosting companies do whatever they want, without op cache Moodle often runs out of memory during install.

          The question is not if we should recommend opcache, but when. I do not see any reason to wait until Moodle 2.7.

          If you do not like my proposed 'OPcache extension significantly improves performance, it is the only officially supported op code cache.' feel free to propose a beter warning, it should imo state following:

          • OPcache is not required, but it helps a lot
          • PHP developers decided to support OPcache exclusively (there is nothing to discuss here)
          • Moodle 2.6 includes explicit support for OPcache (already implemented)
          • Moodle developers will test OPcache compatibility (note that we must do this in any case)
          • Other cache extensions may or may not work (over the years I saw many bug reports in forum and here closed as 'not a bug', 'can not reproduce' or 'reset op cache')
          Show
          Petr Škoda added a comment - PHP developers decided that opcache is the solution for 5.5.x forward and they also support is via PECL in older branches. OPcache is not new, it was a proven caching solution for a very long time. The problem with Debian is that they ignored it because it was closed, it is fully open sourced now. I suppose commercial distros like Redhat will have fewer problems. We should recommend some opcache to our admins - because we need better performance and it also lowers the memory requirements significantly. Anything we recommend should work with any random caching settings defined by admins or shared hosting companies - MDL-40415 makes sure admins do not hit random problems if they enable op caching. Historically the admins with most problems are Windows users especially if this is their first web app installation - there are no security backports on Windows, they should always stick with the latest and greatest PHP release - next year it will be 5.5.x. Shared web hosting companies do whatever they want, without op cache Moodle often runs out of memory during install. The question is not if we should recommend opcache, but when. I do not see any reason to wait until Moodle 2.7. If you do not like my proposed 'OPcache extension significantly improves performance, it is the only officially supported op code cache.' feel free to propose a beter warning, it should imo state following: OPcache is not required, but it helps a lot PHP developers decided to support OPcache exclusively (there is nothing to discuss here) Moodle 2.6 includes explicit support for OPcache (already implemented) Moodle developers will test OPcache compatibility (note that we must do this in any case) Other cache extensions may or may not work (over the years I saw many bug reports in forum and here closed as 'not a bug', 'can not reproduce' or 'reset op cache')
          Hide
          Petr Škoda added a comment -

          I have toned down the warning message: 'PHP opcode caching improves performance and lowers memory requirements, OPcache extension is recommended and fully supported.';

          Hopefully this will be more acceptable for admins that insist on using APC beta releases on production systems (there is no stable apc release for PHP 5.4.x).

          Show
          Petr Škoda added a comment - I have toned down the warning message: 'PHP opcode caching improves performance and lowers memory requirements, OPcache extension is recommended and fully supported.'; Hopefully this will be more acceptable for admins that insist on using APC beta releases on production systems (there is no stable apc release for PHP 5.4.x).
          Hide
          Sam Hemelryk added a comment -

          Ok this has been integrated now, as MDL-40554 has not been backported yet I've integrated this master only and will be noting on that issue to backport this change at the same time to the same branches.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Ok this has been integrated now, as MDL-40554 has not been backported yet I've integrated this master only and will be noting on that issue to backport this change at the same time to the same branches. Many thanks Sam
          Hide
          Sam Hemelryk added a comment -

          Tested and passed thanks Petr.

          Show
          Sam Hemelryk added a comment - Tested and passed thanks Petr.
          Hide
          Sam Hemelryk added a comment -

          Against all probability we've achieved normality. You changes didn't break the tests I pretended to run and are now immortalised upstream. Good for you!

          "It was a programming technique that had been reverse-engineered from the sort of psychotic mental blocks that otherwise perfectly normal people had been observed invariably to develop when elected to high political office."
          Adams, D (1992) Mostly Harmless. London: William Heinemann.

          Show
          Sam Hemelryk added a comment - Against all probability we've achieved normality. You changes didn't break the tests I pretended to run and are now immortalised upstream. Good for you! "It was a programming technique that had been reverse-engineered from the sort of psychotic mental blocks that otherwise perfectly normal people had been observed invariably to develop when elected to high political office." Adams, D (1992) Mostly Harmless. London: William Heinemann.
          Hide
          Helen Foster added a comment -

          Just noting that I have deleted http://docs.moodle.org/26/en/Installing_eAccelerator_In_Ubuntu_Server and removed references to eaccelerator in the 2.6 docs wiki.

          Show
          Helen Foster added a comment - Just noting that I have deleted http://docs.moodle.org/26/en/Installing_eAccelerator_In_Ubuntu_Server and removed references to eaccelerator in the 2.6 docs wiki.

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: