[5.2] SessionDatabaseHandler confirm existence before committing

[deanmraz]

To resolve this issue:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'af5d46ed1a5399b59f186fa59c98d734a04cc5be' for key 'sessions_id_unique'

[GrahamCampbell]

Please squash to one commit. ;)

[taylorotwell]

Can we get a description on how this solves the issue?

[deanmraz]

@GrahamCampbell squashed

@taylorotwell This indirectly solves the issue by forcing a read check before database write. Yes, there may be a chance we do an additional select query, however the 2nd query will ensure the session does not already exist in the database.

The reason why this is necessary is because of database replication. I believe one of the replicas may not be completely up to date thus when reading at the beginning of the script $this->exist still equals false and then trying to write with a database insert at the end it throws the session_unique_id integrity error.

Another scenario could be ajax requests. Few requests get sent out and hit the database roughly at the same time. 1st one does the insert but the 2nd request has already done the database read, but does not exist at that time.

Its hard to say where the issue stems from because I only see it in production. I've seen it more often the more clients I add to the database so I definitely think its due to load and lots of concurrent requests.

I agree this isn't the complete solution, however it does ensure one last check before it tries to insert when it should update.

[deanmraz]

Maybe better solution would be to let the database figure it out with insert on duplicate or upsert depending on database type.

https://dev.mysql.com/doc/refman/5.6/en/insert-on-duplicate.html

https://wiki.postgresql.org/wiki/UPSERT

[taylorotwell]

@deanmraz so did this fix solve your issues in production?

[deanmraz]

@taylorotwell going good so far, I will report back Monday.

[deanmraz]

@taylorotwell so... its not the silver bullet, but its definitely happening a lot less. We are down to a few times a week versus a few times a day.

I believe the issue stems from concurrent requests via ajax calls. These requests both read that the session id does not exist, and when the requests try to insert one would fail. This solution makes it happen a lot less because it narrows the window and does another existence check right before it tries to insert.

I am going to attempt using MySQL's INSERT ON DUPLICATE and will let you know how it goes.

[ybr-nx]

@deanmraz

I just lunched site with about 5 000 000 page views per month. My fast fix was something like that (using 5.1).

if ($this->connection->getDriverName() == 'mysql') {
    $this->connection->insert('INSERT INTO ' . $this->table . ' (id, payload, last_activity) values (?, ?, ?) ON DUPLICATE KEY UPDATE payload = ?, last_activity = ?', [
        $sessionId, 
        base64_encode($data), 
        time(),
        base64_encode($data), 
        time(),
    ]);
}
else {
    $this->getQuery()->insert([
        'id' => $sessionId, 'payload' => base64_encode($data), 'last_activity' => time(),
    ]);    
}

Works like a charm.

Need to be rewritten to fit in Laravel

[jdavidbakr]

@ybr-nx - where did you put that code? Is it overwriting the base model class or did you do it elsewhere? I'm running into this issue but also running into an issue where I have a multi-key insert that I need to do the same thing to ( $model->insert[...lots of rows...]) ) and am trying to come up with the best place to override it. I'm thinking just replacing the insert() call (or making a 'replace()' call or something like that) in a trait that I can apply to the model, but am curious where you put the code to override this.

[ybr-nx]

@jdavidbakr - I edited DatabaseSessionHandler.php code under vendor as I needed some really fast fix. Still haven't figured out how/where to rewrite it.

    /**
     * {@inheritdoc}
     */
    public function write($sessionId, $data)
    {   

        if ($this->exists) {
            $this->getQuery()->where('id', $sessionId)->update([
                'payload' => base64_encode($data), 'last_activity' => time(),
            ]);
        } else {

                  //...added that code here

        }

        $this->exists = true;
    }

[jdavidbakr]

@ybr-nx - I think I found a work-around that works and doesn't break with a composer update:

1. In AppServiceProvider in boot, I registered an extension of the Session class:

use App\Services\DatabaseSessionHandler;

...

        Session::extend('extended-database', function($app) {
            return new DatabaseSessionHandler(\DB::connection('master'), 'sessions');
        });

Then, I created a session handler class that extends the default database session handler:

<?php 
namespace App\Services;

use Illuminate\Session\DatabaseSessionHandler as LaravelDatabaseSessionHandler;
use Illuminate\Database\ConnectionInterface;

class DatabaseSessionHandler extends LaravelDatabaseSessionHandler {

    public function write($sessionId, $data)
    {
        try {
            parent::write($sessionId, $data);
        } catch(\Illuminate\Database\QueryException $e) {
            if($e->getCode() != 23000) {
                // Only report an error if this is not a duplicate key error
                abort(500, "Query Exception: ".$e->getMessage().': '.$e->getSql());
            }
        }
    }
}

In my case I'm ignoring the duplicate key error but alternatively you could re-save the session if the code is 23000.

I pushed this live last Friday and I haven't had the duplicate key error since then.

[congkhuong]

Is there any chance to update 4.2 version for this bug?