Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Sergio Salvi wrote: I've applied both patches into this branch: http://dev.catalyst.perl.org/svnweb/Catalyst/browse/branches/Catalyst-Plugin-Session/both/ Hi, sorry for the very late followup on this, but it's been noted that the documentation wasn't adjusted to reflect the changes made. I don't suppose you could document how it works now correctly: http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Plugin-Session/0.00/trunk/ Many thanks in advance. t0m ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On Thu, Dec 11, 2008 at 12:04 PM, Sergio Salvi wrote: > On Thu, Nov 20, 2008 at 4:49 PM, Tobias Kremer wrote: >> On 20.11.2008, at 21:16, Sergio Salvi wrote: >>> >>> I still think the final solution (besides finding a way to make >>> find_or_create() atomic), is to store flash data the session row >>> (either on the same column of session or on a new, dedicated column). >> >> Sergio++ >> >> FWIW, I rolled my own flash mechanism which does exactly that (store the >> flash value in the session) and haven't looked back since. I've seen about 3 >> duplicate entry errors in the last 3 months opposed to several hundreds a >> week with C::P::Session's flash method. >> > > I've committed the "flash in session" patch, please try it out: > > http://lists.scsys.co.uk/pipermail/catalyst-dev/2008-December/001573.html > I've applied both patches into this branch: http://dev.catalyst.perl.org/svnweb/Catalyst/browse/branches/Catalyst-Plugin-Session/both/ This is supposed to fix: - duplicate key issue when using flash() - session finalization order race condition Please try it out :) Sergio Salvi ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On Thu, Nov 20, 2008 at 4:49 PM, Tobias Kremer wrote: > On 20.11.2008, at 21:16, Sergio Salvi wrote: >> >> I still think the final solution (besides finding a way to make >> find_or_create() atomic), is to store flash data the session row >> (either on the same column of session or on a new, dedicated column). > > Sergio++ > > FWIW, I rolled my own flash mechanism which does exactly that (store the > flash value in the session) and haven't looked back since. I've seen about 3 > duplicate entry errors in the last 3 months opposed to several hundreds a > week with C::P::Session's flash method. > I've committed the "flash in session" patch, please try it out: http://lists.scsys.co.uk/pipermail/catalyst-dev/2008-December/001573.html Regards, Sergio Salvi > --Tobias > > > ___ > List: Catalyst@lists.scsys.co.uk > Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst > Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ > Dev site: http://dev.catalyst.perl.org/ > ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On 20.11.2008, at 21:16, Sergio Salvi wrote: I still think the final solution (besides finding a way to make find_or_create() atomic), is to store flash data the session row (either on the same column of session or on a new, dedicated column). Sergio++ FWIW, I rolled my own flash mechanism which does exactly that (store the flash value in the session) and haven't looked back since. I've seen about 3 duplicate entry errors in the last 3 months opposed to several hundreds a week with C::P::Session's flash method. --Tobias ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On Fri, Sep 26, 2008 at 3:49 PM, Daniel Westermann-Clark <[EMAIL PROTECTED]> wrote: > On 2008-08-26 09:47:59 +0200, Tobias Kremer wrote: >> a) Patch Catalyst::Plugin::Session::Store::DBIC to wrap the flash >>functionality in a transaction (of course, this must be >>configurable). > > I've released a new version which includes this functionality: > > 0.07 Wed Sep 24 17:08:34 EDT 2008 >- Code was silently truncating storage to MySQL, rendering the > session unreadable. Patched to check DBIx::Class size from > column_info (if available) >- Wrap find_or_create calls in a transaction to (hopefully) > avoid issues with duplicate flash rows > Thanks for the patch, but unfortunately it does not solve the problem with duplicate flash rows, because just wrapping find_or_create() inside a txn_do() doesn't make it an atomic operation (because find_or_create is simply not atomic, as pointed out in this thread). The biggest problem is that the flash row gets deleted from the database when flash is empty, so we're always doing insert & delete and triggering the find_or_create problem. I have a template to display error messages that is included by almost every page, and these error messages are stored in flash. So every request that does *not* have anything in flash and does not add anything to flash, basically inserts the row (because my template did something like "FOREACH c.flash.my_error_messages"), leaves the flash empty and then decides to delete it. Make this happen too quickly and you're hitting this problem very often (like we were on our application). I've modified Session.pm not to delete flash, even when it's empty. The problem is gone, but it's a temporary hack I did to my local version of Session.pm. I still think the final solution (besides finding a way to make find_or_create() atomic), is to store flash data the session row (either on the same column of session or on a new, dedicated column). I could try coming up with a patch + tests for this. Thoughts? Thanks, Sergio Salvi ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On 2008-08-26 09:47:59 +0200, Tobias Kremer wrote: > a) Patch Catalyst::Plugin::Session::Store::DBIC to wrap the flash >functionality in a transaction (of course, this must be >configurable). I've released a new version which includes this functionality: 0.07 Wed Sep 24 17:08:34 EDT 2008 - Code was silently truncating storage to MySQL, rendering the session unreadable. Patched to check DBIx::Class size from column_info (if available) - Wrap find_or_create calls in a transaction to (hopefully) avoid issues with duplicate flash rows Thanks, -- Daniel Westermann-Clark ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Quoting Jonathan Rockway <[EMAIL PROTECTED]>: > * On Wed, Aug 27 2008, Tobias Kremer wrote: > > We definitely should! IMHO five queries per request to the database just > > for the session and flash stuff is inacceptable. > If your app is really slowed by the number of queries to load > session/flash, use this: Hmm .. That wasn't really my point. I was trying to say that it should theoretically be possible to reduce the database queries from 5 down to 2 without losing any of the functionality just by changing the way the flash DBIC storage is implemented. On a side note: I have replaced all my $c->flash calls with a custom method that stores the flash message in the already existent $c->session hash. I haven't had a single "duplicate entry" error since that change. So implementing the suggested change would eliminate the race condition, too. Unfortunately I haven't had any luck patching the existing DBIC session store to utilize the session instead of separate "flash:" entries :( --Tobias ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
* On Wed, Aug 27 2008, Tobias Kremer wrote: > Quoting Daniel Westermann-Clark <[EMAIL PROTECTED]>: > >> On 2008-08-26 14:18:18 +0200, Tobias Kremer wrote: >> > Just out of pure curiosity: Why is it that there are dedicated >> > "flash:" entries in the storage for the flash? Wouldn't the >> > session be enough? >> >> The "flash:" rows were used for compatibility with Store::DBI. We can >> break compatibility if people find the it not very useful. > > We definitely should! IMHO five queries per request to the database just for > the > session and flash stuff is inacceptable. If your app is really slowed by the number of queries to load session/flash, use this: http://git.jrock.us/?p=Catalyst-Plugin-Session-HMAC.git;a=blob;f=lib/Catalyst/Plugin/Session/HMAC.pm;hb=HEAD Regards, Jonathan Rockway -- print just => another => perl => hacker => if $,=$" ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On Wed, Aug 27, 2008 at 10:41:11AM +0200, Tobias Kremer wrote: > Quoting Moritz Onken <[EMAIL PROTECTED]>: > > Am 27.08.2008 um 10:19 schrieb Tobias Kremer: > > > Ok, a second glance (after the first coffee) revealed that the > > > separation is indeed there :) The question is, why? > > > > Just guessing: > > not every request has its own session object. There are users with no > > session attached to them. > > That was my first guess, too. But AFAICT each and every user receives a > session > on its first visit - logged-in or not. Only if you write to $c->session. But if you write to $c->flash, then you have to create a cookie, so I guess you're effectively creating a session anyway. So as long as you have to write to one of the two to create a session, I don't really see a problem. -- Matt S Trout Need help with your Catalyst or DBIx::Class project? Technical Directorhttp://www.shadowcat.co.uk/catalyst/ Shadowcat Systems Ltd. Want a managed development or deployment platform? http://chainsawblues.vox.com/http://www.shadowcat.co.uk/servers/ ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Quoting Moritz Onken <[EMAIL PROTECTED]>: > Am 27.08.2008 um 10:19 schrieb Tobias Kremer: > > Ok, a second glance (after the first coffee) revealed that the > > separation is indeed there :) The question is, why? > > Just guessing: > not every request has its own session object. There are users with no > session attached to them. That was my first guess, too. But AFAICT each and every user receives a session on its first visit - logged-in or not. Can we confirm this? --Tobias ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Am 27.08.2008 um 10:19 schrieb Tobias Kremer: Quoting Tobias Kremer <[EMAIL PROTECTED]>: Quoting Daniel Westermann-Clark <[EMAIL PROTECTED]>: The "flash:" rows were used for compatibility with Store::DBI. We can break compatibility if people find the it not very useful. I have to admit that I don't understand what compatibility with Store::DBI you're talking about. AFAICT the Store::DBI source doesn't separate between "session" and "flash" (does it support the flash at all?). Maybe you (or Andy) can shed some light on this! Ok, a second glance (after the first coffee) revealed that the separation is indeed there :) The question is, why? Just guessing: not every request has its own session object. There are users with no session attached to them. ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Quoting Tobias Kremer <[EMAIL PROTECTED]>: > Quoting Daniel Westermann-Clark <[EMAIL PROTECTED]>: > > The "flash:" rows were used for compatibility with Store::DBI. We can > > break compatibility if people find the it not very useful. > I have to admit that I don't understand what compatibility with Store::DBI > you're talking about. AFAICT the Store::DBI source doesn't separate between > "session" and "flash" (does it support the flash at all?). Maybe you (or > Andy) can shed some light on this! Ok, a second glance (after the first coffee) revealed that the separation is indeed there :) The question is, why? --Tobias ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Quoting Daniel Westermann-Clark <[EMAIL PROTECTED]>:
> On 2008-08-26 09:47:59 +0200, Tobias Kremer wrote:
> > Please note, that this is ONLY happening with the flash part - my
> > sessions work 100% accurate all the time!
> How are you interacting with the flash vs. your sessions? Could you
> provide some code illustrating the problem? A test would be even
> better.
Hmm .. Nothing special there. The session stuff works just by use'ing the
neccessary plugins (no additional tinkering is done) and the flash stuff is
used like this:
a) In my controllers:
push @{$c->flash->{'messages'}}, "You have a message!";
b) In my wrapper template:
[% flash_messages = Catalyst.flash.messages %]
[% IF flash_messages %]
[% FOR message IN flash_messages %]
[% message %]
[% END %]
[% END %]
It's hard to come up with a test-case for this because it only happens in
moderate to high load situations.
--Tobias
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Quoting Daniel Westermann-Clark <[EMAIL PROTECTED]>: > On 2008-08-26 14:18:18 +0200, Tobias Kremer wrote: > > Just out of pure curiosity: Why is it that there are dedicated > > "flash:" entries in the storage for the flash? Wouldn't the > > session be enough? > > The "flash:" rows were used for compatibility with Store::DBI. We can > break compatibility if people find the it not very useful. We definitely should! IMHO five queries per request to the database just for the session and flash stuff is inacceptable. The alternative approach could very well get rid of the race-condition, too. I think it's really worth it. I have to admit that I don't understand what compatibility with Store::DBI you're talking about. AFAICT the Store::DBI source doesn't separate between "session" and "flash" (does it support the flash at all?). Maybe you (or Andy) can shed some light on this! --Tobias ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On 2008-08-26 09:47:59 +0200, Tobias Kremer wrote: > a) Patch Catalyst::Plugin::Session::Store::DBIC to wrap the flash >functionality in a transaction (of course, this must be configurable). >Advantages: > - Easy to implement > - Most sensible solution. >Disadvantages: > - Slight performance overhead? > - Doesn't solve the problem for non-transactional databases I would be happy to apply a patch that implemented this, at least until something is done at the DBIx::Class level. Non-transactional databases will continue to have the problem, and in my opinion it isn't the job of the session store to fix that problem. The session store docs can discourage use of MyISAM, for example. > Please note, that this is ONLY happening with the flash part - my > sessions work 100% accurate all the time! How are you interacting with the flash vs. your sessions? Could you provide some code illustrating the problem? A test would be even better. -- Daniel Westermann-Clark ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On 2008-08-26 14:18:18 +0200, Tobias Kremer wrote: > Just out of pure curiosity: Why is it that there are dedicated > "flash:" entries in the storage for the flash? Wouldn't the > session be enough? The "flash:" rows were used for compatibility with Store::DBI. We can break compatibility if people find the it not very useful. -- Daniel Westermann-Clark ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Quoting Tobias Kremer <[EMAIL PROTECTED]>:
> just wanted to inform you that switching from MyISAM to InnoDB for the
> session table does NOT solve the "duplicate entry" problem when
> using flash() :(
Just out of pure curiosity: Why is it that there are dedicated "flash:"
entries in the storage for the flash? Wouldn't the session be enough? I mean a
session is always established on the first request (logged-in or not), why not
just use it for the flash data (actually that's how other *cough* PHP
frameworks are doing it)? Something like this in my MyApp.pm seems to work
perfectly:
sub flash_msg {
my( $c, $value ) = @_;
if( $value ) {
$c->session->{ '__flash__' } = $value;
} else {
return delete $c->session->{ '__flash__' };
}
}
This should solve the race-condition and will reduce the queries per request
from FIVE(!) when using the flash mechanism down to 2.
What am I missing? :)
--Tobias
___
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Re: [Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
On Tue, Aug 26, 2008 at 09:47:59AM +0200, Tobias Kremer wrote: [...] > b) Patch DBIx::Class's find_or_create() method and reverse its order >(insert first, check for duplicate, then select). >Disadvantages: > - Not that easy to implement because every database returns >a different error to signal a unique key constraint violation. > - It's quite possible that this doesn't really solve the problem >but just changes the error. If the row is deleted between insert >and select (this race condition still exists), find_or_create() >will return nothing and AFAICT C::P::Session::Store::DBIC's >flash() method expects a working row object. Also, it'll cancel any transaction in progress. However, that can be fixed by using SAVEPOINT instead. SAVEPOINT; INSERT ...; ROLLBACK TO SAVEPOINT; SELECT/UPDATE ...; seems to be the recommended approach for PostgreSQL. PostgreSQL supports SAVEPOINT as of 8.0; you're SOL with 7.4 and earlier. (I was in fact considering concocting and submitting a patch as I'm being bitten by this race condition too.) I can't speak for whether MySQL's advertised SAVEPOINT support works, or even does anything other than acknowledge that the keyword exists. >Advantages: > - find_or_create() will be faster everywhere in many cases. There would be the overhead of a transaction, but it's a worthwhile cost in a non-toy application. ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
[Catalyst] Duplicate entries with C::P::Session::Store::DBIC and MySQL - new findings
Hey guys, just wanted to inform you that switching from MyISAM to InnoDB for the session table does NOT solve the "duplicate entry" problem when using flash() :( I really can't believe that me and 3 or 4 others are the only people experiencing this problem! We're getting close to 400 of those errors over a weekend with moderate traffic (2-3 million page impressions). The problem with this error is that the user receives a 500 error page when in reality nothing really bad happened (it's just a flash message that couldn't get inserted, you know). Now AFAICT we have these solutions: a) Patch Catalyst::Plugin::Session::Store::DBIC to wrap the flash functionality in a transaction (of course, this must be configurable). Advantages: - Easy to implement - Most sensible solution. Disadvantages: - Slight performance overhead? - Doesn't solve the problem for non-transactional databases b) Patch DBIx::Class's find_or_create() method and reverse its order (insert first, check for duplicate, then select). Disadvantages: - Not that easy to implement because every database returns a different error to signal a unique key constraint violation. - It's quite possible that this doesn't really solve the problem but just changes the error. If the row is deleted between insert and select (this race condition still exists), find_or_create() will return nothing and AFAICT C::P::Session::Store::DBIC's flash() method expects a working row object. Advantages: - find_or_create() will be faster everywhere in many cases. Anything else? What do you propose? There's a hint in the POD about a "soon to be developed locking solution" which should get rid of this race condition. Please note, that this is ONLY happening with the flash part - my sessions work 100% accurate all the time! We really should sort this thing out because it's bugging me for ages. Again, I can't believe that I'm almost the only person with this problem. If we are unable to fix it, I'll remove the flash() stuff from my code and tell everyone that it's b0rken ;-) --Tobias ___ List: Catalyst@lists.scsys.co.uk Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/ Dev site: http://dev.catalyst.perl.org/
