? Pending

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
14 Jun 2016

This is a part of other my PR #10565.
I split it because it is more important part and simpler to test.

[UPDATED]

Summary of Changes

Catch exception from storage (ex memcache) class in main cache class.

If we turn off memcache for a few seconds or minutes or set incorrect port then joomla website display JLIB_APPLICATION_ERROR_COMPONENT_NOT_LOADING.

On php7 memcache is not available so when admin move to php7
then website should display correct error message.

Testing Instructions

Before patch:
1) Go to joomla backend and enable working cache like memcache(-d).
2) Turn off cache server or change the port (for memcache) from 11211 to some not working like 11234.
3) You should get error JLIB_APPLICATION_ERROR_COMPONENT_NOT_LOADING.

After patch:
1) Go to joomla back-end and enable working cache like memcache(-d).
2) Turn off cache server or change the port (for memcache) from 11211 to some not working like 11234.
3) You should get correct warning message.

avatar csthomas csthomas - open - 14 Jun 2016
avatar csthomas csthomas - change - 14 Jun 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 14 Jun 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 14 Jun 2016
Category Cache
avatar mbabker
mbabker - comment - 14 Jun 2016

There is a part of me that says this is wrong as a method should NOT be returning an Exception unless it's a factory method used to create an Exception that's to be thrown. Also, the coupling to the application here is wrong; use JLog and do NOT wrap it in a conditional based on what type of application is running.

avatar csthomas
csthomas - comment - 14 Jun 2016

$app replaced by JLog.

I know that self::$_handler[$hash] = $e; is not the best way but all cache methods support it now by condition:
if (!($handler instanceof Exception) && $this->_options['caching'])

For better solution we have to change more and I think that should be another PR.

avatar mbabker
mbabker - comment - 14 Jun 2016

Honestly the entire error handling layer of Joomla needs a major overhaul with v4 to get decoupled from JError and some of the oddities like returning Exceptions versus throwing them.

avatar csthomas csthomas - change - 14 Jun 2016
The description was changed
avatar csthomas
csthomas - comment - 14 Jun 2016

Yes, this is right, but for now this should be enough for 3.6.
If you do not see more problems then can I ask testers for tests?

avatar csthomas csthomas - reference | 21a9438 - 16 Jun 16
avatar schultz-it-solutions
schultz-it-solutions - comment - 13 Jul 2016

Hi,
I just updated a multitude of sites with Joomla 3.6.0-stable and getting this WARNING message
JLIB_APPLICATION_ERROR_COMPONENT_NOT_LOADING (backend and frontend)
only on ONE site.

Can anyone give some advice on how to solve that
Best regards
Ruediger Schultz


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10815.

avatar csthomas
csthomas - comment - 13 Jul 2016

@schultz-it-solutions If you use cache then try to turn it off for a few seconds in joomla
or restart memcached server (if you use memcache).

You can turn it of in configuration.php by replace:

public $caching = '1';

or:

public $caching = '2';

to:

public $caching = '0';


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10815.

avatar coaleb
coaleb - comment - 13 Jul 2016

I tried it out, and it's definitely an improvement. Instead of the vague language string I get a much more useful "Could not connect to memcache server" error. +1

avatar schultz-it-solutions
schultz-it-solutions - comment - 14 Jul 2016

I am afraid this does not fix it for my site... I had caching off ( public $caching = '0'; ) . I even set it to 1 for a short time and then back again to 0 .
No changes, I am still getting the language string and nothing else...

avatar csthomas
csthomas - comment - 14 Jul 2016

Please add a few details:

avatar schultz-it-solutions
schultz-it-solutions - comment - 14 Jul 2016

ok,

  • message shows up on all frontend and backend pages
  • CacheHandler: that was a good one. I changed manually (in configuration.php) to "file" and the messages disappeard. They reappear when I change back to "memcache" (as it was in the first place)
  • no, I am not explicitly using your patch, but the current JOOMLA 3.6.0-stable version
  • this website runs on PHP 5.6.17

