Database exception using DbSession

[puku]

What steps will reproduce the problem?

When I'm trying to login on the website, from time to time I'm receiving an error message.
After reloading of the page everything is ok.
YII_DEBUG is enabled, YII_ENV_DEV is set to true.

Maybe this issue is linked with this one

Stackoverflow question

What do you get instead?

Integrity constraint violation – yii\db\IntegrityException

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '0' for key 'PRIMARY'
The SQL being executed was: UPDATE session SET id=0 WHERE id='3u5r7gq6pqh8s0gjrusfqpmni7'
Error Info: Array
(
[0] => 23000
[1] => 1062
[2] => Duplicate entry '0' for key 'PRIMARY'
)
↵
Caused by: PDOException

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '0' for key 'PRIMARY'

in /var/www/develop/vendor/yiisoft/yii2/db/Command.php at line 844

Additional info

Q	A
Yii version	2.0.10
PHP version	7.0
Operating system	Ubuntu 14.04

[bizley]

I think DbSession should check if $newID is properly set before trying to update DB with it.

[dynasource]

looks like it. If session_regenerate_id fails to generate an ID, its no use to update the DB with it.

[bizley]

Should the old session row be deleted in case the new ID can not be generated? For something like to prevent loosing the old data in race condition case?

I've tested DbSession simulating loosing ID and deleting session row in that case - everything looks ok.

[yanhuixie]

Same issue with Redis session implementation.
session_regenerate_id(): Session object destruction failed. ID: user (path: )
/vendor/yiisoft/yii2/web/Session.php L282
Yii2 2.0.10

[andrewnester]

@bizley I guess there is no need to delete session row. session_regenerate_id could fail, for instance, because of network issues and when will be ran second time it will return valid ID.

[andrewnester]

@yanhuixie it's a bit different, DbSession::regenerateID doesn't ask session_regenerate_id for session destruction

[bizley]

@andrewnester can you confirm that old row is deleted or updated later on? Because if not there is the risk of DB storage overfill.

[andrewnester]

@bizley thanks for reasonable question! In this case DbSession:: gcSession will handle all expired data

[puku]

I still have issue with it. Here is what I have in log:

Next yii\db\IntegrityException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '1' for key 'PRIMARY'
The SQL being executed was: INSERT INTO session (data, id, expire) VALUES ('', 1, 1499947188) in /var/www/release/vendor/yiisoft/yii2/db/Schema.php:636

Next yii\db\IntegrityException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '5' for key 'PRIMARY'
The SQL being executed was: UPDATE session SET id=5 WHERE id='1v0dqepld1pk9eqn3rp0ckpsfc' in /var/www/release/vendor/yiisoft/yii2/db/Schema.php:636

One of the solution is to check if there is any duplicates in the database, before inserting or updating db. But I'm not sure this solution is the correct ones.

[ddinchev]

I would like to reopen this issue as I now (after 2.0.13 upgrade) get a lot of errors like this. I've proposed a PR.

[sergeymakinen]

ATM the only proper way to fix it (IMO) is #13879.

[rob006]

We can catch exception and retry?

[samdark]

Yes, we can but it's like a hotfix. Proper solution is #13879

[rob006]

Implementation of upsert looks really complicated to me, it probably will be source of new bugs and may make situation even worse. I prefer use this hotfix in patch release and switch to upsert in 2.0.14.

[samdark]

Makes sense. Let's do try-catch for 2.0.13.1 and replace implementation with upsert when it's ready. Would you help with a pull request?

[sergeymakinen]

Yes, a hotfix in 2.0.13.1 sounds reasonable.
For example it took a whole day to make PostgreSQL (pre 9.5, since 9.5 it's as easy as ABC) QB produce this (it's correct and handles lots of DBMS' specifics):

WITH
        "EXCLUDED" ("email", "name", "address", "bool_status", "profile_id") AS (VALUES (:qp0, :qp1, :qp2, :qp3, CAST(NULL AS int4))),
        "upsert" AS (UPDATE "customer" SET "email"="EXCLUDED"."email", "name"="EXCLUDED"."name", "address"="EXCLUDED"."address", "bool_status"="EXCLUDED"."bool_status", "profile_id"="EXCLUDED"."profile_id" FROM "EXCLUDED" WHERE (("customer"."email"="EXCLUDED"."email")) RETURNING "customer".*)
INSERT INTO "customer" ("email", "name", "address", "bool_status", "profile_id") SELECT "email", "name", "address", "bool_status", "profile_id" FROM "EXCLUDED" WHERE NOT EXISTS (SELECT 1 FROM "upsert" WHERE (("upsert"."email"="EXCLUDED"."email")))

So it will definitely require testing (in addition to unit tests).

[samdark]

Wow, ↑ isn't how it is usually done.

[ddinchev]

I thought about resubmitting an MR with try/catch and ignore if there is a duplicate key. There is no need to retry when we've hit the race condition as session data has just been saved.

However, I'm not sure how to verify it's a duplicate key in DBMS-independent way. For MySQL the PDO error code is 1062 but I couldn't find the ones for MSSQL, Postgres and SQLite.

[samdark]

@sergeymakinen https://github.com/symfony/symfony/pull/10652/files may give you some hints (related problem).

[sergeymakinen]

@samdark It was 04 or 05 am and I was not clear. :) The code was for a generic upsert implementation where REPLACE is not a replacement.

[sergeymakinen]

@samdark While the sample code is pretty logical when working with sessions. But did they miss pgsql? I would wonder why. 👹

[samdark]

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php — here's latest version.

[samdark]

Tests are quite interesting as well: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php

[sergeymakinen]

This means that the hardest part (pgsql <9.5) is done. Other implementations are trivial. MySQL’s and MSSQL’s upserts are also completed.

[cebe]

Duplicate entry '1' for key 'PRIMARY'

your sessionid is 1?

[samdark]

Oh. Original exception is even "better". ID is 0

o_0

[ddinchev]

@samdark if there is no progress on this, I can try to pick it and create a PR based on Symphony's solution. So that we can get it out in 2.0.13.2 hopefully?

[samdark]

@sergeymakinen is trying hard to finish upserts support.

[puku]

Sorry, for confusions, I've found out that my problem was linked with incorrect type of Mysql field id in the table session.

[samdark]

@puku thanks for letting us know. We still want upserts for sessions though so I'm leaving it open for now.

[samdark]

See #11401