tollmanz / wordpress-pecl-memcached-object-cache Goto Github PK

There is a lot of redundancy handling the local cache. It should be abstracted.

I set up test coverage for the Redis drop-in here: alleyinteractive/wp-redis#2

Should be pretty easy to port over should you want to.

Get multiple functions return the derived keys (e.g., "default:my-key"). The group should be extracted.

See if casting in prepend is the same as it is with append. If so, apply the same routines in prepend as was used in append.

The following code fails:

$m->get( 'my-key', null, $cas_token );

It fails because the first conditional in the method sees the third argument as null and therefore does not call the Memcached get method. Instead it pulls from the internal cache. A way around this is to do something like:

$cas_token = ''; $m->get( 'my-key', null, $cas_token );

Using the native Memcached class' get method works with the first example. This class should emulate that.

But it doesn't seem this class has that method defined. Any suggestions?

$this->set_visible( is_object( $wp_object_cache ) && method_exists( $wp_object_cache, 'stats' ) );

Need to explore if the blog_id is sufficient or if blog_prefix is also necessary. This will be very clear once MS is tested more fully.

Any thoughts on adding composer support? I think it would go well with publishing on the WP plugin repo (#26) as it would then be included on http://wpackagist.org/

http://roots.io/wordpress-plugins-with-composer/

So I have a high availability setup with a load balancer to two wordpress servers with wordpress-pecl-memcached-object-cache installed.

Obviously when one cache is updated, the other is not. If I specify each individual server IP in global $memcached_servers, does it mean that when one memcached is updated, the other will be concurrently updated?

The normal $memcache_servers array needs to be converted to the style array used in this class.

After installing the drop in, I get the error:
Warning: Cannot use a scalar value as an array in /var/www/vhosts/domain.com/httpdocs/wp-includes/option.php on line 68

Any ideas?
WP Version 3.8.1

Using like this:
$memcached_servers = 'unix:/tmp/memcached.sock';
or
$memcached_servers = 'unix:///tmp/memcached.sock:0';

After significant thinking, waffling, and misunderstandings about the *ByKey methods, I have decided to remove the notion of server keys within the internal cache.

Memcached's implementation of the ByKey methods are a little confusing and have been the primary reason I've changed my mind about how to tackle this issue. After a lot of testing, I have found the following truths to be self-evident:

1. When adding a value to cache with a ByKey method, you should be able to retrieve that value with a ByKey method if you use the same server key.

2. When adding a value to cache with a ByKey method, if you request the key with a ByKey method using a different server key, it may or may not find the result. This is dependent upon whether the different server key used is mapped to the same server or not.

3. The server key mapping is entirely arbitrary. The only way to tell if two server keys refer to the same server would be to compare them using the getServerByKey method.

4. I could theoretically add my own server key/server mapping to the class, which would allow me to appropriately store values in the internal cache by key which should lead to the exact same result returned from the internal cache as from memcached.

5. By not storing values by server key, there is a reduction in overhead to store/access keys from the internal cache. Additionally, I do not see any benefits of doing so.

Given this, I will be removing the notion of server keys from the internal cache (which I unfortunately just reimplemented).

Additionally, make sure that values are coming from the appropriate place (i.e., memcached vs. internal cache).

i see that message when typing: $m = new Memcached();

Any Help?

Please consider supporting
http://redislabs.com/memcached-cloud
Redis Labs needs SASL auth for memcached.

Thank you!!

There are possibly performance benefits with the binary protocol. Currently, setting this option causes the CAS functions to fail. The option should be set via the config rearrangement in #40.

Just asking...

PECL Memcached exposes a lot of options. This library allows you to set some of them via the wp_cache_set_option() function. While this will work in some cases, it's not a great way to set options for the object cache prior to core using the cache.

I would like to extend $memcached_servers into something like $memcached_config that contains these options. These options would be initiated as soon as the object cache is initiated. This would also make it easier to test these options in Travis CI.

The other WordPress object cache backends have stats functionality. It might be worth adding this here, but perhaps using a different method to reduce overhead.

When PHP returns an object, it does so by reference, which means that the returned value has the capability to change the "original" value. This tends not to matter, but in the case of object caching it can cause big (and confusing) problems. If an object (i.e. is_object(), not just a generic name for data) is returned from cache, then changed, the change affects the data in the cache. The most common way this manifests itself in WordPress is through filters. Here's an example.

function es_runaway_filter( $theme ) {
    $theme->price = $theme->price * 2;
    return $theme;
}
add_filter( 'get_premium_theme', 'es_runaway_filter' );

function get_premium_theme( $id ) {
    // Let's assume this object is always in cache
    return apply_filters( 'get_premium_theme', wp_cache_get( $id, 'themes' ) );
}

echo get_premium_theme( 123 )->price; // Let's say this prints 100
echo get_premium_theme( 123 )->price; // This prints 200
echo get_premium_theme( 123 )->price; // This prints 400

As you can see, each successive call gets the data that's already been altered by all of the previous calls.

The solution to this is to check if a value is an object (again, is_object() style) before returning it from cache (local and remote). If it is, clone it before returning it for happy filtering.

Nacin recommended that the "wp" namespace not be used for these functions. I need to settle on a good namespace prefix. Thinking "memd" for the moment. Once that is done, will need to update unit tests.

First of all thanks for your work this.

Me, @franz-josef-kaiser and other fellows are very interested on the usage of this package on a pretty big website and integrate it in our workflow that is based on Composer and environment variables.

I already seen there's a PR about supporting Composer, and that's fine, so what is missing is the second part.

What I have in mind is something that affect only WP_Object_Cache constructor, that should be something like this:

public function __construct( $persistent_id = NULL ) {

    global $memcached_servers;

    // some code here

   $default_servers = array( array( '127.0.0.1', 11211 ) );

   if ( ! isset($memcached_servers) && ( $env = getenv( 'WP_MEMCACHED_SERVERS' ) ) ) {
     $servers = explode( ';', $env );
     $memcached_servers = array();
     foreach( $servers as $server ) {
          $data = explode( ':',  $server );
          if ( ! empty( $data ) ) {
            $port = isset( $data[1] ) && is_numeric( $data[1] ) ? (int) $data[1] : 11211;
            $memcached_servers[] = array( $data[0], $port );
          }               
       }
   }
   $this->servers =  $memcached_servers ? $memcached_servers : $default_servers;

   // rest of the code here
}

In this way would be pretty simple to add servers like this:

putenv( "WP_MEMCACHED_SERVERS=127.0.0.1:11211;127.0.0.2:11211" );

With a little edit on code above it would be also possible support unix socket (as proposed in #42) and for authentication (related: #32):

// unix socket
putenv( "WP_MEMCACHED_SERVERS=unix:/tmp/memcached.sock" );

// authentication
putenv( "WP_MEMCACHED_SERVERS=username:[email protected]:11211" );

Finally, a very similar approach can be used to support various Memcached settings (related: #40) using a single ENV var (e.g. WP_MEMCACHED_CONFIG) or more variables, one per setting.

I think that a configuration based on environment variables would be apprecciated by a lot people: Bedrock is pretty popular, and WP Starter is young but promising, even if I'm biased regarding the latter.

Moreover, per code above, it only has effect if global var is not set and the environment variable is, so it will not affect who don't use environment vars.

If you like this idea I can make a PR.

Any reason why this would add 6-10 seconds of page generation time? I'm assuming I have a configuration problem somewhere. I have both memcache and memcached pecl extensions installed, wp-config is configured as you specified and my memcached config is below:

-d
logfile /var/log/memcached.log
-m 512
-p 11211
-u memcache
-l 127.0.0.1
-c 4096
-k
-r
-t 8
-b 8196

Cheers,
Ed

Need to make sure that the internal cache is not inadvertently avoid when using the wrapper functions. If the num of args isn't examined, a call to memcached may be prompted when not necessary.

Need to fail gracefully when this error occurs.

I failed to specify a license when this was initially released. Since then, I have accepted contributions from others. Following VVV's lead, I think it is important that current contributors have a say in the licensing.

I think we should settle on MIT or GPLv2 and I personally lean toward MIT, but am up for any thoughts folks have.

Can the following individuals please let me know your preference:

• @Giuseppe-Mazzapica
• @swalkinshaw
• @aaronjorbin
• @JustinSainton
• @cmmarslender
• @webaware
• @Ramoonus

Thanks, everyone!

This might be handled by Memcached internally, but it is important that this be explicitly explored. In the event that a non-callable function is sent to the "get" methods, this class would need to check for the issue if Memcached does not explicitly handle it.

All of the wp_cache_* functions have not been tested. The current unit testing directly tests the WP_Object_Cache methods. It should be relatively straight forward to port the tests over for tested the wrapper functions. It is important to do this testing in order to verify that the methods are being called correctly.

Possible noob question...I'm using the Wordpress REST API and have implemented this, but when I make an xhr request it doesn't seem to to be caching properly. The test script works and returns:

Value is 100. The WordPress Memcached Backend is working!

Is there anything else I need to do in order for this to work on data loaded via the wordpress REST API? This is the response header ...

Access-Control-Allow-Headers:Authorization
Access-Control-Expose-Headers:X-WP-Total, X-WP-TotalPages
Allow:GET
Cache-Control:no-cache, must-revalidate, max-age=0
Connection:Keep-Alive
Content-Length:1741
Content-Type:application/json; charset=UTF-8
Date:Sat, 14 May 2016 21:37:46 GMT
Expires:Wed, 11 Jan 1984 05:00:00 GMT
Keep-Alive:timeout=5, max=99
Last-Modified:
Pragma:no-cache
Server:Apache/2.2.22 (Ubuntu)
X-Content-Type-Options:nosniff
X-Powered-By:PHP/5.3.10-1ubuntu3.22
X-WP-DeprecatedFunction:register_api_field (since WPAPI-2.0; use register_rest_field instead)
X-WP-Total:1
X-WP-TotalPages:1

I'm not sure if this was intentional or discussed before, we use(d) a fork of this dropin: https://wordpress.org/plugins/memcached/ which I'm sure most are familiar with.

The format for defining $memached_servers global in that plugin is as such:

$memcached_servers = array( 'hostname:port' );

Whereas wordpress-pecl-memcached-object-cache uses:

$memcached_servers = array( array( 'hostname', 'port' ) );

This annoyingly makes the two configurations very similar but different. I am running the same codebase with each dropin, which makes it a pain to do different configuration. I just wondered if compatibility had been discussed for these?

Thanks!

See here: https://remaintenance.io/blog/2015/02/fixing-woocommerce-memcached-performance/

Basically when expiration is set more than 30 days in the future, other memcache object cache implementations have problems.

Does this implementation handle that scenario correctly?

This was originally fixed in #44 - however it looks like those changes haven't made it into the develop branch, and are therefore not included in the version of the drop-in that is available to composer.

In the add, set, replace, append, etc. function, instead of checking against (bool) false to determine if the internal cache should be set, getResultCode should instead be checked. This allows for values of (bool) false to be properly stored.

Hi @tollmanz
Following some work on my side to test this plugin under load tests I thing I see an issue when non of the memcached servers are available.
When running the load test and stopping the memcached instance(s), over time, the usage of memory on the php server is increasing.

Once I start the memcached instance(s) again, the memory utilization is kept on the same level.

My setup is apache 2.4 with mod_php.
Memory is used by the apache/php processes.

I'll try to make sure this issue is not only due to my PR #54 #55 and if it isn't, I'll try to look into PHP memory dumps to troubleshoot that.
Are there any known memory issues?

I have multiple single-site installs on a single server. All use the same memcache server.

I want to make sure each site uses distinct keys to avoid collisions.

From the code, it looks like I have to manually specify a unique WP_CACHE_KEY_SALT in each site's wp-config.php to guarantee each site has unique namespacing of keys.

Might it be better to make WP_CACHE_KEY_SALT default to db_name + table_prefix? This should prevent collisions if I forget to define WP_CACHE_KEY_SALT. Some single-site installs on a single server will use the same database with different table prefixes, some will use different databases with the same table prefix, but a combination of the two should guarantee uniqueness. Inspired by here.

I also looked into using site_id + blog_id, but because they're single-site installs and thus unaware of each other, it doesn't look like this works. Alternatively, perhaps there's a variable for the domain name/subfolder that could be used instead?

I can submit a PR if desired, just wasn't sure if this was the best way to handle the issue... I'm rather new to Wordpress.

Per @swalkinshaw, it is a much better idea to use a built in PHP function to create a symlink than shelling out to do it.

Hey hey, what's the license of this wonderful thing? I want to use it in another plugin I'm making, is that okelidokely?

There is now a wp_cache_switch_to_blog that needs to be supported in WP 3.5. Research and add this support.

Would you use a define?

define('MEMCACHE_SERVERS', serialize(array(
    array(
        '127.0.0.1',
        11211
    ),
    array(
        'domain.com',
        11211
    )
)));

var_dump(unserialize(MEMCACHES_SERVER));

Shortish version. If I activate this for my multisite, everything works except the shopping cart. That gets all weird and will show cached data, when it really shouldn't.

The whole site is https. Shouldn't that not be getting cached?

Since removing the server keys from the internal cache (26116b1), all of the methods that have a ByKey relative, are essentially the same function other than the difference in the call to memcached. The code could be greatly simplified by trying to integrate methods with their ByKey relatives. This would likely involve adding another parameter to the functions that allow to toggle between the normal function and the ByKey version of the function.

@bradyvercher reported that when using Batcache, wp_cache_add() is called before wp_suspend_cache_addition() is defined causing a fatal error. This is due to Batcache loading the object cache before it is normally added. As such, wp_suspend_cache_addition() is not defined.

This issue can be resolved by adding a function_exists() call before using wp_suspend_cache_addition().

This isn't so much of a bug report as a question about how object-caching in Wordpress actually works under the covers. I thought I understood Wordpress caching, but then I read some docs + code, and now I'm confused... any help is appreciated.

I'm trying to accomplish the following:

1. Use Wordpress built-in caching mechanism to cache stuff for logged-in users. From my understanding, this stores the output of object compilation so PHP can work with pre-assembled objects and doesn't have to recompute values and re-hit the DB... among other things, it can make the admin panels faster. I've been happily using this plugin to tell Wordpress to use memcache as the backend for the built-in object cache with no issues.
2. Add a full-page cache for logged-out users. I'd like to use WP-FFPC or WP-Supercache to serve pre-generated pages from Nginx/memcache or Nginx/disk and entirely skip the PHP interpreter/MySQL.

Conceptually these are very different, so I assumed they wouldn't conflict, but I just tried installing WP-FFPC and WP-Supercache, and both of them require that I set define('WP_CACHE', true); in wp-config.php, which makes me wonder if my mental model of Wordpress caching is inaccurate? Any idea if those full-page caching plugins will conflict with this object-cache?

Cross-posting here: petermolnar/wp-ffpc#40

This needs testing, but there is a good chance this will mess up global groups. The fix would be to use prefixes like Boren's object cache.

It appears that the *ByKey methods in Memcached are very arbitrary. It is hard to match a key to a server. The following changes are suggested:

1. In the event that there is a single server, route all *ByKey requests to a single $server_key for the runtime change. The getServerByKey method may need to be utilized to make this work correctly.

2. Will likely need to create an internal mapping of server keys and servers in order to keep runtime cache and memcache servers in sync.

By now there are no tags. This makes it hard (or: impossible) to use anything aside from fake releases (branch aliasing) via Composer. This also makes it impossible to request this package without Composer using Git, cloning everything and then checking out the specific tag. This should get fixed.