Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different consistent distribution between v2.2.0 and v3.0.3 (MEMCACHED_BEHAVIOR_KETAMA vs MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED) #344

Closed
Vincent-- opened this issue May 10, 2017 · 7 comments
Milestone

Comments

@Vincent--
Copy link

To be sure we are speaking about the same things, see https://github.com/trondn/libmemcached/blob/master/docs/memcached_behavior.pod

MEMCACHED_BEHAVIOR_KETAMA
Sets the default distribution to MEMCACHED_DISTRIBUTION_CONSISTENT_KETAMA and the hash to MEMCACHED_HASH_MD5.

MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED
Sets the default distribution to MEMCACHED_DISTRIBUTION_CONSISTENT_KETAMA with the weighted support. and the hash to MEMCACHED_HASH_MD5.

So basically, I have 4 webservers using a pool of 2 memcache servers to store session.
I'm trying to upgrade to php7, so my current setup is like that:

  • web1: php7 + memcached3.0.3
  • web2,3,4: php5.6 + memcache2.2.0

Everytimes I try to enable web1, I'm loosing all my sessions. I've checked all the configuration options and everything seems to be ok. With a basic php script I can store and load the session from any webservers, but randomly the session created with one php version is not available from the other php version. Based on these tests, I'm pretty sure the culprit is the consistent distribution which seems different between 2.2.0 and 3.0.3.

From what I can see in the code, a change has been introduced in dc5b22a#diff-25c5e7d3aa23f27466a7f0bbc2c251ec

Previously the session engine was enabling MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED when sess_consistent_hash_enabled was in use:

                if (MEMC_G(sess_consistent_hash_enabled)) {
                    if (memcached_behavior_set(memc_sess->memc_sess, MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED, (uint64_t) 1) == MEMCACHED_FAILURE) {
                        PS_SET_MOD_DATA(NULL);
                        if (plist_key) {
                            efree(plist_key);
                        }
                        memcached_free(memc_sess->memc_sess);
                        php_error_docref(NULL, E_WARNING, "failed to enable memcached consistent hashing");
                        return FAILURE;
                    }
                }

After that, the default behaviour has been switched to MEMCACHED_BEHAVIOR_KETAMA:

    if (MEMC_SESS_INI(consistent_hash_enabled)) {
        check_set_behavior(MEMCACHED_BEHAVIOR_KETAMA, 1);
    }

I just wanted to know if this change has been done on purpose or if it's a mistake as I cannot see anything related to that in the changelog.
Is it possible to restore the initial behaviour so php5.6 and php7 can coexists in the same infra and use the same memcache pool?
Thanking you in advance.

@Vincent--
Copy link
Author

It also seems the PERSISTENT= option in session_path has been removed too (within the same commit)

@Vincent-- Vincent-- changed the title MEMCACHED_BEHAVIOR_KETAMA vs MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED Different consistent distribution between v2.2.0 and v3.0.3 (MEMCACHED_BEHAVIOR_KETAMA vs MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED) May 10, 2017
@Vincent--
Copy link
Author

Any update on that?

@sodabrew
Copy link
Contributor

I haven't looked into this yet. The changes you describe were almost certainly on-purpose, and the new settings might be "better" but at the expense of backwards compatibility. If there are some settings you want to see exposed to php.ini or to the Memcached constructor, I would welcome a PR that implements this!

@Vincent--
Copy link
Author

I'm not expert in the Ketama algorithm but from what I understand the KETAMA_WEIGHTED is an optimisation of the KETAMA that has been introduced in libmemcached v0.23, see http://bazaar.launchpad.net/~tangent-trunk/libmemcached/1.2/revision/351

So I'd really like to be sure this change has been made on purpose and is not the result of a bad copy/paste. If it's really on purpose, could we have a short explanation about this change?

Both the PERSISTENT= option in session_path and KETAMA_WEIGHTED are breaking changes: some simple comments about that in the changelog might be really helpful

@Vincent--
Copy link
Author

@mkoppanen

sodabrew added a commit that referenced this issue Apr 9, 2018
…eighted) (#392)

Provides a new INI option to select the same consistent hash behavior as older versions of php-memcached, to aid in migration from PHP 5.x to PHP 7.x. When `memcached_sess_consistent_hash_type` is set to `ketama` or `ketama_weighted`, that sets either MEMCACHED_BEHAVIOR_KETAMA or MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED, respectively.

Resolves #344
sodabrew added a commit that referenced this issue Apr 9, 2018
…eighted) (#392)

Provides a new INI option to select the same consistent hash behavior as
older versions of php-memcached, to aid in migration from PHP 5.x to PHP
7.x. When `memcached_sess_consistent_hash_type` is set to `ketama` or
`ketama_weighted`, that sets either MEMCACHED_BEHAVIOR_KETAMA or
MEMCACHED_BEHAVIOR_KETAMA_WEIGHTED, respectively.

Resolves #344
@sodabrew
Copy link
Contributor

sodabrew commented Apr 9, 2018

I'm not sure what's up with the PERSISTENT setting, but the weighted is now exposed as a new INI option for the 3.1.0 release (not released yet).

@sodabrew
Copy link
Contributor

sodabrew commented Apr 9, 2018

Can someone explain if it's necessary to provide the old PERSISTENT= setting, or is it just a matter of documenting that it's no longer compatible to have that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants