Session ID generation strategy #1547
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
November 13, 2019 15:07
* Session id creation and id change is now session repo responsibility * Created default UUID based id generation strategy * Unless specified otherwise UUID is used as default id creation method * Add capability to change generation strategy for most repo-impl
spring-projects-issues
added
the
status: waiting-for-triage
zapp88
changed the title
|
This feature is a work in progress untill i can fix integration tests. |
zapp88
changed the title
|
Integration erros have been resolved |
|
I have a multiple concurrent file uploader which suffers from deadlocks on the session table with alarming frequency, resulting in the data having to be resent. Does this change resolve the issue of deadlocks occuring where multiple requests are received for the same session using mariadb and innodb tables? For instance would this let me change the spring session table to the strategy used by the rest of my database which is db generated integer IDs? If so is there any plan for including this in a release? I am sure a lot of people are keen to resolve the deadlock issue. |
This was referenced Mar 28, 2023
|
Closing in favor of #2286 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After creating PR : #1545 I became aware of long ongoing disscusion about
session generation strategy mainly : #204 which has merge conflicts and author is not present for quite a while, and a general issue #11 going back to 3 Jul 2014.
I tought i would try to takle the problem myself with the strategy approach (differently than in #1545). This PR is my attempt at assimilating strategy based way of controlling id generation process.
While I still prefer #1545 way of doing things 'this' PR is closer the expected behaviour of #11.
But not without a few pros and cons.
#1545 is less invasive - it provides means of controlling session id without changing too much. #1545 doesn't break any existing functionality - especially it doesn't remove any existing methodes - it only adds new.
At the same time
thisPR while gives us more elegant way of handling session id. It requires us to change a few things , mainly :Session creation is not session class reponsibility - it is session-repository reponsibility and as a result -
MapSessionshould get valid id from session-repository instead of creating it by itself. While this is easly guaranteed by removal of no-args constructor inMapSessionit requires us to modifychangeIdmethode signature in MapSession. The new id for session should be provided to session externally.Existing session repository implementations should provide a way of changing session generation strategy and as a result all session repository should implement additional method "changeSessionId".
Additionally since spring-session is a library - we don't want to introduce any new unwanted behaviour to projects rellying on it. Thats why i additionaly introduce UUIDSessionIdStrategy which is a default go-to behaviour for all session repositories if no strategy is provided.
For now i dont want this PR to grow to large or out of scope so I willy probably try to add the aformentioned feature as a separate PR if
thisgets accepted.