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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

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

Jeremy CreditAttribution: 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 CreditAttribution: Jeremy commented
Status: Needs work » Fixed

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

Thanks, Nat!

Jeremy’s picture

Jeremy CreditAttribution: 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 CreditAttribution: ogi commented

subscribe

markpavlitski’s picture

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

Patch for latest 7.x-dev attached.

pwaterz’s picture

pwaterz CreditAttribution: pwaterz commented
Issue summary: View changes

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

markpavlitski’s picture

markpavlitski CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: tsphethean commented
FileSize
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 CreditAttribution: 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 CreditAttribution: tsphethean commented

Thanks @pwaterz, will take a look at memcache_storage.

Jeremy’s picture

Jeremy CreditAttribution: Jeremy at Tag1 Consulting commented
Status: Needs review » Fixed

Memcache module doesn't support sessions.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent 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.