So in fact switching the Cache Handler did solve the issue (but I like to return to "memcache" as handler in due time...

avatar schultz-it-solutions
schultz-it-solutions - comment - 14 Jul 2016

CacheHandler: that was a good one. I changed manually (in configuration.php) to "file" and the messages disappeard. They reappear when I change back to "memcache" (as it was in the first place)
Even if "caching" is disabled!

avatar csthomas
csthomas - comment - 14 Jul 2016

It seems that some part of code do not check if cache is enabled and try to connect to cache handler first, which generate exception which is not caught.

This PR catch it, which resolve part of the problem.

Memcache for php 7 does not work and probably won't be working in the future.
Memcached should work with php 7.

avatar brianteeman
brianteeman - comment - 14 Jul 2016

MemcachE development stopped in 2013!!

avatar csthomas
csthomas - comment - 14 Jul 2016

Yes, but php 5 works with MemcachE and a lots of people will use it for some time.

If someone is interest and like memcachE performance may try to use PR #10565.
I use it for a big website with success on joomla 3.5.1.
Currently there are not exist a test instruction and proof of performance gain
because I do not have enough time.
Real world testing and simple comment will be helpful.

Can I ask you @schultz-it-solutions to test MemcacheD handler.
Please change it in configuration file manually, you do not need to have MemcacheD installed.

I only want a proof that this is not directly related only to memcachE.

avatar schultz-it-solutions
schultz-it-solutions - comment - 14 Jul 2016

if i change the cache-handler in configuriation.php from "file" to "memcached", I get:

Error displaying the error page: The memcached Cache Storage is not supported on this platform.: The memcached Cache Storage is not supported on this platform.

avatar csthomas
csthomas - comment - 14 Jul 2016

But with that error can you work on backend and back to file handler in browser?
This is the same type of error as on memcachE but with other text?

avatar coaleb
coaleb - comment - 14 Jul 2016

My experience was that with memcache as the cache handler, even if caching is disabled, I get an error in J! 3.6.0; Without this PR it's the very misleading "JLIB_APPLICATION_ERROR_COMPONENT_NOT_LOADING" error, but WITH the PR it's the much more useful "Could not connect to memcache server" error. In either case, the error shows up as a raiseWarning and everything still works on both front end & the back end.

IMHO I think this PR is an improvement.

Not directly related to this PR: I can't understand why Joomla tries to connect to the cache handler when caching is disabled? Also, when caching is disabled, Joomla hides the option to change the cache handler, potentially making this issue harder to diagnose/fix when experiencing this error.

avatar coaleb
coaleb - comment - 14 Jul 2016

I tried both memcachE and memcacheD with the same result. Interesting thing is that according to phpinfo, memcache is enabled on my server. Also, I think it goes without saying that memcache was my handler for J! 3.5.1 and this issue either did not exist or did not raise a warning in that version.

avatar coaleb
coaleb - comment - 14 Jul 2016

FYI when I try to use the Patch Tester, I get this error: "Could not connect to GitHub: No commit found for the ref cacheexc" -- am I doing something wrong here?

avatar mbabker
mbabker - comment - 14 Jul 2016

The last alpha package has a known bug. Use the stable package.

avatar coaleb coaleb - test_item - 14 Jul 2016 - Tested successfully
avatar coaleb
coaleb - comment - 14 Jul 2016

I have tested this item successfully on 3259f78

After update to Joomla 3.6.0 any site using memcache as its cache handler (even with cache disabled) would receive the "JLIB_APPLICATION_ERROR_COMPONENT_NOT_LOADING" warning. After patching in this PR, I now receive the much more helpful "Could not connect to memcache server" warning. Note that in my case, the website itself would still function both before & after patching in this PR.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10815.

avatar csthomas csthomas - change - 20 Jul 2016
The description was changed
avatar csthomas csthomas - change - 20 Jul 2016
The description was changed
avatar csthomas
csthomas - comment - 20 Jul 2016

I have updated Summary and Test instruction.

Can I ask for second test, I think it is worth to go to 3.6.1

avatar sandstorm871
sandstorm871 - comment - 21 Jul 2016

Sorry to hop in, I'll try to test this today as I have a few sites using "Memcached" with Sitegrounds VPS & Shared Hosting solutions.

Reading @coaleb input - Does this fix only change the "JLIB_APPLICATION_ERROR_COMPONENT_NOT_LOADING" warning to a message that says ""Could not connect to memcache server"

Is there a fix to allow J3.6 to use memcached?
Does it matter if the test is running PHP5.6.x or PHP7.0.x


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10815.

avatar csthomas
csthomas - comment - 21 Jul 2016

You can look at #11121 but it is patch for joomla with disabled cache.

Is there a fix to allow J3.6 to use memcached?

MemcacheD (storage in joomla) should work with J3.6

Does it matter if the test is running PHP5.6.x or PHP7.0.x

Directly no, but MemcachE is not available on PHP7.

If you test and this patch do what should do, means display better error message,
can you set comment "I have tested this item successfully" as in #10815 (comment)

avatar sandstorm871
sandstorm871 - comment - 21 Jul 2016

Tested Memcache (No D) with Joomla 3.6 PHP7 - I see the message The memcache Cache Storage is not supported on this platform.

Tested Memcache with Joomla 3.6 with PHP 5.6, 5.5, 5.4 - I see the message The memcache Cache Storage is not supported on this platform.

So have not tested with patch as appears to be OK for me without the patch?

screen shot 2016-07-21 at 13 05 35
Tested with these settings

        public $caching = '0';
    public $cache_handler = 'memcache';
    public $cachetime = '15';
        public $caching = '1';
    public $cache_handler = 'memcache';
    public $cachetime = '15';
        public $caching = '2';
    public $cache_handler = 'memcache';
    public $cachetime = '15';
avatar csthomas
csthomas - comment - 21 Jul 2016

Thanks,

then you have not installed memcachE extension on php 5.4/5.5/5.6
(on php7 ext. does not exists).

You have only memcacheD extension installed on all php versions.

avatar csthomas
csthomas - comment - 21 Jul 2016

This #11121 took my arguments. Now message is OK.

For now this PR will be only useful
when memcache/memcacheD server stop working after created connection in php request.

@mbabker What do you think about split isSupported() method into:

  • isSupported() which only check if extension is installed
  • isConnected() check connection

Example from currect redis:

    /**
     * Test to see if the storage handler is available.
     *
     * @return  boolean
     *
     * @since   3.4
     */
    public static function isSupported()
    {
        return class_exists('Redis');
    }
    /**
     * Test to see if the Redis connection is available.
     *
     * @return  boolean
     *
     * @since   3.4
     */
    public static function isConnected()
    {
        return static::$_redis instanceof Redis;
    }

then my PR still should be helpful.

If not, why does joomla use in cache.php if (!($handler instanceof Exception))
The handler is never set to instance of exception.

avatar mbabker
mbabker - comment - 21 Jul 2016

The code probably still implements Joomla 1.5 style error handling since return JError::raiseWarning() returns a JException instance and PHP 4 didn't let you throw stuff so they could only return the object. It'd still be better written as a try/catch versus checking returns at this point.

Honestly I don't think a public facing isConnected() method is all that helpful to the API and considering it's adapter specific it just makes things even harder (if I want to use it I have to wrap in method_exists() checks). Simple isSupported() should be all that's required to try and boot/use the adapter and proper catching of thrown Exceptions (no more of this JError style error handling or returning booleans) needs to be implemented.

I still don't agree with the notion of adding new code in 2016 that catches and returns an Exception object; I honestly believe only factory methods which build Exception objects should be returning them, but considering Joomla's code base is stuck somewhere between PHP 4.3 and 5.3 for structure what can you do?

avatar csthomas
csthomas - comment - 25 Jul 2016

I still don't agree with the notion of adding new code in 2016 that catches and returns an Exception object;

OK I agree. Then I close that PR.

Honestly I don't think a public facing isConnected() method is all that helpful to the API and considering

IMHO it will be helpful in storage.php as:

    public static function isConnected()
    {
        return static::isSupported();
    }

and only for a few storages (memcachE, redis, memcacheD) we will overrides that method.
I have problem with explaining benefits so I leave it for a next PR.

avatar csthomas csthomas - change - 25 Jul 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-07-25 09:54:04
Closed_By csthomas
avatar csthomas csthomas - close - 25 Jul 2016
avatar joomla-cms-bot joomla-cms-bot - change - 25 Jul 2016
Category Cache Libraries Cache
avatar mbabker
mbabker - comment - 25 Jul 2016

Just for reference my personal issue with the isConnected() method is it implies the notion that all storage adapters have some notion of a connection, which as you pointed out is only the case for not even half of the adapters we support, so to me there isn't a benefit to making it a standardized method for all adapters and especially when in most cases it'll simply be an alias to isSupported(). I've got no issue creating a JCacheStorageConnectable interface with the isConnected() method and having drivers that make a connection to an external resource implement that (instead of method_exists($driver, 'isConnected') you end up with a $driver instanceof JCacheStorageConnectable style check), but I'm not convinced nor do I see the benefit to that particular check being a part of the base class and a "requirement" for all drivers to have.

avatar csthomas
csthomas - comment - 25 Jul 2016

I more concern on isSupported() method.
I think that the method should only check if require extensions are avail. You don't.

Main issue.

Default port for memcache/D is hard-coded to 11211.
It is OK, but isSupported() tests the port first. Connection fails. The handler is not visible, but extension exists and memcache/D works on the other port 11311. But joomla admin can not set other port to test.

There is not place in back-end to test other port because backend not display that option.

If my server has memcache/D on port 11311 I do not see option to set memcache as cache handler.
I have to do it manual in configuration.php and change port to 11311.
Then it start working and backend start to show me all configuration options for memcache/D.

If server administrator set memcache/D on non standard port then joomla admin does not see Memcache/D handler options in Backend->Global Configuration->Cache Settings.

avatar mbabker
mbabker - comment - 25 Jul 2016

I more concern on isSupported() method.
I think that the method should only check if require extensions are avail. You don't.

In hindsight that was probably a bad move.

avatar csthomas
csthomas - comment - 25 Jul 2016

Can you say more explicitly.
Do you see some light in my approach?

May be you see other way to resolve my problem.

avatar mbabker
mbabker - comment - 25 Jul 2016

The part of isSupported() that's making sure you can actually connect to the server was probably a bad move and being over-eager.

Like I said, isConnected() isn't a bad idea. I just don't think it's appropriate at the base class (which is essentially the interface since we don't have one explicitly defined) level.

avatar csthomas
csthomas - comment - 25 Jul 2016

isConnected() not need to be at the base.
I only want to remove any test connection from isSupported() method.

Thanks.

avatar csthomas
csthomas - comment - 24 Aug 2016

I have prepared a better PR for catch cache exception.
I ask for test at #11731.

If someone interest with use memcache(-d) handler on J >= 3.6 then:

  • I fixed and added performance optimization for memcachE handler
  • I added performance optimization for memcacheD handler

at #11565

Add a Comment

Login with GitHub to post a comment