Postgres Pro
Downloads
Home   >   mailing lists

Thread: [pgAdmin4][Patch]: RM #3674 Old session files are not cleared in.pgAdmin session and keep piling up

[pgAdmin4][Patch]: RM #3674 Old session files are not cleared in.pgAdmin session and keep piling up

From
Akshay Joshi
Date:
Hi Hackers

Attached is the patch to fix the RM #3674 "Old session files are not cleared in .pgAdmin session and keep piling up". Patch contains following changes:
  • With current implementation we have override get_expiration_time() method of flask and return the expiry time as "datetime.datetime.now() + 1 day (hard coded)". Also the time used as now() instead of utcnow() which is there in base class method of flask.
    • Solution:  We should use session.permanent flag and set the permanent_session_lifetime variable of app. I have remove the overridden function and set the session.permanent flag to True, and set the permanent_session_lifetime with the specified value in config.py. Added new configuration parameter "SESSION_EXPIRATION_TIME_IN_DAYS".
  • Added CHECK_SESSION_FILES_INTERVAL configuration parameter in Hour with default value is 24. This is an interval to check the session files for the cleanup.    

--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Attachment

Re: [pgAdmin4][Patch]: RM #3674 Old session files are not cleared in.pgAdmin session and keep piling up

From
Khushboo Vashi
Date:


On Mon, Oct 8, 2018 at 6:30 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

Attached is the patch to fix the RM #3674 "Old session files are not cleared in .pgAdmin session and keep piling up". Patch contains following changes:
  • With current implementation we have override get_expiration_time() method of flask and return the expiry time as "datetime.datetime.now() + 1 day (hard coded)". Also the time used as now() instead of utcnow() which is there in base class method of flask.
    • Solution:  We should use session.permanent flag and set the permanent_session_lifetime variable of app. I have remove the overridden function and set the session.permanent flag to True, and set the permanent_session_lifetime with the specified value in config.py. Added new configuration parameter "SESSION_EXPIRATION_TIME_IN_DAYS".
  • Added CHECK_SESSION_FILES_INTERVAL configuration parameter in Hour with default value is 24. This is an interval to check the session files for the cleanup.    
Can we have this call inside FileBackedSessionManager class? As we have already created the SessionInterface, we should accommodate this into that interface. Also, I would prefer to have the removal check code while opening the session. So, before creating any new session, all the old files (with the threshold we have set in config) should be removed.

@ Akshay,
When you have proposed the design, I couldn't think of this, so, I have raised my concern now.

@ Team,
Your thoughts?


--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

Re: [pgAdmin4][Patch]: RM #3674 Old session files are not cleared in.pgAdmin session and keep piling up

From
Khushboo Vashi
Date:


On Mon, Oct 8, 2018 at 6:30 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

Attached is the patch to fix the RM #3674 "Old session files are not cleared in .pgAdmin session and keep piling up". Patch contains following changes:
  • With current implementation we have override get_expiration_time() method of flask and return the expiry time as "datetime.datetime.now() + 1 day (hard coded)". Also the time used as now() instead of utcnow() which is there in base class method of flask.
    • Solution:  We should use session.permanent flag and set the permanent_session_lifetime variable of app. I have remove the overridden function and set the session.permanent flag to True, and set the permanent_session_lifetime with the specified value in config.py. Added new configuration parameter "SESSION_EXPIRATION_TIME_IN_DAYS".
  • Added CHECK_SESSION_FILES_INTERVAL configuration parameter in Hour with default value is 24. This is an interval to check the session files for the cleanup.    
Can we have this call inside FileBackedSessionManager class? As we have already created the SessionInterface, we should accommodate this into that interface. Also, I would prefer to have the removal check code while opening the session. So, before creating any new session, all the old files (with the threshold we have set in config) should be removed.

@ Akshay,
When you have proposed the design, I couldn't think of this, so, I have raised my concern now.

@ Team,
Your thoughts?


--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

Re: [pgAdmin4][Patch]: RM #3674 Old session files are not cleared in.pgAdmin session and keep piling up

From
Akshay Joshi
Date:
Hi Khushboo

On Tue, Oct 9, 2018 at 9:38 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Mon, Oct 8, 2018 at 6:30 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

