User tests: Successful: Unsuccessful:
Because apparently nobody seems to care that garbage collection doesn't happen until it hits the database, put back the broken session management code so that the database stays happy.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
This PR is not a solution for me
Cronjob is designed to do that. For other users that does not have that possibility, they should use a plugin designed for that (for B/C it should be enabled).
Could we add a new config option (show-on option, only show when Database Handler is used):
Delete session on page load with two options Yes/No. Yes should be default behavior and for power users, he can set it to No and setup cron job merged in PR #19548 to do the cleanup?
No. We should not have a different behavior path for the database handler in relation to other handlers.
To be fair, the session data which we are deleting on page load at the moment can be considered as session meta data. Even we can move it to be stored in a separate table as you mentioned elsewhere, that delete query should be run on page load (at least default behavior) because look like it is difficult for end users to setup cron job to do the clean up
The point in moving the cleanup operation to the end of the request cycle was to move the performance penalty for that operation from being a blocking event to something that happens after the user should have the response document so the penalty (even if we're only talking about milliseconds) isn't seen by the user as visibly (you might still see it in a network waterfall chart on dev tools).
Additionally, 3.8.3 and earlier don't error trap that operation. So if that query fails for whatever reason, it results in a 500 response. There is no reason that needs to be a request blocking operation, and no reason the cleanup operation needs to be a pre-execute action versus something that can happen post-execute.
The point in moving the cleanup operation to the end of the request cycle
I don't think anyone here against that change. That's why people don't like this full revert (I even prepared a PR to do partial revert https://github.com/joomla/joomla-cms/compare/staging...joomdonation:patch-20?expand=1 but didn't submit it)
The point here is that we need to clean up session meta data somehow and the easiest way to do that is through page load as how we were doing. Adding a new setting to allow disable that clean up on page load for power users as I said should make everyone happy (again, this is not related to session session garbage collection, it is just the session meta data)
The cleanup is still happening during a HTTP request cycle. Just at the end of it, not at the beginning. As it should be, this maintenance task should not be a request blocking task and having it at the beginning as was the case in 3.8.3 and earlier makes it request blocking.
I've been fighting for over two years to fix every architectural flaw in the session API (both with the real data and the metadata). Every pull request I have made to fix that gets reverted for one reason or another, and I honestly don't have the energy anymore to keep fighting a broken community that is willing to accept that a critical API piece is fatally implemented. I've laid out exactly what I am trying to do, to make things manageable for both "simple" users who just want Joomla to run and advanced users who are able to move these maintenance tasks out of the HTTP request scope (i.e. cron job). But aside from you and maybe 3 or 4 others, it is next to impossible to convince people that the architectural benefits are worth the growing pains.
The cleanup is still happening during a HTTP request cycle. Just at the end of it, not at the beginning
Except that it is not happening for Database Session Handler and that's why people complain about it
If it is possible, I would suggest that:
Restore the delete session meta data on page load for Database Handler
I'm trying not to, because at that point we are back to the application class acting as an arbitrary session garbage collector for the database handler.
As explained in #19585 (comment) I will prepare a PR that will have everything needed to make the system work efficiently. The one hard line I am going to draw in the sand though is that I am not removing the database check. The code that I am writing/proposing explicitly differentiates between the metadata and the session data, and explicitly only cleans the metadata (and as explained, with the schema changes I am aiming for, in 4.0 that check will be unnecessary).
I'm trying not to, because at that point we are back to the application class acting as an arbitrary session garbage collector for the database handler.
As mentioned, the intention is clear session meta data. We just have that issue because we store both session and session meta data in the same table which we cannot address until J4 because B/C promise. Assume we move session meta data to a separate table In J4, we need that delete operation anyway
Anyway, looking forward to seeing that PR. And please close this un-welcomed PR.
I've been fighting for over two years to fix every architectural flaw in the session API
Reminds me of 'Hackwar's' fight to fix the router (7 years). Sometimes contributors to Joomla need extraordinary perseverance. :-)
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-02-20 08:47:04 |
Closed_By | ⇒ | brianteeman | |
Labels |
Added:
?
|
It is NOT true that nobody cares!!!!