-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Insert of JDBC session attribute fails if concurrent request deletes session #1031
Comments
Thanks for the report @rfajarachmad. Can you show us your Spring Session related tables? With psql you can execute these commands to obtain the required information:
Also it would be useful to know which sequence of actions has led to this error. |
Hi, I've been getting the same errors periodically. Not sure how to reproduce the issue as I found it scanning the logs. Here is how my tables look:
|
I did add some extra logging to the rest error handler for this particular exception and the session looks intact.
|
@cwmccann If you are using Spring Session 1.3 then you have configured your database schema incorrectly. The presence of Make sure you use the correct schema scripts, which are included in the Spring Session JAR itself. |
I missed that the original issue was with spring session 1.3. I'm using: |
Thanks for the update @cwmccann. So it appears the same issue, affecting both 1.3 and 2.0. If someone could provide a sample to reproduce this, that would be great. |
One additional piece of information that could be useful is how many instances of your application do you run in a cluster? Are the system clocks on all servers in sync? |
I haven't figured out how to reproduce it yet. In this setup I have 3 services using the jdbc session, 2 on one box and 1 on another. NTP is not setup on them but the time seems to be within a second of each other. |
I am a little confused with this issue. The original stack trace above is from 1.3.2 because JdbcOperationsSessionRepository.java:464 in 2.0.3 is a "}". But there is mention that it is in both 1.3 and 2.0: "So it appears the same issue, affecting both 1.3 and 2.0.". I am not sure the issue is in 2.0. We are seeing the same error in our production environment with 1.3.1. However, we are not seeing this error in our testing environment which has 2.0.2. We are releasing out to production in the next week, or so. Maybe something will pop up then; hopefully not. Does anyone have a stack trace for 2.0? Since the code base has changed a fair bit between 1.3 and 2.0; it would be nice to have a newer stack trace. I was looking through the 2.0 code base and could not see how it could have a primary key issue except in a few situations:
A stack trace from the 2.0.3 code base would be helpful; but I have yet to find one. Also, we are using Spring Security 5.0.4.RELEASE. In production we use: Spring Security 4.2.3.RELEASE and Spring Session 1.3.1.RELEASE. Could there have been something between Spring Security and Spring Session? |
I'm pretty sure this is a duplicate of #1070 |
We just got a stack trace for version 2.0.2 I know things have changed in 2.0.3/4, but I think the logic is similar. What it looks like is happening is, somehow the Session is not being flagged as new. This leads to the code trying to update the session, and since updateCount == 0, an insert happens. This causes the Stack Trace. Since, nothing else is logged out I can not tell what was in the session. However, I cross referenced the time stamp in the stack trace with our access logs. One second before the timestamp on the stack trace there was a logout. The logout returns a 302 (redirect) to send the user back to the login page. One of the frameworks we are using makes a call back to the server before the login page (still trying to figure out why). It seems that the session is not being dropped from the server. It is getting dropped from the DB, but not the server which is Tomcat in our case. This means the session is not being flagged, or validated, as a new session?? Even though, it is not in the DB. I didn't look upstream in the code to see how the session is found and verified as new. But it does bring up the point that it is possible for a Session to exist in the Server and not in the DB.
|
Did a little more research and we think we figured out what is going on. It's a race condition. Essentially, what is going on is this:
I noticed in 2.0.3.RELEASE the code changed to this:
Not sure if the change in logic here will fix this problem. I did not look upstream to see how the DeltaValue is figured out. In a race condition like this, there is still the possibility for the DeltaValue.ADDED to be set even though there is no Session in the DB. So, maybe, right before the insertSessionAttribute(), or in the insertSessionAttribute() method, validate the Session still exists. |
Thank you for the detailed analysis @pminearo, this is very useful info. Could you give |
Will try it next week. Hopefully, we will have some more information within the next couple of weeks. |
We upgraded our production environment to 2.0.4 and got the stack trace this morning. So, it looks like it is still possible to have a race condition where one thread deletes the session right before the second thread tries to update it. I understand you are trying to optimize write operations, but it seems like it would be a good idea to validate the parent Session ID exists in the DB, which would only be a read, before calling the insertSessionAttributes() method on line 413 in JdbcOperationsSessionRepository. At least that will keep the validation to just the updates and not the inserts of new sessions.
|
We are seeing this error hundreds of time per day on production. We have 5 application nodes running and thousands of concurrent users. Would you have any estimation on the time to release a fix ? Thanks in advance ! |
I've just assigned this to @LouisVN Can you confirm the scenario in which you're encountering this error is similar to one described by @pminearo, i.e. concurrent requests where one invalidates/deletes the session while other attempts to update it? |
Good to hear. Thanks for that @vpavic ! I ran some experimentation and crossed-checked with the production environment. So I do believe that the scenario mentioned by @pminearo is the remaining point to be fixed. |
I have been trying to reproduce this locally, but have so far been unable to do so. I've tried using H2, PostgreSQL and MariaDB. My test scenario is basically something like this:
@pminearo & @LouisVN, could you provide as much info as possible about your database, which vendor do you use (@pminearo from stacktraces you posted I assume it's Oracle in your case), which version, as well as provide DDL dumps of the Spring Session tables. Additionally, how many instances of your application do you run in a cluster and do the concurrent requests involved end up being processed on the same or different servers? |
We have the following :
The DDL is :
It does not seem to happen much since we upgraded to the last version. It now seems like an edge use case. |
Interesting @LouisVN saw less exceptions when upgrading, when we saw an increase in exceptions when we upgraded to 2.0.4.RELEASE.
Reproducing the problem is difficult because it is a race condition. You have to time the requests in such a way that they both get the session and have it flagged as not new and then have the first session remove the entries from spring_session and spring_session_attribute before the second request tries to insert data in spring_session_attribute. I understand you are trying to increase performance and reduce the number of writes. But a quick check to ensure the Session ID exists before writing to the Session Attribute table should not be a significant decrease in performance. Especially, since it is a read. You just need to confirm the session still exists before writing. The other option is to catch any exception when trying to write to the spring_session_attribute table and log it as info, or warn, or just return assuming the user's session is over with anyways. Our problem is the exception is getting kicked out to the container which sends the on-call person a CRITICAL error text when this isn't a critical error. There is no way for us to control the logging level of the exception.
|
Using |
I've finally managed to dedicate some more time to this and reproduce the problem locally. I believe the optimal way to address this would be change SQL used to insert session attribute to be a conditional statement. This wouldn't require issuing additional Specifically, this means that instead of: INSERT INTO %TABLE_NAME%_ATTRIBUTES(SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES)
VALUES (?, ?, ?) we'd use: INSERT INTO %TABLE_NAME%_ATTRIBUTES(SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES)
SELECT PRIMARY_ID, ?, ?
FROM %TABLE_NAME%
WHERE SESSION_ID = ? @pminearo and @LouisVN, do you have any thoughts on this proposal? |
At present, the insert of new attributes in JdbcOperationsSessionRepository is done unconditionally. This can cause data integrity violation errors with concurrent requests, where one request attempts to add new session attribute while the other, concurrent request, deletes the session. This commit addresses the described scenario by executing insert of new attributes conditionally on presence of parent record. Closes spring-projectsgh-1031
I've pushed the proposed solution to this branch in my fork. The commit also includes a test that reproduces the problem. |
One additional detail I'm considering is to also add some logging when affected record count returned by |
Thanks @vpavic, this looks good at a quick glance. |
Thanks for the feedback @LouisVN. Snapshot are available from https://repo.spring.io/libs-snapshot/ so if you're able to give it a spin, we'd greatly appreciate it. |
JdbcOperationsSessionRepository recently introduced validation when inserting new session attributes in order to prevent data integrity violations in highly concurrent environments. This is done by using INSERT INTO ... SELECT statement to verify existence of session record in parent table. Such arrangement causes problems with Oracle if inserted attribute is of size 4 kb or more. This commit enhances JdbcHttpSessionConfiguration to detect Oracle database is used, and set createTemporaryLob option on default LobHandler to true. Resolves: #1203 See also: #1031
Hello We observed this behavior with spring-session 2.1.2 Which information can we provide to help investigation ? |
@blacelle Are you sure your comment references the right Spring Session version? I'm asking because JDBC session store support was added in Spring Session |
@vpavic You are right. I meant |
Hello, I also encounter the same issue reported here using spring-session 2.1.4, springboot 2.1.3.RELEASE. I cannot find the new issue opened by @blacelle The DDL script used is: sql
In our setup, we have multiple springboot applications being accessed by a client simultaneously. Each application point to the same db (spring session tables) to validate session. |
Hi everyone,
I'm using spring-session 1.3.2.RELEASE with postgresql, I got this following error on my production environment. When I tried to login into the system, the session was created in the database normally, but somehow this exception occurred, I haven't found how to replicate this. does anyone know how to fix it ?
The configuration
The text was updated successfully, but these errors were encountered: