Bug 57300

Created attachment 33017 [details]
Proposed new directive SessionExpiryUpdateInterval

This patch adds a new directive to mod_session, SessionExpiryUpdateInterval, similar to the description by Christoph. It defines the number of seconds that the expiry value may change in a clean session without having to save the session again.

It's added to mod_session instead of mod_session_dbd because the session submodules don't have access to the original expiry value. It also has the benefit of skipping session encoding/crypto. This change is disabled by default, meaning sessions with expiry changes will be written every request unless this directive is set.

By putting this in mod_session, it means submodules won't always receive a call to save. So the Cache-Control headers must be set on session load, which I would argue is more correct anyway, since the use of session data in processing the request (e.g. form auth) should prevent caching, regardless of whether or not the response contains session data.

The patch also includes a change to how sessions without a max age are saved. Following the same logic that we don't need to write a session if the data hasn't changed and the expiry has only changed by a small amount, we don't need to write a session if the data hasn't changed and there is no expiry at all. In this case, it's a behavior change that is not controlled by a directive. Perhaps the most noticeable effect is that new sessions that are never written to are never saved. For example, the presence of Session On without anything that uses the session will no longer result in a session cookie.

I will attach a second patch containing the test changes.

Created attachment 33018 [details]
Updated tests for proposed patch

Test changes for the SessionExpiryUpdateInterval. It adds new tests that are skipped in 2.4 for now, and updates existing tests that do not modify the session to expect that the session will not be saved. Those tests are marked todo in 2.4 since they will fail without the changes in the proposed patch.

All tests pass against 2.4 without the patch and trunk with the patch. In my manual tests using mod_session_dbd with sqlite, I observed a 10-20x improvement in time required to make 600 requests that use mod_auth_form. That's essentially a worst-case scenario since sqlite is not optimized for writes.

I also observed a MySQL installation where the same 600 requests no longer generated 600 UPDATE queries to the DB server.

Just to be explicit about it given the size of the patches:

The code in this patchset is Copyright (c) National Instruments, licensed to the public under the Apache License 2.0.

Thanks Paul, committed in r1709121.
I will let others review it on trunk, commit and run the tests, and then propose a backport.

(In reply to Yann Ylavic from comment #4)
> Thanks Paul, committed in r1709121.
Forgot to mention that I made minor changes, like s/long/apr_time_t/ for expiry_update_time or SessionExpiryUpdateInterval directive doc/command description and validation.

(In reply to Yann Ylavic from comment #5)
> (In reply to Yann Ylavic from comment #4)
> > Thanks Paul, committed in r1709121.
> Forgot to mention that I made minor changes, like s/long/apr_time_t/ for
> expiry_update_time or SessionExpiryUpdateInterval directive doc/command
> description and validation.

Sounds good. Thanks for taking a look at it Yann!

Created attachment 33187 [details]
Updated tests

Tests updated to match changes made in r1707969 for handling large numbers. Now applies cleanly to httpd test trunk.

Backported today in r1709121. Will be part of 2.4.41.