From 74df75b45033469e7b8f6c9d4b83afd74e016f67 Mon Sep 17 00:00:00 2001 From: Paulius Date: Sat, 5 Aug 2023 19:01:59 +0300 Subject: [PATCH 1/2] Improve session timeout handling --- docs/configuration.md | 31 ++++++++++--------- .../application/sessions/SessionHandler.java | 11 +++++-- .../application/sessions/SessionService.java | 4 +++ .../configuration/AppConfiguration.java | 7 +++++ server/src/main/resources/application.yml | 1 + .../sessions/SessionHandlerTest.groovy | 17 ++++++++++ .../exacaster/lighter/test/Factories.groovy | 3 +- 7 files changed, 55 insertions(+), 19 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 526a9bdd..2dc7df29 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -4,21 +4,22 @@ Lighter can be configured by using environment variables. Currently, Lighter sup ## Global properties -| Property | Description | Default | -|----------------------------------------|----------------------------------------------------------------|---------------------------------| -| LIGHTER_MAX_RUNNING_JOBS | Max running Batch jobs in parallel | 5 | -| LIGHTER_MAX_STARTING_JOBS | Max starting Batch jobs in parallel | 5 | -| LIGHTER_SPARK_HISTORY_SERVER_URL | Spark history server URL used on frontend | http://localhost/spark-history/ | -| LIGHTER_EXTERNAL_LOGS_URL_TEMPLATE | Template for link to external logs | | -| LIGHTER_PY_GATEWAY_PORT | Port for live Spark session communication | 25333 | -| LIGHTER_URL | URL which can be used to access Lighter form Spark Job | http://lighter.spark:8080 | -| LIGHTER_SESSION_TIMEOUT_MINUTES | Session lifetime in minutes | 90 | -| LIGHTER_STORAGE_JDBC_URL | JDBC url for lighter storage | jdbc:h2:mem:lighter | -| LIGHTER_STORAGE_JDBC_USERNAME | JDBC username | sa | -| LIGHTER_STORAGE_JDBC_PASSWORD | JDBC password | | -| LIGHTER_STORAGE_JDBC_DRIVER_CLASS_NAME | JDBC driver class name | org.h2.Driver | -| LIGHTER_BATCH_DEFAULT_CONF | Default `conf` props for batch applications (JSON)* | | -| LIGHTER_SESSION_DEFAULT_CONF | Default `conf` props for session applications (JSON) | | +| Property | Description | Default | +|----------------------------------------|-------------------------------------------------------------------------------------------|---------------------------------| +| LIGHTER_MAX_RUNNING_JOBS | Max running Batch jobs in parallel | 5 | +| LIGHTER_MAX_STARTING_JOBS | Max starting Batch jobs in parallel | 5 | +| LIGHTER_SPARK_HISTORY_SERVER_URL | Spark history server URL used on frontend | http://localhost/spark-history/ | +| LIGHTER_EXTERNAL_LOGS_URL_TEMPLATE | Template for link to external logs | | +| LIGHTER_PY_GATEWAY_PORT | Port for live Spark session communication | 25333 | +| LIGHTER_URL | URL which can be used to access Lighter form Spark Job | http://lighter.spark:8080 | +| LIGHTER_SESSION_TIMEOUT_MINUTES | Session lifetime in minutes (from last statement creation). Use negative value to disable | 90 | +| LIGHTER_SESSION_TIMEOUT_ACTIVE | Should lighter kill sessions with waiting statements | false | +| LIGHTER_STORAGE_JDBC_URL | JDBC url for lighter storage | jdbc:h2:mem:lighter | +| LIGHTER_STORAGE_JDBC_USERNAME | JDBC username | sa | +| LIGHTER_STORAGE_JDBC_PASSWORD | JDBC password | | +| LIGHTER_STORAGE_JDBC_DRIVER_CLASS_NAME | JDBC driver class name | org.h2.Driver | +| LIGHTER_BATCH_DEFAULT_CONF | Default `conf` props for batch applications (JSON)* | | +| LIGHTER_SESSION_DEFAULT_CONF | Default `conf` props for session applications (JSON) | | * default confs will be merged with confs provided in submit request, if property is defined in submit request, default will be ignored. Example of `LIGHTER_BATCH_DEFAULT_CONF`: `{"spark.kubernetes.driverEnv.TEST1":"test1"}`. diff --git a/server/src/main/java/com/exacaster/lighter/application/sessions/SessionHandler.java b/server/src/main/java/com/exacaster/lighter/application/sessions/SessionHandler.java index 849c37e3..757f14cf 100644 --- a/server/src/main/java/com/exacaster/lighter/application/sessions/SessionHandler.java +++ b/server/src/main/java/com/exacaster/lighter/application/sessions/SessionHandler.java @@ -111,11 +111,11 @@ public void handleTimeout() { assertLocked(); var sessionConfiguration = appConfiguration.getSessionConfiguration(); var timeout = sessionConfiguration.getTimeoutMinutes(); - if (timeout != null) { + if (timeout != null && timeout > 0) { sessionService.fetchRunning() .stream() - .filter(s -> sessionConfiguration.getPermanentSessions().stream() - .noneMatch(conf -> conf.getId().equals(s.getId()))) + .filter(s -> isNotPermanent(sessionConfiguration, s)) + .filter(s -> sessionConfiguration.shouldTimeoutActive() || !sessionService.isActive(s)) .filter(s -> sessionService.lastUsed(s.getId()).isBefore(LocalDateTime.now().minusMinutes(timeout))) .peek(s -> LOG.info("Killing because of timeout {}, session: {}", timeout, s)) .forEach(sessionService::killOne); @@ -123,6 +123,11 @@ public void handleTimeout() { } + private boolean isNotPermanent(AppConfiguration.SessionConfiguration sessionConfiguration, Application session) { + return sessionConfiguration.getPermanentSessions().stream() + .noneMatch(conf -> conf.getId().equals(session.getId())); + } + private List selfOrEmpty(List list) { return ofNullable(list).orElse(List.of()); } diff --git a/server/src/main/java/com/exacaster/lighter/application/sessions/SessionService.java b/server/src/main/java/com/exacaster/lighter/application/sessions/SessionService.java index d3d424d4..b7557329 100644 --- a/server/src/main/java/com/exacaster/lighter/application/sessions/SessionService.java +++ b/server/src/main/java/com/exacaster/lighter/application/sessions/SessionService.java @@ -131,4 +131,8 @@ public List getStatements(String id, Integer from, Integer size) { .limit(size) .collect(Collectors.toList()); } + + public boolean isActive(Application application) { + return statementHandler.hasWaitingStatement(application); + } } diff --git a/server/src/main/java/com/exacaster/lighter/configuration/AppConfiguration.java b/server/src/main/java/com/exacaster/lighter/configuration/AppConfiguration.java index b16349b5..0fef8090 100644 --- a/server/src/main/java/com/exacaster/lighter/configuration/AppConfiguration.java +++ b/server/src/main/java/com/exacaster/lighter/configuration/AppConfiguration.java @@ -141,12 +141,15 @@ public String toString() { public static class SessionConfiguration { private final Integer timeoutMinutes; + private final Boolean timeoutActive; private final List permanentSessions; @ConfigurationInject public SessionConfiguration(@Nullable Integer timeoutMinutes, + Boolean timeoutActive, List permanentSessions) { this.timeoutMinutes = timeoutMinutes; + this.timeoutActive = timeoutActive; this.permanentSessions = permanentSessions; } @@ -154,6 +157,10 @@ public Integer getTimeoutMinutes() { return timeoutMinutes; } + public boolean shouldTimeoutActive() { + return Boolean.TRUE.equals(timeoutActive); + } + public List getPermanentSessions() { return permanentSessions; } diff --git a/server/src/main/resources/application.yml b/server/src/main/resources/application.yml index 7a175171..85d86840 100644 --- a/server/src/main/resources/application.yml +++ b/server/src/main/resources/application.yml @@ -7,6 +7,7 @@ lighter: url: http://lighter.spark:8080 session: timeout-minutes: 90 + timeout-active: false permanent-sessions: [] kubernetes: enabled: false diff --git a/server/src/test/groovy/com/exacaster/lighter/application/sessions/SessionHandlerTest.groovy b/server/src/test/groovy/com/exacaster/lighter/application/sessions/SessionHandlerTest.groovy index 8355c0eb..388ea5ff 100644 --- a/server/src/test/groovy/com/exacaster/lighter/application/sessions/SessionHandlerTest.groovy +++ b/server/src/test/groovy/com/exacaster/lighter/application/sessions/SessionHandlerTest.groovy @@ -59,6 +59,23 @@ class SessionHandlerTest extends Specification { 0 * service.killOne(permanentSession) } + def "preserves active timeouted sessions"() { + given: + def oldSession = newSession() + service.lastUsed(oldSession.id) >> LocalDateTime.now().minusMinutes(conf.sessionConfiguration.timeoutMinutes + 1) + service.isActive(oldSession) >> true + + 1 * service.fetchRunning() >> [ + oldSession, + ] + + when: + handler.handleTimeout() + + then: + 0 * service.killOne(oldSession) + } + def "tracks running"() { given: def session = app() diff --git a/server/src/test/groovy/com/exacaster/lighter/test/Factories.groovy b/server/src/test/groovy/com/exacaster/lighter/test/Factories.groovy index d237ece6..71e551d0 100644 --- a/server/src/test/groovy/com/exacaster/lighter/test/Factories.groovy +++ b/server/src/test/groovy/com/exacaster/lighter/test/Factories.groovy @@ -62,7 +62,8 @@ class Factories { null, 5432, "http://lighter:8080", - new AppConfiguration.SessionConfiguration(20, [new AppConfiguration.PermanentSession("permanentSessionId", submitParams())]), + new AppConfiguration.SessionConfiguration(20, false, + [new AppConfiguration.PermanentSession("permanentSessionId", submitParams())]), ["spark.kubernetes.driverEnv.TEST": "test"], ["spark.kubernetes.driverEnv.TEST": "test"] ) From dd70f1fb09f20d31e66d941d0e0c0a049499e0ea Mon Sep 17 00:00:00 2001 From: Paulius Dambrauskas Date: Mon, 7 Aug 2023 20:28:32 +0300 Subject: [PATCH 2/2] Improve config documentation --- docs/configuration.md | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 2dc7df29..92ed267e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -4,22 +4,22 @@ Lighter can be configured by using environment variables. Currently, Lighter sup ## Global properties -| Property | Description | Default | -|----------------------------------------|-------------------------------------------------------------------------------------------|---------------------------------| -| LIGHTER_MAX_RUNNING_JOBS | Max running Batch jobs in parallel | 5 | -| LIGHTER_MAX_STARTING_JOBS | Max starting Batch jobs in parallel | 5 | -| LIGHTER_SPARK_HISTORY_SERVER_URL | Spark history server URL used on frontend | http://localhost/spark-history/ | -| LIGHTER_EXTERNAL_LOGS_URL_TEMPLATE | Template for link to external logs | | -| LIGHTER_PY_GATEWAY_PORT | Port for live Spark session communication | 25333 | -| LIGHTER_URL | URL which can be used to access Lighter form Spark Job | http://lighter.spark:8080 | -| LIGHTER_SESSION_TIMEOUT_MINUTES | Session lifetime in minutes (from last statement creation). Use negative value to disable | 90 | -| LIGHTER_SESSION_TIMEOUT_ACTIVE | Should lighter kill sessions with waiting statements | false | -| LIGHTER_STORAGE_JDBC_URL | JDBC url for lighter storage | jdbc:h2:mem:lighter | -| LIGHTER_STORAGE_JDBC_USERNAME | JDBC username | sa | -| LIGHTER_STORAGE_JDBC_PASSWORD | JDBC password | | -| LIGHTER_STORAGE_JDBC_DRIVER_CLASS_NAME | JDBC driver class name | org.h2.Driver | -| LIGHTER_BATCH_DEFAULT_CONF | Default `conf` props for batch applications (JSON)* | | -| LIGHTER_SESSION_DEFAULT_CONF | Default `conf` props for session applications (JSON) | | +| Property | Description | Default | +|----------------------------------------|--------------------------------------------------------------------------------------------------------------------|---------------------------------| +| LIGHTER_MAX_RUNNING_JOBS | Max running Batch jobs in parallel | 5 | +| LIGHTER_MAX_STARTING_JOBS | Max starting Batch jobs in parallel | 5 | +| LIGHTER_SPARK_HISTORY_SERVER_URL | Spark history server URL used on frontend | http://localhost/spark-history/ | +| LIGHTER_EXTERNAL_LOGS_URL_TEMPLATE | Template for link to external logs | | +| LIGHTER_PY_GATEWAY_PORT | Port for live Spark session communication | 25333 | +| LIGHTER_URL | URL which can be used to access Lighter form Spark Job | http://lighter.spark:8080 | +| LIGHTER_SESSION_TIMEOUT_MINUTES | Session lifetime in minutes (from last statement creation). Use negative value to disable | 90 | +| LIGHTER_SESSION_TIMEOUT_ACTIVE | Should Lighter kill sessions with waiting statements (obsolete when `LIGHTER_SESSION_TIMEOUT_MINUTES` is negative) | false | +| LIGHTER_STORAGE_JDBC_URL | JDBC url for lighter storage | jdbc:h2:mem:lighter | +| LIGHTER_STORAGE_JDBC_USERNAME | JDBC username | sa | +| LIGHTER_STORAGE_JDBC_PASSWORD | JDBC password | | +| LIGHTER_STORAGE_JDBC_DRIVER_CLASS_NAME | JDBC driver class name | org.h2.Driver | +| LIGHTER_BATCH_DEFAULT_CONF | Default `conf` props for batch applications (JSON)* | | +| LIGHTER_SESSION_DEFAULT_CONF | Default `conf` props for session applications (JSON) | | * default confs will be merged with confs provided in submit request, if property is defined in submit request, default will be ignored. Example of `LIGHTER_BATCH_DEFAULT_CONF`: `{"spark.kubernetes.driverEnv.TEST1":"test1"}`.