You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/07/20 17:43:53 UTC
[GitHub] [geode] sabbeyPivotal opened a new pull request #5385: GEODE-8370: Add test for maxInactiveInterval
sabbeyPivotal opened a new pull request #5385:
URL: https://github.com/apache/geode/pull/5385
Write test verifying that maxInactiveInterval propagates correctly to all buckets
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] sabbeyPivotal commented on a change in pull request #5385: GEODE-8370: Add test for maxInactiveInterval
Posted by GitBox <gi...@apache.org>.
sabbeyPivotal commented on a change in pull request #5385:
URL: https://github.com/apache/geode/pull/5385#discussion_r458229693
##########
File path: geode-redis/src/acceptanceTest/java/session/NativeRedisSessionExpirationAcceptanceTest.java
##########
@@ -50,4 +50,10 @@ public static void setup() {
public void sessionShouldTimeout_whenAppFailsOverToAnotherRedisServer() {
// Only using one server for Native Redis
}
+
+ @Test
+ @Ignore
Review comment:
Just updated it and took out the ignores!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] sabbeyPivotal commented on pull request #5385: GEODE-8370: Add test for maxInactiveInterval
Posted by GitBox <gi...@apache.org>.
sabbeyPivotal commented on pull request #5385:
URL: https://github.com/apache/geode/pull/5385#issuecomment-661949026
Failure in Distributed Test unrelated to this PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] sabbeyPivotal commented on a change in pull request #5385: GEODE-8370: Add test for maxInactiveInterval
Posted by GitBox <gi...@apache.org>.
sabbeyPivotal commented on a change in pull request #5385:
URL: https://github.com/apache/geode/pull/5385#discussion_r458229811
##########
File path: geode-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionExpirationDUnitTest.java
##########
@@ -86,15 +100,78 @@ public void sessionShouldTimeout_whenAppFailsOverToAnotherRedisServer() {
}
}
+ @Test
+ public void sessionShouldNotTimeout_whenPersisted() {
+ String sessionCookie = createNewSessionWithNote(APP2, "note1");
+ setMaxInactiveInterval(APP2, sessionCookie, -1);
+
+ compareMaxInactiveIntervals();
+ }
+
private void waitForTheSessionToExpire(String sessionId) {
GeodeAwaitility.await().ignoreExceptions().atMost((SHORT_SESSION_TIMEOUT + 5), TimeUnit.SECONDS)
.until(
- () -> jedisConnetedToServer1.ttl("spring:session:sessions:expires:" + sessionId) < 0);
+ () -> jedisConnetedToServer1.ttl("spring:session:sessions:expires:" + sessionId) == -2);
}
private void refreshSession(String sessionCookie, int sessionApp) {
GeodeAwaitility.await()
.during(SHORT_SESSION_TIMEOUT + 2, TimeUnit.SECONDS)
.until(() -> getSessionNotes(sessionApp, sessionCookie) != null);
}
+
+ void setMaxInactiveInterval(int sessionApp, String sessionCookie, int maxInactiveInterval) {
+ HttpHeaders requestHeaders = new HttpHeaders();
+ requestHeaders.add("Cookie", sessionCookie);
+ HttpEntity<Integer> request = new HttpEntity<>(maxInactiveInterval, requestHeaders);
+ new RestTemplate()
+ .postForEntity(
+ "http://localhost:" + ports.get(sessionApp) + "/setMaxInactiveInterval",
+ request,
+ Integer.class)
+ .getHeaders();
+ }
+
+ private void compareMaxInactiveIntervals() {
+ cluster.getVM(1).invoke(() -> {
+ for (int j = 0; j < 113; j++) {
Review comment:
Good suggestion! Just updated it!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] dschneider-pivotal merged pull request #5385: GEODE-8370: Add test for maxInactiveInterval
Posted by GitBox <gi...@apache.org>.
dschneider-pivotal merged pull request #5385:
URL: https://github.com/apache/geode/pull/5385
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] dschneider-pivotal commented on a change in pull request #5385: GEODE-8370: Add test for maxInactiveInterval
Posted by GitBox <gi...@apache.org>.
dschneider-pivotal commented on a change in pull request #5385:
URL: https://github.com/apache/geode/pull/5385#discussion_r458216460
##########
File path: geode-redis/src/acceptanceTest/java/session/NativeRedisSessionExpirationAcceptanceTest.java
##########
@@ -50,4 +50,10 @@ public static void setup() {
public void sessionShouldTimeout_whenAppFailsOverToAnotherRedisServer() {
// Only using one server for Native Redis
}
+
+ @Test
+ @Ignore
Review comment:
Normally when I see an ignore on a test I want to know why the test is being ignored. I think it is something we could fix and then have this test. In this case I think we are overriding a test that we will never do with native redis. It seems like the empty method impl takes care of that and you would not need the ignore. This is just my opinion, I'd be okay if you leave the ignore if you think that is best.
##########
File path: geode-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionExpirationDUnitTest.java
##########
@@ -86,15 +100,78 @@ public void sessionShouldTimeout_whenAppFailsOverToAnotherRedisServer() {
}
}
+ @Test
+ public void sessionShouldNotTimeout_whenPersisted() {
+ String sessionCookie = createNewSessionWithNote(APP2, "note1");
+ setMaxInactiveInterval(APP2, sessionCookie, -1);
+
+ compareMaxInactiveIntervals();
+ }
+
private void waitForTheSessionToExpire(String sessionId) {
GeodeAwaitility.await().ignoreExceptions().atMost((SHORT_SESSION_TIMEOUT + 5), TimeUnit.SECONDS)
.until(
- () -> jedisConnetedToServer1.ttl("spring:session:sessions:expires:" + sessionId) < 0);
+ () -> jedisConnetedToServer1.ttl("spring:session:sessions:expires:" + sessionId) == -2);
}
private void refreshSession(String sessionCookie, int sessionApp) {
GeodeAwaitility.await()
.during(SHORT_SESSION_TIMEOUT + 2, TimeUnit.SECONDS)
.until(() -> getSessionNotes(sessionApp, sessionCookie) != null);
}
+
+ void setMaxInactiveInterval(int sessionApp, String sessionCookie, int maxInactiveInterval) {
+ HttpHeaders requestHeaders = new HttpHeaders();
+ requestHeaders.add("Cookie", sessionCookie);
+ HttpEntity<Integer> request = new HttpEntity<>(maxInactiveInterval, requestHeaders);
+ new RestTemplate()
+ .postForEntity(
+ "http://localhost:" + ports.get(sessionApp) + "/setMaxInactiveInterval",
+ request,
+ Integer.class)
+ .getHeaders();
+ }
+
+ private void compareMaxInactiveIntervals() {
+ cluster.getVM(1).invoke(() -> {
+ for (int j = 0; j < 113; j++) {
Review comment:
instead of the hard coded "113", ask the PartitionedRegion for its max bucket. See PartitionedRegion.getTotalNumberOfBuckets. This will help in the future when we allow the redis bucket count to be configured
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org