Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SessionRepositoryFilter.SessionRepositoryRequestWrapper.commitSession() triggers extraneous call to SessionRespository.findById(...) #1731

Open
pferraro opened this issue Nov 20, 2020 · 4 comments
Assignees
Labels
in: core type: enhancement A general enhancement

Comments

@pferraro
Copy link

pferraro commented Nov 20, 2020

Describe the bug
For a valid session, SessionRepositoryFilter.SessionRepositoryRequestWrapper.commitSession() does the following:

  1. Clears cached values for references for requestedSessionCache, requestedSession, and requestedSessionId.
  2. Saves the state of the session
  3. If the requested session ID is not valid, or if the ID of the session changed from the requested session ID, then the session ID is sent to the client

However, since #1 already cleared the requested session cached values #3 requires a completely redundant call to SessionRepository.findById(...). Depending on the implementation, this can be unnecessarily expensive.

To Reproduce
Use a tracer to detect calls to SessionRepository.findById(...). For a requested session, this is triggered twice, once at the beginning of the request, and again after the session is saved.

Expected behavior
SessionRepository.findById(...) should be considered a potentially expensive operation, and only triggered when necessary. Clearing cached values after determining whether to send the session ID to the client (i.e. in a finally block) would solve the problem.

@pferraro pferraro added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 20, 2020
pferraro added a commit to pferraro/wildfly-clustering-spring that referenced this issue Nov 22, 2020
pferraro added a commit to pferraro/wildfly-clustering-spring that referenced this issue Nov 23, 2020
Upgrade spring-session to 2.4.1.
Add workaround for spring-projects/spring-session#1731
@eleftherias eleftherias self-assigned this Nov 27, 2020
@eleftherias eleftherias added in: core type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 27, 2020
pferraro added a commit to pferraro/spring-session that referenced this issue Nov 28, 2020
…epositoryRequestWrapper.commitSession() due to premature cache clearing.

Closes spring-projectsgh-1731
pferraro added a commit to pferraro/spring-session that referenced this issue Dec 4, 2020
…epositoryRequestWrapper.commitSession() due to premature cache clearing.

Closes spring-projectsgh-1731
@ashwaniagarwal226
Copy link

Hi All can i know in which release above fix will be available as i am facing the same issue mentioned above

@anilgupta22
Copy link

Spring Session works great for session management but noticed lot of slowness because of this known issue. This is very urgent issue and blocker. Is there any official release date for this?

@jlibasci
Copy link

jlibasci commented Mar 8, 2021

All this is a significant impact to performance for spring with apps that are using session management, Any chance we will see this fix pushed to the master branch in the next week or so.

Thanks in advance.

@kunalsurana
Copy link

This issue impacts the performance for large JSPs which has jsp:include where it tries to commit on each include but due to session getting cleared and call to getRequestedSessionId makes getSessionQuery again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core type: enhancement A general enhancement
Projects
None yet
6 participants