Problem/Motivation

Session support for memcache has been partially ported to Drupal 7, however it has not had enough testing to be considered stable, and there are core bugs with session support (particularly mixed http/https sessions) which are not correctly handled by memcache either (and likely won't be until the core bugs are fixed).

Proposed resolution

To help memcache session support in Drupal 7, please test the memcache-session.inc and open issues for any specific bugs found, linking to them from here, if an existing bug report does not already exist.

Remaining tasks

#1785744: Memcache Session Handler does not initialize user timezone
#1908348: Session backend fails to update user access time
#577214: $user->session_data_present_at_load is never set in sess_write()
#1908358: Incorrect require_once path in memcache-session.inc
#1984226: Session handler uses D6 hook_user() instead of D7 hook_user_update()
#973228: Port D7 core lazy session writing
#1396358: After installing memcache, getting mixed session problem - http & https

Comments

StatusFileSize
new11.38 KB

Some session_save_session were not renamed to drupal_save_session.

StatusFileSize
new31.02 KB

Mostly works. The tests , copied from core session tests do not pass. Other tests now do.

I've merged what you've implemented so far so as to not lose this effort, after confirming that there were no regressions to the core memcache functionality. Leaving open until all tests pass.

The challenge here is that we need to be able to query by both ssid and sid. I think storing (ssid, sid) pairs would help and then you need two requests when ssid comes into play but that's not a biggie.

Multiget is now fully functional in this module with both the PECL memcache and PECL memcached plugins, so multiple gets is not a problem.

but this multiple get is that if you have a ssid you request the sid and then request the value of sid. Ie. the second get depends on the first.

What's the status of this? What are the flaws or deficiencies of the session handling in the current d7 memcache module?

See above. Someone needs to write code so that if the request is per ssid then we can first retrieve the sid and then from the sid then the record. Or so I think.

Any update on this? I believe we already have code freeze on drupal sessions

StatusFileSize
new1.35 KB

I've got a patch that will allow for the session handling to work ( minus the ssid versus sid pieces ).

Thanks bblake, patch committed!
http://drupal.org/cvs?commit=473770

We're still needing to fix the ssid/sid issue, leaving status on this issue as 'needs work'.

subscribe

sub

with this patch in place, what's the right way to implement it?

if in settings.php,

$conf['session_inc'] = './sites/all/modules/memcache/memcache-session.inc';

on page load i see,

Notice: Undefined property: stdClass::$session in _drupal_session_read() (line 56 of /nas/srv/d7/006/sites/all/modules/memcache/memcache-session.inc).

do i need to include instead?

I think you need to do both the include and the $conf setting.

Priority:Normal» Major

I made an interim commit to remove an outdated cache constant from the test, there's still one failure here that looks genuine, let alone http vs. https sessions being broken in core too of course.

What failure exactly? I've run the tests this week and everything was green...

The fail I get is "Session data persists through browser close." line 98 of memcache-session.test

I didn't debug this though. Interesting if that test passes for you...

Yep, the tests seemed to work for me however, when actually switching memcache-session.inc on, I got the same error as reported in #14, probably just because memcache expected a session key to exist which didn't, so maybe a quick check there would make sense.

I left it on and everything worked just fine. On the next day, I did some unrelated, simple load tests and it suddenly stopped working *completely*. Plain simply no sessions at all, e.g. filling out the login form redirected you to the user page but then you were logged out again. Reproducable, for everyone in the room. Had to no time to investigate as others were working on the site so I switched it back off.

Not sure if it is still recommended to have a separate memcache instance for the sessions and memcache was rather full (still working on fine-tuning the whole setup) but even then, new cache_set()'s shouldn't simply be ignored?

Session should be in a separate bin - you want headroom so that sessions don't get evicted before they expire naturally.

However I agree that brand new sets shouldn't be affected by this, would probably be good to rule it out though.

Status:Needs work» Needs review
StatusFileSize
new1.18 KB

D7 drupal_anonymous_user() is updated so we also need to renew _memcache_session_user_load() with new syntax (http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...).

Patch via memcache-7.x-1.x, tested with drupal-7.9 and Memcache v3.0.4.

StatusFileSize
new1.38 KB

Thanks @hswong3i for the patch in #23. I found a few issues with the approach however, specifically for me drupal_set_message() was not working when the user was anonymous. That's because the patch was setting $user->session = '' if the $user->uid was 0. In most situations that's fine, but in the case of messages, they're currently broken (even with the patch in #23).

This patch makes safe changes by adjusting for the changes in drupal_anonymous_user(). If you look at the current documentation, you can see that passing in a $session is absolutely silly, because that function doesn't take any arguments at all. This patch simply provides the same functionality as D6 by adjusting for the change in the API. This patch in my testing solves the notice mentioned above and also work with anonymous user messages.

Not sure if this helps but I was getting the error:
Notice: Undefined property: stdClass::$session in _drupal_session_read() (line 56 of /var/www/www.mysite.com/sites/all/modules/contrib/memcache/memcache-session.inc).

Right after logging out of Drupal. Using:

Drupal core 7.10
Memcache 7.x-1.x-dev

This is probably not a good solution but I fixed it by placing this:
if (!isset($user->session)) {
$user->session = '';
}

Right before:
return $user->session;

In function _drupal_session_read($key)

Is exists any roadmap for porting this to D7? I need to move sessions from database and this solution will be great for me.

Status:Needs review» Active

I've committed #24 to 7.x-1.x, moving back to active since there's more to do here.

@jaroslaw.kaminski there are outstanding critical/major bugs in Drupal core's session handling in D7 that make it trickier to port the memcache version than it otherwise would be (since the actual desired behaviour is not set yet). As well as this, I have no time to spend on testing/fixing the session handling in D7 for memcache. I will however try to review/commit patches if they're posted here, and testing/reviewing existing patches in the queue, or posting bug reports with steps to reproduce if there's not an existing issue is very welcome.

I use memcache-session.inc and it works fine for me. Do you know any problems with memcache sessions? I need sessions for production site but I don't know if it can be usable. In readme.txt is written that it is dev version only. I can fix memcache-sessions but don't know about problems with it.

Nobody knows how stable this is, that's basically the reason for this issue to be still open and the code to be in the unstable directory.

So, test it and report back with your findings :)

My own "survey" answer: I use the memcache-session.inc from unstable on several D7 site and it works fine until now. None of them is really in production yet, but as far as I can tell it doesn't loose any session so far (a dozen different users, sometimes simultaneous).

I think We can commit it first, but stated which needs more testing, Then more people can test.

I'm having an issue with this. If I use user_save to change the current user's name it will not update the current session.

global $user;
echo $user->name // wedge
user_save($user, array('name' => 'wedge1'));

And then later in the session check $user->name
global $user;
echo $user->name // still wedge...

It works fine with the standard db session handling. Any ideas?

Hey, can somebody link in the "multiple core issues" that chx is referencing so we can see/track the status of those in this thread?

you meant catch in #27

I didn't actually, this is C&P from the OP:

and there are core bugs with session support (particularly mixed http/https sessions) which are not correctly handled by memcache either (and likely won't be until the core bugs are fixed).

But yes, including catch's references would be great too :)

I've had great success with the current memcache-session.inc file. I've been running http://webform.com for months using the memcache session handler with no loss of sessions, user login/logout works fine.

A few things I did that may be helping prevent any problems:
- I set up an entirely different memcache daemon/port just for sessions.
- I don't allow HTTP session cookies at all, all sessions are over HTTPS by setting session.cookie_secure = 1 in php.ini.

So unfortunately I don't know if my situation helps qualify the session handler as stable since I've worked to separate session handling from the rest of the site.

I did discover a bug with timezone handling (apparently in D7, the session initialization is responsible for timezones): #1785744: Memcache Session Handler does not initialize user timezone

@#36 -- The README currently claims:

Note you MUST have a session and a users server set up for memcached sessions to work.

So apparently you've just followed the directions.

StatusFileSize
new884 bytes

Hi,

I just found memcache-session.inc has wrong hook

drupal 6 style filter_user should be replace to filter_user_update, or user roles can't update correctly.

StatusFileSize
new522 bytes

I've been debugging this some and found the following:

<?php
 
// Otherwise, if the session is still active, we have a record of the
  // client's session in memcache.
 
$session = dmemcache_get($key, 'session');
 
$user = _memcache_session_user_load($session);
 
// Record whether this session contains data so that in sess_write() it can
  // be determined whether to skip a write.
 
$user->session_data_present_at_load = !empty($session->session);
  return
$user->session;
?>

$session->session contains the correct information that one would expect.
$user->session will never exist for when $user->uid == 0 (anon). See http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/drupa...

What I found in $session->session is a serialized variable of all the messages from drupal_get_messages(), so... wouldn't the accurate thing to be:

<?php
if ( (empty($user->session)) && (!empty($session->session)) ) {
 
$user->session = $session->session;
}
?>

This seems to make more sense to me. But I don't know enough about memcache as a whole to know what else this might break... but if the solution so far is: pass empty string back, then I don't see a problem with this.

Of course, I can't find where the returned value is actually used, so does the return matter?

in drupal_session_start(), there's no capturing of the return value. Maybe this is a different bug, though.

StatusFileSize
new572 bytes

I wrote a patch for #40 in case it's useful.

@Jeremy Would it help to have each of these patches in a separate ticket so they can be RTBC'd?

#1908358: Incorrect require_once path in memcache-session.inc fixes a WSOD when trying to use memcache-session.inc in the unstable directory.

StatusFileSize
new1.08 KB

Updated the patch I created, since I found an edge case where it's possible for $session to be lost when memcached is restarted.

<?php
 
if (empty($user->session)) {
    if (!empty(
$session->session)) {
     
$user->session = $session->session;
    }
    else {
     
$user->session = '';
    }
  }
?>

I've yet to have any warnings using this code. I've also been able to use $_SESSION variables reliably with anon users. Win-win.

Issue summary:View changes

Updated issue summary

Title:Port sessions to D7 [META] Port sessions to D7
Component:Code» memcache-session.inc
Issue summary:View changes

Issue summary:View changes