[Kimchi-devel] [PATCH] issue #325: Use RamSession instead of FileSession

[Aline Manera]

On 02/25/2014 10:42 AM, Sheldon wrote:
> Not sure RamSession is a good choice.
>
> What's the different between MemcachedSession and RamSession.
>
> Have check the cherrpy doc, does not tell the details.
>
> http://docs.cherrypy.org/en/latest/refman/lib/sessions.html?highlight=sessions.init#cherrypy.lib.sessions.init
> cherrypy.lib.sessions.init(/storage_type='ram'/, /path=None/, 
> /path_header=None/, /name='session_id'/, /timeout=60/, /domain=None/, 
> /secure=False/, /clean_freq=5/, /persistent=True/, /httponly=False/, 
> /debug=False/, /**kwargs/)
>
>     Initialize session object (using cookies).
>
>     storage_type
>         One of 'ram', 'file', 'postgresql', 'memcached'. This will be
>         used to look up the corresponding class in
>         cherrypy.lib.sessions globals. For example, 'file' will use
>         the FileSession class.
>
>

I am not sure the different between those 2 sessions. I will try to 
figure out

>
> On 02/25/2014 11:54 AM, Aline Manera wrote:
>> From: Aline Manera<alinefm at br.ibm.com>
>>
>> There is a readers/writes problem in cherry.sessions.FileSession
>> implementation which makes it non thread safe.
>>
>> >Fromhttps://groups.google.com/forum/#!topic/cherrypy-users/biitlom41T8
>> "When the session tool is turned on it hooks the lib.session.save
>> function onto 'before_finalize' so that the session data is always
>> re-saved at the end of each request.  For RamSession this is fine
>> because save just overwrites a dictionary value.  FileSession uses
>> pickle.dump to write the session data to file and uses pickle.load to
>> load it. If two requests overlap in just the wrong way then it's
>> possible that pickle.load gets called halfway through the pickle.dump
>> which throws an EOFError. More concurrent requests will increase the
>> chance of this happening."
>>
>> On Host tab, more concurrent requests happen, specially while creating a
>> debug report: /host/stats and /tasks/<id>. Because that the problem was
>> identified there.
>>
>> To solve this issue, there are 2 options:
>> 1) overwrite cherrypy.session.FileSession._load to fix the
>> readers/writers and make it thread safe;
>> or
>> 2) Use RamSession instead of FileSession
>>
>> As using RamSession makes the application faster because it will not need to
>> read/write file for each request, this is the best solution in this case.
>>
>> This patch also uses acquire_lock() and release_lock() for each
>> read/write session data occurrence.
>>
>> Signed-off-by: Aline Manera<alinefm at br.ibm.com>
>> ---
>>   src/kimchi/auth.py      |   22 +++++++++++++---------
>>   src/kimchi/config.py.in |    4 ----
>>   src/kimchi/server.py    |    4 +---
>>   3 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
>> index 94e7eab..fda4e1f 100644
>> --- a/src/kimchi/auth.py
>> +++ b/src/kimchi/auth.py
>> @@ -131,13 +131,13 @@ def check_auth_session():
>>       A user is considered authenticated if we have an established session open
>>       for the user.
>>       """
>> -    try:
>> -        if cherrypy.session[USER_ID]:
>> -            debug("Session authenticated for user %s" %
>> -                  cherrypy.session[USER_ID])
>> -            return True
>> -    except KeyError:
>> -        pass
>> +    cherrypy.session.acquire_lock()
>> +    session = cherrypy.session.get(USER_ID, None)
>> +    cherrypy.session.release_lock()
>> +    if session is not None:
>> +        debug("Session authenticated for user %s" % session)
>> +        return True
>> +
>>       debug("Session not found")
>>       return False
>>
>> @@ -188,11 +188,15 @@ def logout():
>>       cherrypy.lib.sessions.expire()
>>
>>
>> +
>>   def has_permission(admin_methods):
>> +    cherrypy.session.acquire_lock()
>> +    session = cherrypy.session.get(USER_ID, None)
>> +    cherrypy.session.release_lock()
>> +
>>       return not admin_methods or \
>>           cherrypy.request.method not in admin_methods or \
>> -        (cherrypy.request.method in admin_methods and
>> -            cherrypy.session[USER_SUDO])
>> +        (cherrypy.request.method in admin_methods and session)
>>
>>
>>   def kimchiauth(admin_methods=None):
>> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
>> index 32d61c6..92d5b91 100644
>> --- a/src/kimchi/config.py.in
>> +++ b/src/kimchi/config.py.in
>> @@ -36,10 +36,6 @@ from kimchi.xmlutils import xpath_get_text
>>   DEFAULT_LOG_LEVEL = "debug"
>>
>>
>> -def get_session_path():
>> -    return os.path.join(paths.state_dir, 'sessions')
>> -
>> -
>>   def get_object_store():
>>       return os.path.join(paths.state_dir, 'objectstore')
>>
>> diff --git a/src/kimchi/server.py b/src/kimchi/server.py
>> index 6dd0404..ef8e701 100644
>> --- a/src/kimchi/server.py
>> +++ b/src/kimchi/server.py
>> @@ -73,8 +73,7 @@ class Server(object):
>>                 'tools.sessions.name': 'kimchi',
>>                 'tools.sessions.httponly': True,
>>                 'tools.sessions.locking': 'explicit',
>> -              'tools.sessions.storage_type': 'file',
>> -              'tools.sessions.storage_path': config.get_session_path(),
>> +              'tools.sessions.storage_type': 'ram',
>>                 'tools.kimchiauth.on': False},
>>           '/css': {
>>               'tools.staticdir.on': True,
>> @@ -136,7 +135,6 @@ class Server(object):
>>               os.path.dirname(os.path.abspath(options.error_log)),
>>               os.path.dirname(os.path.abspath(config.get_object_store())),
>>               os.path.abspath(config.get_screenshot_path()),
>> -            os.path.abspath(config.get_session_path()),
>>               os.path.abspath(config.get_debugreports_path()),
>>               os.path.abspath(config.get_distros_store())
>>           ]
>
>
> -- 
> Thanks and best regards!
>
> Sheldon Feng(???)<shaohef at linux.vnet.ibm.com>
> IBM Linux Technology Center

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140225/2baa284c/attachment.html>