Improve cleanUpExpiredSessions query

[okohub]

Hello,

I was profiling our production database, which is MySQL, and found that expiration query is not optimized.

DELETE FROM %TABLE_NAME% WHERE MAX_INACTIVE_INTERVAL < (? - LAST_ACCESS_TIME) / 1000;

This query is not using any index. However, if you change query to something like this you will get a point;

DELETE FROM %TABLE_NAME% WHERE (? - (MAX_INACTIVE_INTERVAL * 1000)) >  LAST_ACCESS_TIME

We know that MAX_INACTIVE_INTERVAL is constant in our table, so this expression can be evaluated before query execution

(? - (MAX_INACTIVE_INTERVAL * 1000)) 

Now, final query will be one of these:

DELETE FROM %TABLE_NAME% WHERE ? >  LAST_ACCESS_TIME

or 

DELETE FROM %TABLE_NAME% WHERE LAST_ACCESS_TIME < ?

Simple and index friendly.

Is there a chance to review this?

Thank you.

[cemo]

@vpavic also MAX_INACTIVE_INTERVAL column might be not necessary. It can be added to necessary queries by just calculating. This can also eliminate a necessary update in session tables in case a change in this value.

[vpavic]

Hi @okohub and @cemo,

It seems that you are not taking into account that both Servlet API (via HttpSession#setMaxInactiveInterval) and Spring Session's API (via Session#setMaxInactiveInterval, or ExpiringSession#setMaxInactiveInterval in pre-2.0) allow for session's max inactive interval to be set on a per-session basis.

There's an example of such API usage in Spring Session's SpringSessionRememberMeServices:

public final void loginSuccess(HttpServletRequest request,
		HttpServletResponse response, Authentication successfulAuthentication) {
	if (!this.alwaysRemember
			&& !rememberMeRequested(request, this.rememberMeParameterName)) {
		logger.debug("Remember-me login not requested.");
		return;
	}
	request.setAttribute(REMEMBER_ME_LOGIN_ATTR, true);
	request.getSession().setMaxInactiveInterval(this.validitySeconds);
}

With that in mind, naturally we need to persist the max inactive interval in a dedicated column in order for session cleanup to work correctly.

Regarding the SQL statement used for cleanup of expired sessions, actually the statement originally was basically the same as @okohub suggested but this was problematic in some scenarios - see #679.

Finally, note that you can always use JdbcOperationsSessionRepository#setDeleteSessionsByLastAccessTimeQuery to customize the SQL statement.

[cemo]

Thanks for the clarification @vpavic. It seems dedicated column is required for the Servlet Session API. What about adding another column to find a workaround speed up deleting process? There is a column MaxInactiveInterval and if we can add something like ExpiryTime, it can speed up deletion without using full table scan. What do you think?

[vpavic]

@rwinch and I have discussed this, and we agree that pre-calculating session's expiry time and persisting it in dedicated column seems as the way to go considering the performance of session cleanup.

We're targeting this enhancement at the 2.0 release.

[vpavic]

@okohub @cemo I've opened #872 to address this - I'd be grateful if you could take a quick look and provide some feedback on the proposed change. Thanks.

[cemo]

It seems OK at first glance. 🥇