You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/08/31 06:29:54 UTC

[GitHub] [ignite-3] zstan commented on a diff in pull request #1041: IGNITE-17585 fix flaky ItCommonApiTest.testSessionExpiration test

zstan commented on code in PR #1041:
URL: https://github.com/apache/ignite-3/pull/1041#discussion_r959199399


##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItCommonApiTest.java:
##########
@@ -62,14 +62,14 @@ public void testSessionExpiration() throws Exception {
         sql("CREATE TABLE TST(id INTEGER PRIMARY KEY, val INTEGER)");
         sql("INSERT INTO TST VALUES (1,1), (2,2), (3,3), (4,4)");
 
-        Session ses1 = sql.sessionBuilder().defaultPageSize(1).idleTimeout(1, TimeUnit.MILLISECONDS).build();
+        Session ses1 = sql.sessionBuilder().defaultPageSize(1).idleTimeout(1, TimeUnit.SECONDS).build();

Review Comment:
   I found : public static final long SESSION_EXPIRE_CHECK_PERIOD = TimeUnit.SECONDS.toMillis(1);
   Why do we need hardcoded: **long timeout** ? Also comment : "SessionManager.checkPeriod" confusing.



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItCommonApiTest.java:
##########
@@ -62,14 +62,14 @@ public void testSessionExpiration() throws Exception {
         sql("CREATE TABLE TST(id INTEGER PRIMARY KEY, val INTEGER)");
         sql("INSERT INTO TST VALUES (1,1), (2,2), (3,3), (4,4)");
 
-        Session ses1 = sql.sessionBuilder().defaultPageSize(1).idleTimeout(1, TimeUnit.MILLISECONDS).build();
+        Session ses1 = sql.sessionBuilder().defaultPageSize(1).idleTimeout(1, TimeUnit.SECONDS).build();
         Session ses2 = sql.sessionBuilder().defaultPageSize(1).idleTimeout(100, TimeUnit.SECONDS).build();
 
         ResultSet rs1 = ses1.execute(null, "SELECT id FROM TST");
         ResultSet rs2 = ses2.execute(null, "SELECT id FROM TST");
 
         // waiting for run session cleanup thread
-        Thread.sleep(timeout);
+        Thread.sleep(timeout + ses1.idleTimeout(TimeUnit.MILLISECONDS));

Review Comment:
   why do we need **ses1.idleTimeout(TimeUnit.MILLISECONDS)** addition here ? 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org