You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by upthewaterspout <gi...@git.apache.org> on 2017/08/24 18:00:22 UTC
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
GitHub user upthewaterspout opened a pull request:
https://github.com/apache/geode/pull/740
GEODE-3513: Removing the use of native sessions session caching
In the session module for generic app servers, we were asking the
application server for a 'native' session and then wrapping it on our
own GemfireHttpSession. However, we were not cleaning up that native
session, which means that in PROXY mode we were leaving these sessions
on the client with them being useful.
The GemfireHttpSession now no longer wraps a native session. We are
still temporarily creating a native session because it is the only way
for us to get the session timeout value that was configured in web.xml,
but the native session is immediately invalidated.
Adding and extending cargo session tests to test how sessions are being
cleaned up from the clients and the server.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-3513
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/740.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #740
----
commit 93163bf8077cd6363dfcb9c1e1a37d1876e1669e
Author: Dan Smith <up...@apache.org>
Date: 2017-08-11T16:46:40Z
GEODE-3513: Removing the use of native sessions session caching
In the session module for generic app servers, we were asking the
application server for a 'native' session and then wrapping it on our
own GemfireHttpSession. However, we were not cleaning up that native
session, which means that in PROXY mode we were leaving these sessions
on the client with them being useful.
The GemfireHttpSession now no longer wraps a native session. We are
still temporarily creating a native session because it is the only way
for us to get the session timeout value that was configured in web.xml,
but the native session is immediately invalidated.
Adding and extending cargo session tests to test how sessions are being
cleaned up from the clients and the server.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/740#discussion_r135148933
--- Diff: extensions/geode-modules-session-internal/src/main/java/org/apache/geode/modules/session/internal/filter/attributes/AbstractSessionAttributes.java ---
@@ -66,6 +66,8 @@
*/
protected String jvmOwnerId;
+ protected long creationTime;
--- End diff --
Does this field need to be serialized?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/geode/pull/740
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #740: GEODE-3513: Removing the use of native sessions session ca...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the issue:
https://github.com/apache/geode/pull/740
@jhuynh1 @boglesby
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/740#discussion_r135150926
--- Diff: extensions/session-testing-war/src/main/java/org/apache/geode/modules/session/SessionCountingListener.java ---
@@ -12,28 +12,35 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
+package org.apache.geode.modules.session;
-package org.apache.geode.modules.session.internal.filter;
-
-import org.apache.geode.modules.session.filter.SessionCachingFilter;
-
-import javax.servlet.http.HttpSession;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.http.HttpSessionEvent;
import javax.servlet.http.HttpSessionListener;
-public class HttpSessionListenerImpl2 extends AbstractListener implements HttpSessionListener {
+public class SessionCountingListener extends ListenerStoredInSessionContext
+ implements HttpSessionListener {
+ private final AtomicInteger sessionCount = new AtomicInteger();
--- End diff --
Is this sessionCount field used?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/740#discussion_r135150238
--- Diff: extensions/geode-modules-session/src/main/java/org/apache/geode/modules/session/filter/SessionListener.java ---
@@ -22,6 +22,11 @@
import javax.servlet.http.HttpSessionEvent;
import javax.servlet.http.HttpSessionListener;
+/**
--- End diff --
modify_war should no longer add this session listener.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/740#discussion_r135151782
--- Diff: geode-assembly/src/test/java/org/apache/geode/session/tests/Jetty9CachingClientServerTest.java ---
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.session.tests;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.concurrent.TimeUnit;
+
+import javax.servlet.http.HttpSession;
+
+import org.awaitility.Awaitility;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.Region;
+import org.apache.geode.modules.session.functions.GetSessionCount;
+import org.apache.geode.test.dunit.DUnitEnv;
+
+/**
+ * Jetty 9 Client Server tests
+ *
+ * Runs all the tests in {@link CargoTestBase} on the Jetty 9 install, setup in the
+ * {@link #setupJettyInstall()} method before tests are run.
+ */
+public class Jetty9CachingClientServerTest extends GenericAppServerClientServerTest {
+ private static ContainerInstall install;
+
+ @BeforeClass
+ public static void setupJettyInstall() throws Exception {
+ install = new GenericAppServerInstall(GenericAppServerInstall.GenericAppServerVersion.JETTY9,
+ ContainerInstall.ConnectionType.CACHING_CLIENT_SERVER,
+ ContainerInstall.DEFAULT_INSTALL_DIR + "Jetty9ClientServerTest");
--- End diff --
Change this install directory?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---