Implemented the session->write() DAO method to support an upsert on MySQL and a two-phase insert/update on Pgsql.#1
Conversation
Thanks for the report and PR. Can you paste the error messages you got, as well as steps to reproduce the issue? we will need to reproduce before we can merge the pr. Thanks! |
|
Hey, Matthieu. Apologies, this pull request is specific to @sri-soham's Postgres fork. @sri-soham / @mattab, let me know if you'd still like to see error messages and steps to reproduce. -Jon. |
|
@Jon-IB Ah sorry! Didn't notice the PR was on @sri-soham fork 👍 good to hear you're helping Sri test the fork. Keep it up! |
|
@Jon-IB Yes, error messages and steps to reproduce will be helpful. If I am not wrong, error will be about 'on duplicate key update' not being supported in postgresql. Will it be better if we enclose the 'lookup and insert-or-update' in a transaction or will that be an unnecessary overhead? |
|
OK, I'll compile the details. We could wrap this in a transaction, but that might be overkill. I'll take a closer look at the code to see what sort of transaction isolation would be necessary here. I don't think dbtable-backed sessions work at all right now on Postgres without this change. So risking a (rare) concurrency issue might be better than nothing. We're running this code in production with about 2000 tracked sites, and we haven't seen any such error yet. Even so, I want this fix to be correct, and not merely "workable". |
|
Yes, dbtable-backed sessions would work in postgresql only with the change that you have made. As I understand it, session ids are generated by php. So, not sure if this should be handled at php level or in piwik. Here is a lengthy discussion on uniqueness of session id on stackoverflow, http://stackoverflow.com/questions/138670/how-unique-is-the-php-session-id |
4 participants
We saw some errors with the code when running on NFS, since Piwik uses database-backed sessions in that scenario. We determined the problem was due to missing DAO support for writing session data.
We could have tried the insert on Pgsql and trapped the unique constraint violation. But in this case querying for the session first might be more efficient, since it's a simple primary key lookup, and exceptions are expensive. Granted, this approach isn't thread-safe, but that shouldn't be a major issue.