D7 introduced an optimization for session writing in #744384: Do not write unchanged sessions to the database.

While it's not such a big saving with memcache, when profiling a D6 site I noticed two dmemcache_set() from memcache-session.inc which could be skipped using much the same logic.

Attached patch is against D6, this can likely be ported across to 7.x too though.

CommentFileSizeAuthor
#11 memcache-lazy-session-writing-973228-11.patch4.35 KBtsphethean
#7 memcache-lazy-session-writing-973228-7.patch4.64 KBmarkpavlitski
memcache_session.patch4.86 KBcatch

Comments

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
jeremy’s picture

jeremy commented
Status: Needs review » Needs work

With this patch in place, I've started seeing the following PHP notice when I log out:

Notice: Trying to get property of non-object in /var/www/memcache6/sites/default/modules/memcache/memcache-session.inc on line 60

We could use $user->session_data_present_at_load to determine whether or not $session->sid is available and avoid this error.

Otherwise, I'm not seeing any problems introduced by this patch.

jeremy’s picture

jeremy commented
Status: Needs work » Fixed

Fixed & committed:
http://drupal.org/cvs?commit=465158

Thanks, Nat!

jeremy’s picture

jeremy commented
Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Fixed » Patch (to be ported)

Marking as "to be ported" to remember that this still needs to be ported to D7.

ogi’s picture

ogi commented

subscribe

markpavlitski’s picture

markpavlitski commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new memcache-lazy-session-writing-973228-7.patch4.64 KB

Patch for latest 7.x-dev attached.

pwaterz’s picture

pwaterz commented
Issue summary: View changes

@markpavlitski What the state of that patch? How much testing has been done it?

markpavlitski’s picture

markpavlitski commented

@pwaterz personally I've done a reasonable amount of testing against it (at the time of submission) and have been using it in a production environment.

I can't vouch for it now however as there have been a slew of fixes and changes to the module over the last few months.

Unfortunately I don't have much spare time to work on this at the moment.

pwaterz’s picture

pwaterz commented

Thanks Mark, that exactly what I needed to know. I have big push from my company to get this working, so I will most likely be putting some time into getting the sessions working in the next few weeks.

tsphethean’s picture

tsphethean commented
StatusFileSize
new memcache-lazy-session-writing-973228-11.patch4.35 KB

Had a go at re-rolling the patch from #7 against the latest 7.x-1.x branch.

Will be do some further testing on this soon hopefully.

pwaterz’s picture

pwaterz commented

Just though I would note that we switched to using the memcache_storage module instead of this one because this one seems to be abandoned. Have had zero issue since we made the switch. Session handling works perfect.

tsphethean’s picture

tsphethean commented

Thanks @pwaterz, will take a look at memcache_storage.

jeremy’s picture

jeremy commented
Status: Needs review » Fixed

Memcache module doesn't support sessions.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Fixed » Closed (won't fix)

"Closed (won't fix)" is the appropriate status if the proposed change isn't going to be included.