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

Fixed concurrency issue when finding sessions #99

Merged
merged 1 commit into from
Nov 13, 2020

Conversation

alparslanavci
Copy link
Contributor

Fixes #84.

@alparslanavci alparslanavci added this to the 2.2 milestone Nov 12, 2020
Copy link

@leszko leszko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added one comment.

@@ -201,7 +201,13 @@ public Session findSession(String id) {
+ "Some failover occurred so reading session from Hazelcast map: " + getMapName());
}

HazelcastSession hazelcastSession = sessionMap.get(id);
HazelcastSession hazelcastSession;
sessionMap.lock(id);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any significant performance implication of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is difficult to say, but I don't think it will affect that much since only the session removal/update is done within the same synchronised block. This is not a good solution, however we cannot use EntryProcessors or MapInterceptors to handle invalidation.

@alparslanavci alparslanavci requested a review from leszko November 12, 2020 13:25
Copy link
Contributor

@enozcan enozcan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wonder which one is actually better in terms of performance: having a local cache with all these synchronization work or not having it at all.

@alparslanavci
Copy link
Contributor Author

@enozcan it might be good to have a benchmark for all session replication options, wdyt?

@enozcan
Copy link
Contributor

enozcan commented Nov 12, 2020

Sure! I'm curious about the results especially per module basis for particular scenarios.

@alparslanavci alparslanavci merged commit 32174d1 into hazelcast:master Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cuncurrent calls lead to no session found
3 participants