Nicolas Lehuen wrote:
> Hi Earle,
> 
> Thanks for your input.
> 
> For the sake of performance, you way want to call a single "DELETE FROM
> session WHERE accessed < ?" request instead of fetching all the session id
> and access time from the database and calling a delete per session. See for
> example this code :
> 
> http://svn.apache.org/viewvc/httpd/mod_python/branches/nlehuen/sandbox/SQLiteSession.py?view=markup

You definitely want to do this for performance reasons, but also to
avoid the race condition in your current code.

Consider 2 processes (or threads), A and B.

'A' is handling a request for session '123', with a session timeout of
100 seconds. Session 123 was last saved 99 seconds ago.

The session cleanup is being run in 'B'.

1. At 99.9 seconds, 'A' loads its session data which is still valid.
2. Clock increments to 100.
3. Context switch
4. 'B' runs and and collects the expired session ids, which includes 123
since it was last saved 100 seconds ago.
5. Context switch
6. 'A' runs, and saves its session data.
7. Context switch
8. 'B' runs and deletes session 123 which is in the expired list, even
though the current value in the table for time.time() - accessed < 100.
As result session 123's data is lost even though it is still valid.


Performance wise it's a good idea to optimize the session cleanup any
way you can. I did some benchmarking when I was working on FileSession,
and the session cleanup turned out to be a real bottleneck for
DbmSession and the initial version of FileSession when you get into a
large number of sessions.

The session cleanup will run at random intervals, and it is possible
that do_cleanup will be called simultaneously in more than one thread.
If you allow this and your cleanup code is slow it will really drag down
your server. If the first cleanup is slow there is a good chance that a
second cleanup will start. This adds additional load to the server, and
slows both of the cleanup threads. It will now take longer for these 2
cleanups to complete, increasing the chance that a 3rd will start in the
 interim. Under these conditions with a large number of sessions using
DbmSession, the server response dropped pretty quickly from 400 requests
per second to 20 requests per second. It ends up being a pretty good
self-DOS attack.

In theory your MySQL session cleanup can be much more efficient than
DbmSession (which needs to unpickle *every* session in the dbm file) but
a executing a separate query for each expired session is just not the
way to go. I don't know how busy your site will be, but just imagine
that you need to delete 1000 sessions in 1 go. Making 1000 calls to
MySQL just can't be a good thing. ;)

For additional performance you might also want to consider stuffing your
connection instance in the session instance. I would think the most
common use case would be to have load and save paired in most requests,
so opening and closing the connection in both do_load and do_save seems
like unnecessary overhead. So do something like:

    def __init__(req):
        self.conn = None
        ...

    def do_load(self):
        if self.conn is None:
           self.conn = self.db_connect()
        ...

    def do_save(self):
        if self.conn is None:
           self.conn = self.db_connect()
        ...

    def do_delete(self):
        if self.conn is None:
           self.conn = self.db_connect()
        ...


    def db_connect(self):
        conn = MySQLdb.connect(... )
        self._req.register_cleanup(close_connection, conn)
        return conn


def close_connection(conn):
    conn.close()


You also need to check mysqldb_cleanup, as it will not actually run as
written. You are referencing self.* attributes in that function, but
self is not defined and will raise an exception. You are catching *all*
exceptions with "except: pass", so there will never be any notice that
the cleanup failed.

Jim

> 
> I think MySQL has also the support for INSERT OR UPDATE requests which are
> quite handy when implementing do_save.
> 
> Regards,
> Nicolas
> 
> 2006/8/9, Earle Ady <[EMAIL PROTECTED]>:
>>
>> Hello everyone!
>>
>> The topic of a MySQL session implementation has come up here several
>> times in the past.   We have recently written one in-house and have
>> been running it on a production infrastructure for some time without
>> incident.  As it seems a fairly common component which people can
>> benefit from, it's being provided below for consideration.
>>
>> Currently this was built directly into Session.py and takes
>> configuration options through httpd.conf.  Clearly this could be
>> something which may need to be completely segmented from mod_python
>> given the reliance on MySQL being installed. Hopefully this can get
>> the conversation going.
>>
>> Please note that no connection pooling has been implemented in this
>> version, and the benefits of doing this may or may not prove valuable
>> depending on the scope of its use.  For our clustered session
>> environment it was unnecessary.
>>
>> httpd.conf example:
>>         PythonOption session                             MySQLSession
>>         PythonOption session.db                       sessions
>>         PythonOption session.db_host             db.whatever.com
>>         PythonOption session.db_user             user
>>         PythonOption session.db_password   pass
>>         PythonOption session.timeout               3600
>>
>> Mahalo,
>> earle.
>>
>> 33d32
>> < import MySQLdb
>> 704,829d702
>> <
>> <
>> ########################################################################
>> ###
>> < ## MySQLSession
>> <
>> < class MySQLSession(BaseSession):
>> <
>> <     def __init__(self, req, sid=0, secret=None, timeout=0, lock=1):
>> <
>> <         self.db = None
>> <         self.db_host = None
>> <         self.db_user = None
>> <         self.db_password = None
>> <         self.timeout = timeout
>> <         self.req = req
>> <
>> <         opts = req.get_options()
>> <
>> <         # parse httpd.conf for DB information
>> <         for opt in [ 'db', 'db_host', 'db_user', 'db_password' ]:
>> <             if opts.has_key('.'.join([ 'session', opt ])):
>> <                 setattr(self, opt, opts['.'.join([ 'session', opt ])])
>> <             else:
>> <                 self.req.log_error("MySQLSession: missing
>> PythonOption: session.%s" % (opt, ),
>> <                             apache.APLOG_NOTICE)
>> <
>> <                 raise apache.SERVER_RETURN,
>> apache.HTTP_INTERNAL_SERVER_ERROR
>> <
>> <         if opts.has_key('session.timeout'):
>> <             self.timeout = opts['session.timeout']
>> <
>> <         BaseSession.__init__(self, req, sid=sid, secret=secret,
>> <                              timeout=timeout, lock=lock)
>> <
>> <     def do_cleanup(self):
>> <
>> <         data = [self._req.server, ]
>> <
>> <         self._req.register_cleanup(mysqldb_cleanup, data)
>> <         self._req.log_error("MySQLSession: registered database
>> cleanup.",
>> <                             apache.APLOG_NOTICE)
>> <
>> <     def do_load(self):
>> <         self.conn = MySQLdb.connect(host = self.db_host, user =
>> self.db_user,
>> <                                     passwd = self.db_password, db =
>> self.db, use_unicode=1,
>> <                                     init_command="set names utf8")
>> <
>> <         self.cursor = self.conn.cursor(MySQLdb.cursors.DictCursor)
>> <
>> <         try:
>> <             self.cursor.execute("SELECT * FROM session WHERE id=%
>> s" , self._sid)
>> <             res = self.cursor.fetchone()
>> <
>> <             if res is not None and res.has_key('data'):
>> <                 retVal = cPickle.loads(res['data'].tostring())
>> <             else:
>> <                 retVal = None
>> <         except:
>> <             retVal = None
>> <
>> <         self.cursor.close()
>> <         self.conn.close()
>> <
>> <         return retVal
>> <
>> <     def do_save(self, dict):
>> <         self.conn = MySQLdb.connect(host = self.db_host, user =
>> self.db_user,
>> <                                     passwd = self.db_password, db =
>> self.db, use_unicode=1,
>> <                                     init_command="set names utf8")
>> <
>> <         self.cursor = self.conn.cursor(MySQLdb.cursors.DictCursor)
>> <         self.cursor.execute("SELECT * from session where id=%s" ,
>> self._sid)
>> <
>> <         res = self.cursor.fetchone()
>> <
>> <         if res is None:
>> <             self.cursor.execute("INSERT INTO session (id, data,
>> created, accessed, timeout) VALUES (%s, %s, NOW(), NOW(), %s)",
>> (self._sid, cPickle.dumps(dict), self._timeout, ))
>> <         else:
>> <             self.cursor.execute("UPDATE session SET data=%s,
>> accessed=NOW() WHERE id=%s" , (cPickle.dumps(dict), self._sid, ))
>> <
>> <
>> <         self.cursor.close()
>> <         self.conn.close()
>> <
>> <
>> <     def do_delete(self):
>> <         self.conn = MySQLdb.connect(host = self.db_host, user =
>> self.db_user,
>> <                                     passwd = self.db_password, db =
>> self.db, use_unicode=1,
>> <                                     init_command="set names utf8")
>> <
>> <         self.cursor = self.conn.cursor(MySQLdb.cursors.DictCursor)
>> <
>> <         self.cursor.execute("DELETE FROM session WHERE id=%s",
>> (self._sid,))
>> <
>> <         self.cursor.close()
>> <         self.conn.close()
>> <
>> <
>> < def mysqldb_cleanup(data):
>> <     try:
>> <         conn = MySQLdb.connect(host = self.db_host, user =
>> self.db_user,
>> <                            passwd = self.db_password, db = self.db,
>> use_unicode=1,
>> <                            init_command="set names utf8")
>> <
>> <         cursor = conn.cursor(MySQLdb.cursors.DictCursor)
>> <         server = data
>> <
>> <         expired = []
>> <
>> <         sessions = cursor.execute("SELECT *, UNIXTIME(accessed) AS
>> uaccessed, UNIXTIME(created) AS ucreated FROM session")
>> <
>> <         for session in sessions:
>> <             if ((time.time() - session['accessed']) > session
>> ['timeout']):
>> <                 expired.append(session)
>> <
>> <         for session in expired:
>> <             cursor.execute("DELETE FROM session WHERE id=%s",
>> (session['id'],))
>> <
>> <     except:
>> <         pass
>> <
>> <     try:
>> <         cursor.close()
>> <         conn.cose()
>> <     except:
>> <         pass
>> <
>> <
>> 856,857d728
>> <     elif sess_type == 'MySQLSession':
>> <        sess = MySQLSession
>>
>>
>>
>>
>>
>>
> 

Reply via email to