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 >> >> >> >> >> >> >