Implement findSessions() and return all sessions in the cluster. #40
Conversation
|
Thanks for the contribution @bergander, I'll have a look at this today. |
|
@bergander Can you please create some tests that fail on the current version of the code and pass on this version? It will help us to see the problem clearly, and make sure we don't break it in the future. I'm worried about the performance implications of this approach as it gets all non-local sessions from the distributed map. A better approach might be to configure eviction on the Hazelcast IMap instance. We could use the configured session-timeout value on Tomcat to configure eviction on the Hazelcast IMap, and add an entry listener that would notify Tomcat about the expired session when it happens. Anyway, this can be addressed later. What we need first is a test that shows the problem clearly. |
|
@emre-aydin I've updated this pull request with a test for session expiration after failover. Note that I had to fix the session timeout setting for tomcat 8 and 8.5 which didn't work. Btw, I agree about the performance concerns. However, using map eviction would mean that all sessions must have the same timeout. But timeouts may be set individual on each session using Another option would be to fetch sessions using sessionMap.values(predicate) with an appropriate predicate, but I guess you still need to deserialize all sessions then. |
|
verify |
|
Test PASSed. |
| final int EXTRA_DELAY_IN_SECONDS = 5; | ||
|
|
||
| instance1 = getWebContainerConfigurator(); | ||
| instance1.port(SERVER_PORT_1).sticky(true).clientOnly(false).mapName(SESSION_REPLICATION_MAP_NAME).sessionTimeout(SESSION_TIMEOUT_IN_MINUTES).start(); |
There was a problem hiding this comment.
It would be nicer to read if you put those calls on separate lines, similar to here. Also, you should add .configLocation("hazelcast-1.xml") and .configLocation("hazelcast-2.xml") respectively to the instances.
| @@ -0,0 +1,8 @@ | |||
| package com.hazelcast.session; | |||
|
|
|||
| public class Tomcat6SessionExpireTest extends AbstractSessionExpireTest { | |||
There was a problem hiding this comment.
Can you also please add the following annotations to all the concrete test classes you add?
@RunWith(HazelcastSerialClassRunner.class)
@Category(QuickTest.class)
When I add those, it complains not all instances are stopped after a test, could you please make sure all Hazelcast instances are stopped after a test as well?
# Conflicts: # tomcat-core/src/test/java/com/hazelcast/session/AbstractSessionExpireTest.java # tomcat6/src/test/java/com/hazelcast/session/Tomcat6SessionExpireTest.java # tomcat7/src/test/java/com/hazelcast/session/Tomcat7SessionExpireTest.java # tomcat85/src/test/java/com/hazelcast/session/Tomcat85SessionExpireTest.java
|
verify |
|
Test PASSed. |
There was a problem hiding this comment.
Looks good to me. I'll have another team member have a look at it and then we can merge it. @mesutcelik can you please have a look?
Implement findSessions() and return all sessions in the cluster so that sessions not connected to a cluster node anymore can be expired. Solves issue #39 .