Attached is the patch to fix the RM #3674 "Old session files are not cleared in .pgAdmin session and keep piling up". Patch contains following changes:
  • With current implementation we have override get_expiration_time() method of flask and return the expiry time as "datetime.datetime.now() + 1 day (hard coded)". Also the time used as now() instead of utcnow() which is there in base class method of flask.
    • Solution:  We should use session.permanent flag and set the permanent_session_lifetime variable of app. I have remove the overridden function and set the session.permanent flag to True, and set the permanent_session_lifetime with the specified value in config.py. Added new configuration parameter "SESSION_EXPIRATION_TIME_IN_DAYS".
  • Added CHECK_SESSION_FILES_INTERVAL configuration parameter in Hour with default value is 24. This is an interval to check the session files for the cleanup.    
Can we have this call inside FileBackedSessionManager class? As we have already created the SessionInterface, we should accommodate this into that interface. Also, I would prefer to have the removal check code while opening the session. So, before creating any new session, all the old files (with the threshold we have set in config) should be removed.

   Yesterday we sit together and try to do that as we have created cleanup_session_files() function in FileBackedSessionManager class and from cleanup function of "misc/__init__.py" we will have to call that function like "current_app.sesstion_interface.manager.parent.clean_session_files()" and this doesn't look good as we discussed. Regarding deleting the old file before creating any new session will be an overhead to iterate over the sessions folder check the last modified time of each file and compare that for cleanup, so with my patch that time is configurable and by default it is 24 hours.    

@ Akshay,
When you have proposed the design, I couldn't think of this, so, I have raised my concern now.

@ Team,
Your thoughts?


--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246


--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

Re: [pgAdmin4][Patch]: RM #3674 Old session files are not cleared in.pgAdmin session and keep piling up

From
Akshay Joshi
Date:
Hi Khushboo

On Tue, Oct 9, 2018 at 9:38 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Mon, Oct 8, 2018 at 6:30 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

Attached is the patch to fix the RM #3674 "Old session files are not cleared in .pgAdmin session and keep piling up". Patch contains following changes:
  • With current implementation we have override get_expiration_time() method of flask and return the expiry time as "datetime.datetime.now() + 1 day (hard coded)". Also the time used as now() instead of utcnow() which is there in base class method of flask.
    • Solution:  We should use session.permanent flag and set the permanent_session_lifetime variable of app. I have remove the overridden function and set the session.permanent flag to True, and set the permanent_session_lifetime with the specified value in config.py. Added new configuration parameter "SESSION_EXPIRATION_TIME_IN_DAYS".
  • Added CHECK_SESSION_FILES_INTERVAL configuration parameter in Hour with default value is 24. This is an interval to check the session files for the cleanup.    
Can we have this call inside FileBackedSessionManager class? As we have already created the SessionInterface, we should accommodate this into that interface. Also, I would prefer to have the removal check code while opening the session. So, before creating any new session, all the old files (with the threshold we have set in config) should be removed.

   Yesterday we sit together and try to do that as we have created cleanup_session_files() function in FileBackedSessionManager class and from cleanup function of "misc/__init__.py" we will have to call that function like "current_app.sesstion_interface.manager.parent.clean_session_files()" and this doesn't look good as we discussed. Regarding deleting the old file before creating any new session will be an overhead to iterate over the sessions folder check the last modified time of each file and compare that for cleanup, so with my patch that time is configurable and by default it is 24 hours.    

@ Akshay,
When you have proposed the design, I couldn't think of this, so, I have raised my concern now.

@ Team,
Your thoughts?


--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246


--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

Re: [pgAdmin4][Patch]: RM #3674 Old session files are not cleared in.pgAdmin session and keep piling up

From
Dave Page
Date:
Thanks, patch applied.

On Mon, Oct 8, 2018 at 2:00 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Hackers

Attached is the patch to fix the RM #3674 "Old session files are not cleared in .pgAdmin session and keep piling up". Patch contains following changes:
  • With current implementation we have override get_expiration_time() method of flask and return the expiry time as "datetime.datetime.now() + 1 day (hard coded)". Also the time used as now() instead of utcnow() which is there in base class method of flask.
    • Solution:  We should use session.permanent flag and set the permanent_session_lifetime variable of app. I have remove the overridden function and set the session.permanent flag to True, and set the permanent_session_lifetime with the specified value in config.py. Added new configuration parameter "SESSION_EXPIRATION_TIME_IN_DAYS".
  • Added CHECK_SESSION_FILES_INTERVAL configuration parameter in Hour with default value is 24. This is an interval to check the session files for the cleanup.    

--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company