Skip to content

Memcache::get should set its $flags parameter to an int #105

Open
@p00ya

Description

@p00ya

From https://www.php.net/manual/en/memcache.get.php:

If present, flags fetched along with the values will be written to this parameter. These flags are the same as the ones given to for example Memcache::set(). The lowest byte of the int is reserved for pecl/memcache internal usage (e.g. to indicate compression and serialization status).

However, in the App Engine SDK the $flags parameter of Memcache::get is not a reference parameter: https://github.com/GoogleCloudPlatform/appengine-php-sdk/blob/master/src/Api/Memcache/Memcache.php#L211

  public function get($keys, $flags = null) {

Some Memcache clients rely on the PECL behaviour of setting $flags to an int to indicate a successful lookup. This is the only way the PECL API provides to distinguish a cache hit on a false value from a cache miss. An example is https://github.com/Automattic/wp-memcached/blob/4.0.0/object-cache.php#L432-L441:

			$flags = false;
			$this->timer_start();
			$value = $mc->get( $key, $flags );
			$elapsed = $this->timer_stop();

			// Value will be unchanged if the key doesn't exist.
			if ( false === $flags ) {
				$found = false;
				$value = false;
			}

I think App Engine's Memcache::get() should set the $flags, using the value from $item->getFlags() at https://github.com/GoogleCloudPlatform/appengine-php-sdk/blob/master/src/Api/Memcache/Memcache.php#L190. Some thoughts though:

  • It will be a bit weird that Memcache returns $flags on get but ignore $flag on set. But this isn't a regression, and it doesn't even seem that asymmetric to me (App Engine will continue not to support client-set flags).
  • This will expose some internals, which clients shouldn't rely on (other than $flags being overwritten by an int). But this is similar to the PECL module, whose documentation states "the lowest byte of the int is reserved for pecl/memcache internal usage (e.g. to indicate compression and serialization status)". Alternatively, it could set $flags to a constant like 0 on lookup success.

FYI I'm also a Google SWE, LMK if I should follow up here or internally for contributing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions