Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Cache] Add DSN, createClient & better error reporting to MemcachedAdapter#21108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion.travis.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -38,6 +38,7 @@ cache:
- php-$MIN_PHP

services:
- memcached
- mongodb
- redis-server

Expand All@@ -60,7 +61,7 @@ before_install:
- if [[ ! $skip && $PHP = 5.* ]]; then (echo yes | pecl install -f apcu-4.0.11 && echo apc.enable_cli = 1 >> $INI_FILE); fi
- if [[ ! $skip && $PHP = 7.* ]]; then (echo yes | pecl install -f apcu-5.1.6 && echo apc.enable_cli = 1 >> $INI_FILE); fi
- if [[ ! $deps && $PHP = 5.* ]]; then (cd src/Symfony/Component/Debug/Resources/ext && phpize && ./configure && make && echo extension = $(pwd)/modules/symfony_debug.so >> $INI_FILE); fi
- if [[ ! $skip && $PHP =5.* ]]; thenpecl install -f memcached-2.1.0; fi
- if [[ ! $skip &&!$PHP =hhvm* ]]; thenecho extension = memcached.so >> $INI_FILE; fi
- if [[ ! $skip && ! $PHP = hhvm* ]]; then echo extension = ldap.so >> $INI_FILE; fi
- if [[ ! $skip && ! $PHP = hhvm* ]]; then echo extension = redis.so >> $INI_FILE; fi;
- if [[ ! $skip && ! $PHP = hhvm* ]]; then phpenv config-rm xdebug.ini || echo "xdebug not available"; fi
Expand Down
1 change: 1 addition & 0 deletionsphpunit.xml.dist
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
<env name="LDAP_HOST" value="127.0.0.1" />
<env name="LDAP_PORT" value="3389" />
<env name="REDIS_HOST" value="localhost" />
<env name="MEMCACHED_HOST" value="localhost" />
</php>

<testsuites>
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@

namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler;

use Symfony\Component\Cache\Adapter\RedisAdapter;
use Symfony\Component\Cache\Adapter\AbstractAdapter;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
Expand DownExpand Up@@ -108,13 +108,13 @@ public static function getServiceProvider(ContainerBuilder $container, $name)
{
$container->resolveEnvPlaceholders($name, null, $usedEnvs);

if (0 === strpos($name, 'redis://') || $usedEnvs) {
if ($usedEnvs || preg_match('#^[a-z]++://#', $name)) {
$dsn = $name;

if (!$container->hasDefinition($name = md5($dsn))) {
$definition = new Definition(\Redis::class);
$definition = new Definition(AbstractAdapter::class);
$definition->setPublic(false);
$definition->setFactory(array(RedisAdapter::class, 'createConnection'));
$definition->setFactory(array(AbstractAdapter::class, 'createConnection'));
$definition->setArguments(array($dsn));
$container->setDefinition($name, $definition);
}
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -706,6 +706,7 @@ private function addCacheSection(ArrayNodeDefinition $rootNode)
->scalarNode('default_doctrine_provider')->end()
->scalarNode('default_psr6_provider')->end()
->scalarNode('default_redis_provider')->defaultValue('redis://localhost')->end()
->scalarNode('default_memcached_provider')->defaultValue('memcached://localhost')->end()
->arrayNode('pools')
->useAttributeAsKey('name')
->prototype('array')
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1256,7 +1256,7 @@ private function registerCacheConfiguration(array $config, ContainerBuilder $con
// Inline any env vars referenced in the parameter
$container->setParameter('cache.prefix.seed', $container->resolveEnvPlaceholders($container->getParameter('cache.prefix.seed'), true));
}
foreach (array('doctrine', 'psr6', 'redis') as $name) {
foreach (array('doctrine', 'psr6', 'redis', 'memcached') as $name) {
if (isset($config[$name = 'default_'.$name.'_provider'])) {
$container->setAlias('cache.'.$name, new Alias(Compiler\CachePoolPass::getServiceProvider($container, $config[$name]), false));
}
Expand Down
11 changes: 11 additions & 0 deletionssrc/Symfony/Bundle/FrameworkBundle/Resources/config/cache.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -93,6 +93,17 @@
</call>
</service>

<service id="cache.adapter.memcached" class="Symfony\Component\Cache\Adapter\MemcachedAdapter" abstract="true">
<tag name="cache.pool" provider="cache.default_memcached_provider" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- Memcached connection service -->
<argument /> <!-- namespace -->
<argument>0</argument> <!-- default lifetime -->
<call method="setLogger">
<argument type="service" id="logger" on-invalid="ignore" />
</call>
</service>

<service id="cache.default_clearer" class="Symfony\Component\HttpKernel\CacheClearer\Psr6CacheClearer">
<tag name="kernel.cache_clearer" />
</service>
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -215,6 +215,7 @@
<xsd:element name="default-doctrine-provider" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="default-psr6-provider" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="default-redis-provider" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="default-memcached-provider" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="pool" type="cache_pool" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -272,6 +272,7 @@ protected static function getBundleDefaultConfig()
'system' => 'cache.adapter.system',
'directory' => '%kernel.cache_dir%/pools',
'default_redis_provider' => 'redis://localhost',
'default_memcached_provider' => 'memcached://localhost',
),
'workflows' => array(),
'php_errors' => array(
Expand Down
2 changes: 1 addition & 1 deletionsrc/Symfony/Bundle/FrameworkBundle/composer.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,7 +17,7 @@
],
"require": {
"php": ">=5.5.9",
"symfony/cache": "~3.2",
"symfony/cache": "~3.3",
"symfony/class-loader": "~3.2",
"symfony/dependency-injection": "~3.3",
"symfony/config": "~2.8|~3.0",
Expand Down
1 change: 1 addition & 0 deletionssrc/Symfony/Bundle/FrameworkBundle/phpunit.xml.dist
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@
<php>
<ini name="error_reporting" value="-1" />
<env name="REDIS_HOST" value="localhost" />
<env name="MEMCACHED_HOST" value="localhost" />
</php>

<testsuites>
Expand Down
15 changes: 15 additions & 0 deletionssrc/Symfony/Component/Cache/Adapter/AbstractAdapter.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -115,6 +115,21 @@ public static function createSystemCache($namespace, $defaultLifetime, $version,
return new ChainAdapter(array($apcu, $fs));
}

public static function createConnection($dsn, array $options = array())
{
if (!is_string($dsn)) {
throw new InvalidArgumentException(sprintf('The %s() method expect argument #1 to be string, %s given.', __METHOD__, gettype($dsn)));
}
if (0 === strpos($dsn, 'redis://')) {
return RedisAdapter::createConnection($dsn, $options);
}
if (0 === strpos($dsn, 'memcached://')) {
return MemcachedAdapter::createConnection($dsn, $options);
}

throw new InvalidArgumentException(sprintf('Unsupported DSN: %s.', $dsn));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Should theAbstractAdapter really be aware of all adapters that require a connection?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is a stateless static method, no need for yet another class. This has to be somewhere :)
Other comments addressed.

}

/**
* Fetches several cache items.
*
Expand Down
188 changes: 177 additions & 11 deletionssrc/Symfony/Component/Cache/Adapter/MemcachedAdapter.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,68 +11,234 @@

namespace Symfony\Component\Cache\Adapter;

use Symfony\Component\Cache\Exception\CacheException;
use Symfony\Component\Cache\Exception\InvalidArgumentException;

/**
* @author Rob Frawley 2nd <rmf@src.run>
* @author Nicolas Grekas <p@tchwork.com>
*/
class MemcachedAdapter extends AbstractAdapter
{
private static $defaultClientOptions = array(
'persistent_id' => null,
'username' => null,
'password' => null,
);

protected $maxIdLength = 250;

private $client;

public static function isSupported()
{
return extension_loaded('memcached') && version_compare(phpversion('memcached'), '2.2.0', '>=');
}

public function __construct(\Memcached $client, $namespace = '', $defaultLifetime = 0)
{
if (!static::isSupported()) {
throw new CacheException('Memcached >= 2.2.0 is required');
}
$opt = $client->getOption(\Memcached::OPT_SERIALIZER);
if (\Memcached::SERIALIZER_PHP !== $opt && \Memcached::SERIALIZER_IGBINARY !== $opt) {
throw new CacheException('MemcachedAdapter: "serializer" option must be "php" or "igbinary".');
}
if (!$client->getOption(\Memcached::OPT_BINARY_PROTOCOL)) {
throw new CacheException('MemcachedAdapter: "binary_protocol" option must be enabled.');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I understand the serializer requirement, but why are weforcing binary protocol and no_block=false?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Binary because we must support all chars in keys especially spaces, per psr-6.
No block because we need guarantees when reading/writing to the cache. Dealing with async would make the code complex for no benefit.

Copy link
Contributor

@robfrawleyrobfrawleyDec 30, 2016
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Regardingbinary_protocol requirement: I'm on board with this.

But, regardingno_block requirement: I disagree. The intended use of this option doesn't require any additional code, and, in fact, writing such code would invalidate the entire intended usage and benefits ofno_block. The expectation when it is enabled is that the userdoesn't need guarantees when writing. It is acommonly used option and generally doesn't change expectations; due to the design of memcached, one can never assume a value exists as it may have been purged at any time by the LRU algorithm, and will definitely not exist after a restart of the daemon.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

fixed for no_block

$this->maxIdLength -= strlen($client->getOption(\Memcached::OPT_PREFIX_KEY));

parent::__construct($namespace, $defaultLifetime);
$this->client = $client;
}

public static function isSupported()
/**
* Creates a Memcached instance.
*
* By default, the binary protocol, no block, and libketama compatible options are enabled.
*
* Examples for servers:
* - 'memcached://user:pass@localhost?weight=33'
* - array(array('localhost', 11211, 33))
*
* @param array[]|string|string[] An array of servers, a DSN, or an array of DSNs
* @param array An array of options
*
* @return \Memcached
*
* @throws \ErrorEception When invalid options or servers are provided.
*/
public static function createConnection($servers, array $options = array())
{
return extension_loaded('memcached') && version_compare(phpversion('memcached'), '2.2.0', '>=');
if (is_string($servers)) {
$servers = array($servers);
} elseif (!is_array($servers)) {
throw new InvalidArgumentException(sprintf('MemcachedAdapter::createClient() expects array or string as first argument, %s given.', gettype($servers)));
}
set_error_handler(function ($type, $msg, $file, $line) { throw new \ErrorException($msg, 0, $type, $file, $line); });
try {
if (!static::isSupported()) {
throw new trigger_error('Memcached >= 2.2.0 is required');
}
$options += static::$defaultClientOptions;
$client = new \Memcached($options['persistent_id']);
$username = $options['username'];
$password = $options['password'];
unset($options['persistent_id'], $options['username'], $options['password']);
$options = array_change_key_case($options, CASE_UPPER);

// set client's options
$client->setOption(\Memcached::OPT_BINARY_PROTOCOL, true);
$client->setOption(\Memcached::OPT_NO_BLOCK, true);
if (!array_key_exists('LIBKETAMA_COMPATIBLE', $options) && !array_key_exists(\Memcached::OPT_LIBKETAMA_COMPATIBLE, $options)) {
$client->setOption(\Memcached::OPT_LIBKETAMA_COMPATIBLE, true);
}
foreach ($options as $name => $value) {
Copy link
Contributor

@robfrawleyrobfrawleyDec 30, 2016
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This needs to beforeach (array_reverse($options) as $name => $value) as seen in my PR, otherwise defaul values that affect multiple options (such aslibketama_compatible) will overwrite use-provided values.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think reverse or not, it's the responsibility of the user to order correctly. I'd better behave the same as the native setOptions so that expectations don't have to be reversed when using our lib...

Copy link
Contributor

@robfrawleyrobfrawleyDec 30, 2016
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It's due to our code, though. The default option oflibketama_compatible in combination with$options += static::$defaultClientOptions; means a user cannot change, for example, the hash algorithm, ever...as our default oflibketama_compatible will be appliedafter the user-provided option (for example['hash' => 'something-else']), resetting the hash tomd5. The options need to be applied in reverse order to allow the user to overwrite our defaults.

Copy link
Contributor

@robfrawleyrobfrawleyDec 30, 2016
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

To clarify: the user should, rightfully, assume that any options they provide will overwrite our defaults. The issue is,some options actually affect multiple options. Without reversing the array, the user options will not always result in a proper assignment; while reversing the options ensures user options always overwrite defaults.

It's not theorder of options that is important, it's the order ofdefault options touser options that is important.Default options must be applied first, so either the assignment needs to change:

# from this:$options +=static::$defaultClientOptions;# to this (or something similar):foreach (static::$defaultClientOptionsas$name =>$option) {if (!array_key_exists($name,$options) {array_unshift($options, [$name =>$option]);    }}

OR the order of options needs to be reversed:

# from this:foreach ($optionsas$name =>$value) {}# to this (or something similar):foreach (array_reverse($options)as$name =>$value) {}

Otherwise options will behave unexpectedly for the user.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

understood, fixed also

if (is_int($name)) {
continue;
}
if ('HASH' === $name || 'SERIALIZER' === $name || 'DISTRIBUTION' === $name) {
$value = constant('Memcached::'.$name.'_'.strtoupper($value));
}
$opt = constant('Memcached::OPT_'.$name);

unset($options[$name]);
$options[$opt] = $value;
}
$client->setOptions($options);

// parse any DSN in $servers
foreach ($servers as $i => $dsn) {
if (is_array($dsn)) {
continue;
}
if (0 !== strpos($dsn, 'memcached://')) {
throw new InvalidArgumentException(sprintf('Invalid Memcached DSN: %s does not start with "memcached://"', $dsn));
}
$params = preg_replace_callback('#^memcached://(?:([^@]*+)@)?#', function ($m) use (&$username, &$password) {
if (!empty($m[1])) {
list($username, $password) = explode(':', $m[1], 2) + array(1 => null);
}

return 'file://';
}, $dsn);
if (false === $params = parse_url($params)) {
throw new InvalidArgumentException(sprintf('Invalid Memcached DSN: %s', $dsn));
}
if (!isset($params['host']) && !isset($params['path'])) {
throw new InvalidArgumentException(sprintf('Invalid Memcached DSN: %s', $dsn));
}
if (isset($params['path']) && preg_match('#/(\d+)$#', $params['path'], $m)) {
$params['weight'] = $m[1];
$params['path'] = substr($params['path'], 0, -strlen($m[0]));
}
$params += array(
'host' => isset($params['host']) ? $params['host'] : $params['path'],
'port' => isset($params['host']) ? 11211 : null,
'weight' => 0,
);
if (isset($params['query'])) {
parse_str($params['query'], $query);
$params += $query;
}

$servers[$i] = array($params['host'], $params['port'], $params['weight']);
}

// set client's servers, taking care of persistent connections
if (!$client->isPristine()) {
Copy link
Contributor

@robfrawleyrobfrawleyDec 30, 2016
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We shouldn't add servers after a connection is open (even when the user has added an additional server to their configuration)?

Copy link
MemberAuthor

@nicolas-grekasnicolas-grekasDec 30, 2016
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Not sure to get what you mean. Do you see anything wrong with this logic?
Here it is: if the connection is not pristine (meaning it's a 2nd use persistent connection), then we check if there is any change in the list of server. If there is, we reset the list and set the new one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ok. Had trouble following the code, but makes sense now.

$oldServers = array();
foreach ($client->getServerList() as $server) {
$oldServers[] = array($server['host'], $server['port']);
}

$newServers = array();
foreach ($servers as $server) {
if (1 < count($server)) {
$server = array_values($server);
unset($server[2]);
$server[1] = (int) $server[1];
}
$newServers[] = $server;
}

if ($oldServers !== $newServers) {
// before resetting, ensure $servers is valid
$client->addServers($servers);
$client->resetServerList();
}
}
$client->addServers($servers);

if (null !== $username || null !== $password) {
if (!method_exists($client, 'setSaslAuthData')) {
trigger_error('Missing SASL support: the memcached extension must be compiled with --enable-memcached-sasl.');
}
$client->setSaslAuthData($username, $password);
}

return $client;
} finally {
restore_error_handler();
}
}

/**
* {@inheritdoc}
*/
protected function doSave(array $values, $lifetime)
{
return $this->client->setMulti($values, $lifetime) && $this->client->getResultCode() === \Memcached::RES_SUCCESS;
return $this->checkResultCode($this->client->setMulti($values, $lifetime));
}

/**
* {@inheritdoc}
*/
protected function doFetch(array $ids)
{
return $this->client->getMulti($ids);
return $this->checkResultCode($this->client->getMulti($ids));
}

/**
* {@inheritdoc}
*/
protected function doHave($id)
{
return $this->client->get($id)!== false|| $this->client->getResultCode() ===\Memcached::RES_SUCCESS;
returnfalse !==$this->client->get($id) || $this->checkResultCode(\Memcached::RES_SUCCESS === $this->client->getResultCode());
}

/**
* {@inheritdoc}
*/
protected function doDelete(array $ids)
{
$toDelete =count($ids);
foreach ($this->client->deleteMulti($ids) as $result) {
if (\Memcached::RES_SUCCESS=== $result|| \Memcached::RES_NOTFOUND=== $result) {
--$toDelete;
$ok =true;
foreach ($this->checkResultCode($this->client->deleteMulti($ids)) as $result) {
if (\Memcached::RES_SUCCESS!== $result&& \Memcached::RES_NOTFOUND!== $result) {
$ok = false;
}
}

return0 === $toDelete;
return$ok;
}

/**
* {@inheritdoc}
*/
protected function doClear($namespace)
{
return $this->client->flush();
return $this->checkResultCode($this->client->flush());
}

private function checkResultCode($result)
{
$code = $this->client->getResultCode();

if (\Memcached::RES_SUCCESS === $code || \Memcached::RES_NOTFOUND === $code) {
return $result;
}

throw new CacheException(sprintf('MemcachedAdapter client error: %s.', strtolower($this->client->getResultMessage())));
}
}
Loading

[8]ページ先頭

©2009-2025 Movatter.jp