This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new f473ef9745 log and object cleanup was not working as expected, fixes
#2796 (#7553)
f473ef9745 is described below
commit f473ef9745deb6d3b8053ef00d62c7fe091fb674
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Jul 17 13:49:57 2026 +0200
log and object cleanup was not working as expected, fixes #2796 (#7553)
---
.../org/apache/hop/core/logging/HopLogStore.java | 14 ++
.../apache/hop/core/logging/LoggingBufferTest.java | 92 +++++++++
.../modules/ROOT/pages/hop-server/index.adoc | 62 +++++-
.../java/org/apache/hop/www/GetStatusServlet.java | 16 +-
.../org/apache/hop/www/HopServerSingleton.java | 107 +++++++++--
.../apache/hop/www/jaxrs/HopServerResource.java | 9 +-
.../org/apache/hop/www/HopServerSingletonTest.java | 209 +++++++++++++++++++++
7 files changed, 480 insertions(+), 29 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/logging/HopLogStore.java
b/core/src/main/java/org/apache/hop/core/logging/HopLogStore.java
index 04f2992824..ffac3de533 100644
--- a/core/src/main/java/org/apache/hop/core/logging/HopLogStore.java
+++ b/core/src/main/java/org/apache/hop/core/logging/HopLogStore.java
@@ -39,6 +39,9 @@ public class HopLogStore {
private Timer logCleanerTimer;
+ /** The log line timeout the cleaner is running with, 0 or lower means:
never clean up lines. */
+ private int maxLogTimeoutMinutes;
+
private static AtomicBoolean initialized = new AtomicBoolean(false);
private static ILogChannelFactory logChannelFactory = new
LogChannelFactory();
@@ -72,6 +75,7 @@ public class HopLogStore {
}
public void replaceLogCleaner(final int maxLogTimeoutMinutes) {
+ this.maxLogTimeoutMinutes = maxLogTimeoutMinutes;
ExecutorUtil.cleanup(logCleanerTimer);
logCleanerTimer = new Timer(true);
@@ -207,6 +211,16 @@ public class HopLogStore {
return getInstance().appender;
}
+ /**
+ * The log line timeout the log cleaner is actually running with, which is
not necessarily the one
+ * that was asked for: a timeout of zero or lower makes it fall back to the
default.
+ *
+ * @return the timeout in minutes, 0 or lower means that log lines are never
cleaned up
+ */
+ public static int getMaxLogTimeoutMinutes() {
+ return getInstance().maxLogTimeoutMinutes;
+ }
+
/**
* Discard all the lines for the specified log channel id AND all the
children.
*
diff --git
a/core/src/test/java/org/apache/hop/core/logging/LoggingBufferTest.java
b/core/src/test/java/org/apache/hop/core/logging/LoggingBufferTest.java
index bcb65ef9f5..86c4dc1469 100644
--- a/core/src/test/java/org/apache/hop/core/logging/LoggingBufferTest.java
+++ b/core/src/test/java/org/apache/hop/core/logging/LoggingBufferTest.java
@@ -18,10 +18,13 @@
package org.apache.hop.core.logging;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
import org.junit.jupiter.api.Test;
/** Unit test for {@link LoggingBuffer} */
@@ -98,6 +101,95 @@ class LoggingBufferTest {
assertEquals(20, loggingBuffer.size());
}
+ /**
+ * The maximum number of lines is kept while lines are added, dropping the
oldest ones. This is
+ * what the max_log_lines setting of a server is meant to limit.
+ */
+ @Test
+ void maxNrLinesDropsTheOldestLines() {
+ LoggingBuffer loggingBuffer = new LoggingBuffer(10);
+
+ for (int i = 0; i < 25; i++) {
+ loggingBuffer.addLogggingEvent(event("line " + i, i));
+ }
+
+ assertEquals(10, loggingBuffer.size(), "No more than the maximum number of
lines is kept");
+ List<String> kept = messagesOf(loggingBuffer);
+ assertTrue(kept.contains("line 24"), "The newest line is kept");
+ assertFalse(kept.contains("line 14"), "The oldest lines are dropped");
+ }
+
+ /** A maximum of zero lines means: no limit, which is what a server defaults
to. */
+ @Test
+ void maxNrLinesOfZeroKeepsEveryLine() {
+ LoggingBuffer loggingBuffer = new LoggingBuffer(0);
+
+ for (int i = 0; i < 100; i++) {
+ loggingBuffer.addLogggingEvent(event("line " + i, i));
+ }
+
+ assertEquals(100, loggingBuffer.size(), "Without a limit every line is
kept");
+ }
+
+ /**
+ * The two limits are independent of each other. Lines are dropped as soon
as there are too many
+ * of them, no matter how recent they are.
+ */
+ @Test
+ void maxNrLinesIsReachedBeforeAnyLineTimesOut() {
+ LoggingBuffer loggingBuffer = new LoggingBuffer(5);
+
+ // Ten lines that all carry the same, recent, timestamp.
+ long now = System.currentTimeMillis();
+ for (int i = 0; i < 10; i++) {
+ loggingBuffer.addLogggingEvent(event("line " + i, now));
+ }
+
+ assertEquals(5, loggingBuffer.size(), "The line limit applies to lines
that did not time out");
+
+ // Nothing times out yet, the lines are all recent.
+ loggingBuffer.removeBufferLinesBefore(now - 60000L);
+ assertEquals(5, loggingBuffer.size());
+ }
+
+ /** And the other way around: lines time out while there is still plenty of
room in the buffer. */
+ @Test
+ void linesTimeOutBeforeMaxNrLinesIsReached() {
+ LoggingBuffer loggingBuffer = new LoggingBuffer(1000);
+
+ long now = System.currentTimeMillis();
+ for (int i = 0; i < 10; i++) {
+ loggingBuffer.addLogggingEvent(event("old line " + i, now - 120000L));
+ }
+ for (int i = 0; i < 10; i++) {
+ loggingBuffer.addLogggingEvent(event("new line " + i, now));
+ }
+ assertEquals(20, loggingBuffer.size(), "The line limit is nowhere near
reached");
+
+ loggingBuffer.removeBufferLinesBefore(now - 60000L);
+
+ assertEquals(10, loggingBuffer.size(), "Only the lines that timed out are
removed");
+ assertTrue(messagesOf(loggingBuffer).contains("new line 0"), "Recent lines
are kept");
+ }
+
+ private static HopLoggingEvent event(String message, long timeStamp) {
+ HopLoggingEvent event = new HopLoggingEvent();
+ // The two argument constructor takes a subject, only this one takes a
message.
+ event.setMessage(new LogMessage(message, "a-log-channel", LogLevel.BASIC));
+ event.setTimeStamp(timeStamp);
+ return event;
+ }
+
+ /** The messages still in the buffer, to check which lines were kept and
which were dropped. */
+ private static List<String> messagesOf(LoggingBuffer loggingBuffer) {
+ List<HopLoggingEvent> events =
+ loggingBuffer.getLogBufferFromTo(
+ (List<String>) null, true, 0, loggingBuffer.getLastBufferLineNr());
+ return events.stream()
+ .map(event -> ((LogMessage) event.getMessage()).getMessage())
+ .collect(Collectors.toList());
+ }
+
@Test
void testRemoveChannelFromBuffer() {
String logChannelId = "1";
diff --git a/docs/hop-user-manual/modules/ROOT/pages/hop-server/index.adoc
b/docs/hop-user-manual/modules/ROOT/pages/hop-server/index.adoc
index 15ccfc759a..9ea54d5c1d 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/hop-server/index.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/hop-server/index.adoc
@@ -245,17 +245,21 @@ The syntax of this configuration file is straightforward.
Hostname and port are
<joining>true</joining>
<!-- The maximum number of log lines kept in memory by the server.
- The default is 0 which means: keep all lines
+ Leave this at 0 or leave the element out to use the
HOP_MAX_LOG_SIZE_IN_LINES variable,
+ which keeps all log lines by default.
-->
<max_log_lines>0</max_log_lines>
<!-- The time (in minutes) it takes for a log line to be cleaned up in
memory.
- The default is 0 which means: never clean up log lines
+ Leave this at 0 or leave the element out to use the
HOP_MAX_LOG_TIMEOUT_IN_MINUTES variable,
+ which cleans up log lines after 1440 minutes (one day) by default.
-->
<max_log_timeout_minutes>1440</max_log_timeout_minutes>
- <!-- The time (in minutes) it takes for a pipeline or workflow execution to
be removed from the server status.
- The default is 0 which means: never clean executions
+ <!-- The time (in minutes) it takes for a finished or stopped pipeline or
workflow execution to be
+ removed from the server status.
+ Leave this at 0 or leave the element out to use the
HOP_SERVER_OBJECT_TIMEOUT_MINUTES
+ variable, which removes executions after 1440 minutes (one day) by
default.
-->
<object_timeout_minutes>1440</object_timeout_minutes>
@@ -281,6 +285,56 @@ The syntax of this configuration file is straightforward.
Hostname and port are
</hop-server-config>
----
+==== Cleanup settings and where they come from
+
+`max_log_lines`, `max_log_timeout_minutes` and `object_timeout_minutes` keep
the memory use of a
+long running server in check. Each of them is resolved separately, and the
first of these that
+supplies a value wins:
+
+. the configuration file, when the element is set to a value larger than 0
+. the matching Java system property, for example
`-DHOP_SERVER_OBJECT_TIMEOUT_MINUTES=60`
+. the variable in `hop-config.json`
+. the built-in default
+
+[cols="1,1,1", options="header"]
+|===
+|Configuration element |Variable |Default
+
+|`max_log_lines`
+|`HOP_MAX_LOG_SIZE_IN_LINES`
+|`0`: every log line is kept
+
+|`max_log_timeout_minutes`
+|`HOP_MAX_LOG_TIMEOUT_IN_MINUTES`
+|`1440`: log lines are cleaned up after one day
+
+|`object_timeout_minutes`
+|`HOP_SERVER_OBJECT_TIMEOUT_MINUTES`
+|`1440`: finished and stopped executions are removed from the server status
after one day
+|===
+
+A server that is started without a configuration file, or without these
elements, therefore keeps
+all of its log lines, cleans them up after a day, and removes finished
executions from its status
+after a day. The `Configuration details` section of the status page reports
the values the server
+is actually running with.
+
+WARNING: The default keeps *every* log line of *every* execution for a day. A
server that runs a lot
+of short executions can run out of memory that way. Set `max_log_lines` to
limit this.
+
+[NOTE]
+====
+These three settings are read from Java system properties, not from the
environment of the process.
+Setting an environment variable on a container, for example `-e
HOP_MAX_LOG_SIZE_IN_LINES=5000`,
+therefore has no effect: the value from `hop-config.json` is used instead. To
override them in a
+container, pass them as system properties through `HOP_OPTIONS`:
+
+[source,shell]
+docker run -e HOP_OPTIONS="-Xmx2048m -DHOP_MAX_LOG_SIZE_IN_LINES=5000" ...
+
+Mind that `HOP_OPTIONS` replaces the default `-Xmx2048m`, so repeat the memory
settings you need.
+Alternatively, set the elements in the server configuration file, which takes
precedence over both.
+====
+
Example startup commands with a configuration file are: +
[tabs]
diff --git a/engine/src/main/java/org/apache/hop/www/GetStatusServlet.java
b/engine/src/main/java/org/apache/hop/www/GetStatusServlet.java
index ab46bb3574..fcc7a3b55b 100644
--- a/engine/src/main/java/org/apache/hop/www/GetStatusServlet.java
+++ b/engine/src/main/java/org/apache/hop/www/GetStatusServlet.java
@@ -35,6 +35,7 @@ import org.apache.hop.core.Const;
import org.apache.hop.core.annotations.HopServerServlet;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.json.HopJson;
+import org.apache.hop.core.logging.HopLogStore;
import org.apache.hop.core.xml.XmlHandler;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.pipeline.PipelineMeta;
@@ -662,11 +663,12 @@ public class GetStatusServlet extends BaseHttpServlet
implements IHopServerPlugi
HopServerConfig serverConfig = getPipelineMap().getHopServerConfig();
if (serverConfig != null) {
String maxLines = "";
- if (serverConfig.getMaxLogLines() == 0) {
+ if (HopLogStore.getAppender().getMaxNrLines() == 0) {
maxLines = BaseMessages.getString(PKG, CONST_NO_LIMIT);
} else {
maxLines =
- serverConfig.getMaxLogLines() + BaseMessages.getString(PKG,
"GetStatusServlet.Lines");
+ HopLogStore.getAppender().getMaxNrLines()
+ + BaseMessages.getString(PKG, "GetStatusServlet.Lines");
}
out.print(
CONST_TABLE_ROW
@@ -678,11 +680,11 @@ public class GetStatusServlet extends BaseHttpServlet
implements IHopServerPlugi
// The max age of log lines
//
String maxAge = "";
- if (serverConfig.getMaxLogTimeoutMinutes() == 0) {
+ if (HopLogStore.getMaxLogTimeoutMinutes() == 0) {
maxAge = BaseMessages.getString(PKG, CONST_NO_LIMIT);
} else {
maxAge =
- serverConfig.getMaxLogTimeoutMinutes()
+ HopLogStore.getMaxLogTimeoutMinutes()
+ BaseMessages.getString(PKG, "GetStatusServlet.Minutes");
}
out.print(
@@ -694,13 +696,13 @@ public class GetStatusServlet extends BaseHttpServlet
implements IHopServerPlugi
// The max age of stale objects
//
+ int objectTimeoutMinutes =
HopServerSingleton.determineObjectTimeoutMinutes(serverConfig);
String maxObjAge = "";
- if (serverConfig.getObjectTimeoutMinutes() == 0) {
+ if (objectTimeoutMinutes == 0) {
maxObjAge = BaseMessages.getString(PKG, CONST_NO_LIMIT);
} else {
maxObjAge =
- serverConfig.getObjectTimeoutMinutes()
- + BaseMessages.getString(PKG, "GetStatusServlet.Minutes");
+ objectTimeoutMinutes + BaseMessages.getString(PKG,
"GetStatusServlet.Minutes");
}
out.print(
CONST_TABLE_ROW
diff --git a/engine/src/main/java/org/apache/hop/www/HopServerSingleton.java
b/engine/src/main/java/org/apache/hop/www/HopServerSingleton.java
index 8df83f13d7..aae1cdb346 100644
--- a/engine/src/main/java/org/apache/hop/www/HopServerSingleton.java
+++ b/engine/src/main/java/org/apache/hop/www/HopServerSingleton.java
@@ -33,7 +33,6 @@ import org.apache.hop.core.logging.LoggingObjectType;
import org.apache.hop.core.logging.LoggingRegistry;
import org.apache.hop.core.logging.SimpleLoggingObject;
import org.apache.hop.core.util.EnvUtil;
-import org.apache.hop.core.util.ExecutorUtil;
import org.apache.hop.core.util.Utils;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.pipeline.PipelineMeta;
@@ -64,7 +63,7 @@ public class HopServerSingleton {
private HopServerSingleton(HopServerConfig config) throws HopException {
HopEnvironment.init();
- HopLogStore.init();
+ configureLogStore(config);
this.log = new LogChannel("HopServer");
pipelineMap = new PipelineMap();
@@ -90,24 +89,103 @@ public class HopServerSingleton {
}
}
+ /** How often the server looks for stale objects to purge, in milliseconds.
*/
+ private static final long PURGE_INTERVAL_MS = 20000L;
+
+ /** How long a stale object is kept when neither the configuration nor the
variable says so. */
+ private static final int DEFAULT_OBJECT_TIMEOUT_MINUTES = 24 * 60;
+
+ /**
+ * Hand the log line limits of the server over to the log store. Without
this the max_log_lines
+ * and max_log_timeout_minutes of the server configuration are read and
reported, but never
+ * applied, and the log store keeps running with its own defaults. See issue
#2796.
+ *
+ * <p>A limit of zero or lower keeps the default of the log store, the way
it has always behaved
+ * for a server that does not configure one.
+ */
+ static void configureLogStore(final HopServerConfig config) {
+ // Both limits are resolved separately on purpose. Handing the log store
the two configured
+ // values as they are would make it take both from the configuration as
soon as one of them is
+ // set, so configuring only a line limit would leave the log lines without
a timeout.
+ //
+ HopLogStore.init(
+ determineMaxLogLines(config),
+ determineMaxLogTimeoutMinutes(config),
+ EnvUtil.getSystemProperty(Const.HOP_REDIRECT_STDOUT,
"N").equalsIgnoreCase("Y"),
+ EnvUtil.getSystemProperty(Const.HOP_REDIRECT_STDERR,
"N").equalsIgnoreCase("Y"));
+ }
+
+ /**
+ * The number of log lines a server actually keeps: the configuration file
takes precedence over
+ * the variable, which in turn takes precedence over the default.
+ *
+ * @param config the configuration of the server
+ * @return the number of lines, 0 or lower means that every line is kept
+ */
+ public static int determineMaxLogLines(final HopServerConfig config) {
+ if (config.getMaxLogLines() > 0) {
+ return config.getMaxLogLines();
+ }
+ return Const.toInt(
+ EnvUtil.getSystemProperty(Const.HOP_MAX_LOG_SIZE_IN_LINES),
Const.MAX_NR_LOG_LINES);
+ }
+
+ /**
+ * The age a server actually lets its log lines reach: the configuration
file takes precedence
+ * over the variable, which in turn takes precedence over the default.
+ *
+ * @param config the configuration of the server
+ * @return the timeout in minutes, 0 or lower means that log lines are never
cleaned up
+ */
+ public static int determineMaxLogTimeoutMinutes(final HopServerConfig
config) {
+ if (config.getMaxLogTimeoutMinutes() > 0) {
+ return config.getMaxLogTimeoutMinutes();
+ }
+ return Const.toInt(
+ EnvUtil.getSystemProperty(Const.HOP_MAX_LOG_TIMEOUT_IN_MINUTES),
+ Const.MAX_LOG_LINE_TIMEOUT_MINUTES);
+ }
+
public static void installPurgeTimer(
final HopServerConfig config,
final ILogChannel log,
final PipelineMap pipelineMap,
final WorkflowMap workflowMap) {
+ installPurgeTimer(config, log, pipelineMap, workflowMap,
PURGE_INTERVAL_MS);
+ }
- final int objectTimeout;
- String systemTimeout =
EnvUtil.getSystemProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES, null);
-
- // The value specified in XML takes precedence over the environment
variable!
- //
+ /**
+ * The timeout a server actually purges stale objects with: the
configuration file takes
+ * precedence over the variable, which in turn takes precedence over the
default of one day. A
+ * server that configures nothing still purges, so this never resolves to
zero unless it was asked
+ * for.
+ *
+ * @param config the configuration of the server
+ * @return the timeout in minutes, 0 or lower means that objects are never
purged
+ */
+ public static int determineObjectTimeoutMinutes(final HopServerConfig
config) {
if (config.getObjectTimeoutMinutes() > 0) {
- objectTimeout = config.getObjectTimeoutMinutes();
- } else if (!Utils.isEmpty(systemTimeout)) {
- objectTimeout = Const.toInt(systemTimeout, 1440);
- } else {
- objectTimeout = 24 * 60; // 1440 : default is a one day time-out
+ return config.getObjectTimeoutMinutes();
+ }
+ String systemTimeout =
EnvUtil.getSystemProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES, null);
+ if (!Utils.isEmpty(systemTimeout)) {
+ return Const.toInt(systemTimeout, DEFAULT_OBJECT_TIMEOUT_MINUTES);
}
+ return DEFAULT_OBJECT_TIMEOUT_MINUTES;
+ }
+
+ /**
+ * @param purgeIntervalMs how often to look for stale objects, so that a
test does not have to
+ * wait for the interval the server itself uses
+ */
+ static void installPurgeTimer(
+ final HopServerConfig config,
+ final ILogChannel log,
+ final PipelineMap pipelineMap,
+ final WorkflowMap workflowMap,
+ final long purgeIntervalMs) {
+
+ final int objectTimeout = determineObjectTimeoutMinutes(config);
// If we need to time out finished or idle objects, we should create a
timer
// in the background to clean
@@ -203,7 +281,6 @@ public class HopServerSingleton {
+ " from "
+ workflow.getExecutionStartDate());
}
- ExecutorUtil.cleanup(timer, 1);
}
}
@@ -214,9 +291,9 @@ public class HopServerSingleton {
}
};
- // Search for stale objects every 20 seconds:
+ // Search for stale objects at every interval:
//
- timer.schedule(timerTask, 20000, 20000);
+ timer.schedule(timerTask, purgeIntervalMs, purgeIntervalMs);
}
}
diff --git
a/engine/src/main/java/org/apache/hop/www/jaxrs/HopServerResource.java
b/engine/src/main/java/org/apache/hop/www/jaxrs/HopServerResource.java
index 647c81668e..903dbf10a0 100644
--- a/engine/src/main/java/org/apache/hop/www/jaxrs/HopServerResource.java
+++ b/engine/src/main/java/org/apache/hop/www/jaxrs/HopServerResource.java
@@ -23,6 +23,7 @@ import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import java.util.ArrayList;
import java.util.List;
+import org.apache.hop.core.logging.HopLogStore;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.engine.IPipelineEngine;
import org.apache.hop.workflow.WorkflowMeta;
@@ -88,9 +89,11 @@ public class HopServerResource {
HopServerConfig serverConfig =
HopServerSingleton.getInstance().getPipelineMap().getHopServerConfig();
List<NVPair> list = new ArrayList<>();
- list.add(new NVPair("maxLogLines", "" + serverConfig.getMaxLogLines()));
- list.add(new NVPair("maxLogLinesAge", "" +
serverConfig.getMaxLogTimeoutMinutes()));
- list.add(new NVPair("maxObjectsAge", "" +
serverConfig.getObjectTimeoutMinutes()));
+ list.add(new NVPair("maxLogLines", "" +
HopLogStore.getAppender().getMaxNrLines()));
+ list.add(new NVPair("maxLogLinesAge", "" +
HopLogStore.getMaxLogTimeoutMinutes()));
+ list.add(
+ new NVPair(
+ "maxObjectsAge", "" +
HopServerSingleton.determineObjectTimeoutMinutes(serverConfig)));
list.add(new NVPair("configFile", "" + serverConfig.getFilename()));
return list;
}
diff --git
a/engine/src/test/java/org/apache/hop/www/HopServerSingletonTest.java
b/engine/src/test/java/org/apache/hop/www/HopServerSingletonTest.java
index 36fd9a8f48..a9556ad7bc 100644
--- a/engine/src/test/java/org/apache/hop/www/HopServerSingletonTest.java
+++ b/engine/src/test/java/org/apache/hop/www/HopServerSingletonTest.java
@@ -17,15 +17,25 @@
package org.apache.hop.www;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.contains;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import java.util.Date;
+import java.util.function.BooleanSupplier;
import org.apache.hop.core.Const;
+import org.apache.hop.core.logging.HopLogStore;
import org.apache.hop.core.logging.ILogChannel;
import org.apache.hop.junit.rules.RestoreHopEngineEnvironmentExtension;
+import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.engine.IPipelineEngine;
+import org.apache.hop.workflow.WorkflowConfiguration;
+import org.apache.hop.workflow.WorkflowMeta;
+import org.apache.hop.workflow.engine.IWorkflowEngine;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -55,4 +65,203 @@ class HopServerSingletonTest {
verify(log).logBasic(contains("Installing timer"));
}
+
+ /**
+ * The log line limits of the server configuration have to reach the log
store, or they are read
+ * and reported but never applied. See issue #2796.
+ */
+ @Test
+ void logStoreUsesTheConfiguredLogLimits() {
+ HopServerConfig config = new HopServerConfig();
+ config.setMaxLogLines(123);
+ config.setMaxLogTimeoutMinutes(7);
+
+ HopServerSingleton.configureLogStore(config);
+
+ assertEquals(123, HopLogStore.getAppender().getMaxNrLines());
+ assertEquals(7, HopLogStore.getMaxLogTimeoutMinutes());
+ }
+
+ /**
+ * The two log limits are configured independently: setting one of them may
not silently drop the
+ * other back to "no limit". See issue #2796.
+ */
+ @Test
+ void configuringOnlyTheLineLimitKeepsTheLogTimeout() {
+ HopLogStore.init();
+ int defaultMaxLogTimeoutMinutes = HopLogStore.getMaxLogTimeoutMinutes();
+
+ HopServerConfig config = new HopServerConfig();
+ config.setMaxLogLines(1000);
+
+ HopServerSingleton.configureLogStore(config);
+
+ assertEquals(1000, HopLogStore.getAppender().getMaxNrLines());
+ assertEquals(
+ defaultMaxLogTimeoutMinutes,
+ HopLogStore.getMaxLogTimeoutMinutes(),
+ "Log lines should still time out when only a line limit is
configured");
+ }
+
+ /** The variable is used for the log limits when the configuration file does
not say otherwise. */
+ @Test
+ void logLimitsUseTheVariablesWithoutConfiguration() {
+ System.setProperty(Const.HOP_MAX_LOG_SIZE_IN_LINES, "321");
+ System.setProperty(Const.HOP_MAX_LOG_TIMEOUT_IN_MINUTES, "77");
+ try {
+ HopServerConfig config = new HopServerConfig();
+
+ assertEquals(321, HopServerSingleton.determineMaxLogLines(config));
+ assertEquals(77,
HopServerSingleton.determineMaxLogTimeoutMinutes(config));
+
+ // ... and the configuration file wins over the variable.
+ config.setMaxLogLines(10);
+ config.setMaxLogTimeoutMinutes(20);
+ assertEquals(10, HopServerSingleton.determineMaxLogLines(config));
+ assertEquals(20,
HopServerSingleton.determineMaxLogTimeoutMinutes(config));
+ } finally {
+ System.clearProperty(Const.HOP_MAX_LOG_SIZE_IN_LINES);
+ System.clearProperty(Const.HOP_MAX_LOG_TIMEOUT_IN_MINUTES);
+ }
+ }
+
+ /**
+ * A server that configures no limits keeps the defaults of the log store,
the way it behaved
+ * before the configured limits were passed on.
+ */
+ @Test
+ void logStoreKeepsItsDefaultsWithoutConfiguredLogLimits() {
+ HopLogStore.init();
+ int defaultMaxLogLines = HopLogStore.getAppender().getMaxNrLines();
+ int defaultMaxLogTimeoutMinutes = HopLogStore.getMaxLogTimeoutMinutes();
+
+ HopServerSingleton.configureLogStore(new HopServerConfig());
+
+ assertEquals(defaultMaxLogLines,
HopLogStore.getAppender().getMaxNrLines());
+ assertEquals(defaultMaxLogTimeoutMinutes,
HopLogStore.getMaxLogTimeoutMinutes());
+ }
+
+ /**
+ * A server that configures no object timeout still purges after a day. The
status page reports
+ * this value, so it may not claim that stale objects are never cleaned up.
See issue #2796.
+ */
+ @Test
+ void objectTimeoutFallsBackToADayWithoutConfigurationOrVariable() {
+ System.clearProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES);
+
+ assertEquals(1440, HopServerSingleton.determineObjectTimeoutMinutes(new
HopServerConfig()));
+ }
+
+ /** The variable is used when the configuration file does not say otherwise.
*/
+ @Test
+ void objectTimeoutUsesTheVariableWithoutConfiguration() {
+ System.setProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES, "60");
+ try {
+ assertEquals(60, HopServerSingleton.determineObjectTimeoutMinutes(new
HopServerConfig()));
+ } finally {
+ System.clearProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES);
+ }
+ }
+
+ /** The configuration file takes precedence over the variable. */
+ @Test
+ void objectTimeoutFromConfigurationBeatsTheVariable() {
+ System.setProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES, "60");
+ try {
+ HopServerConfig config = new HopServerConfig();
+ config.setObjectTimeoutMinutes(15);
+
+ assertEquals(15,
HopServerSingleton.determineObjectTimeoutMinutes(config));
+ } finally {
+ System.clearProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES);
+ }
+ }
+
+ /** A pipeline that ran longer ago than the object timeout is purged. */
+ @Test
+ void purgeTimerRemovesStalePipeline() throws Exception {
+ PipelineMap pipelineMap = new PipelineMap();
+ pipelineMap.addPipeline("stale", "p1", finishedPipeline(minutesAgo(5)),
null);
+
+ installPurgeTimer(pipelineMap, new WorkflowMap());
+
+ waitUntil(() -> pipelineMap.getPipelineObjects().isEmpty());
+ assertTrue(pipelineMap.getPipelineObjects().isEmpty(), "The stale pipeline
should be purged");
+ }
+
+ /** A pipeline that ran more recently than the object timeout is kept. */
+ @Test
+ void purgeTimerKeepsRecentPipeline() throws Exception {
+ PipelineMap pipelineMap = new PipelineMap();
+ pipelineMap.addPipeline("recent", "p1", finishedPipeline(new Date()),
null);
+
+ installPurgeTimer(pipelineMap, new WorkflowMap());
+ Thread.sleep(500);
+
+ assertEquals(1, pipelineMap.getPipelineObjects().size(), "The recent
pipeline should be kept");
+ }
+
+ /**
+ * The server has to keep purging for as long as it runs. A finished
workflow that is not stale
+ * enough to be purged used to stop the timer, after which nothing was ever
cleaned up again and
+ * the memory of the server kept growing. See issue #2796.
+ */
+ @Test
+ void purgeTimerKeepsPurgingAfterSeeingAFinishedWorkflow() throws Exception {
+ PipelineMap pipelineMap = new PipelineMap();
+ WorkflowMap workflowMap = new WorkflowMap();
+
+ // A workflow that finished just now, so it is not stale enough to be
purged.
+ workflowMap.addWorkflow(
+ "recent", "w1", finishedWorkflow(new Date()),
mock(WorkflowConfiguration.class));
+
+ installPurgeTimer(pipelineMap, workflowMap);
+
+ // Give the timer the time to run over the workflow at least once.
+ Thread.sleep(500);
+
+ // A pipeline that is stale enough to be purged shows up afterwards.
+ pipelineMap.addPipeline("stale", "p1", finishedPipeline(minutesAgo(5)),
null);
+
+ waitUntil(() -> pipelineMap.getPipelineObjects().isEmpty());
+ assertTrue(
+ pipelineMap.getPipelineObjects().isEmpty(),
+ "The stale pipeline should still be purged after a finished workflow
was seen");
+ }
+
+ private void installPurgeTimer(PipelineMap pipelineMap, WorkflowMap
workflowMap) {
+ HopServerConfig config = mock(HopServerConfig.class);
+ when(config.getObjectTimeoutMinutes()).thenReturn(1);
+ System.clearProperty(Const.HOP_SERVER_OBJECT_TIMEOUT_MINUTES);
+
+ HopServerSingleton.installPurgeTimer(
+ config, mock(ILogChannel.class), pipelineMap, workflowMap, 50L);
+ }
+
+ private static Date minutesAgo(int minutes) {
+ return new Date(System.currentTimeMillis() - minutes * 60_000L);
+ }
+
+ private IPipelineEngine<PipelineMeta> finishedPipeline(Date
executionStartDate) {
+ IPipelineEngine<PipelineMeta> pipeline = mock(IPipelineEngine.class);
+ when(pipeline.isFinished()).thenReturn(true);
+ when(pipeline.getExecutionStartDate()).thenReturn(executionStartDate);
+ when(pipeline.getLogChannelId()).thenReturn("pipeline-log-channel");
+ return pipeline;
+ }
+
+ private IWorkflowEngine<WorkflowMeta> finishedWorkflow(Date
executionStartDate) {
+ IWorkflowEngine<WorkflowMeta> workflow = mock(IWorkflowEngine.class);
+ when(workflow.isFinished()).thenReturn(true);
+ when(workflow.getExecutionStartDate()).thenReturn(executionStartDate);
+ when(workflow.getLogChannelId()).thenReturn("workflow-log-channel");
+ return workflow;
+ }
+
+ private void waitUntil(BooleanSupplier condition) throws
InterruptedException {
+ long deadline = System.currentTimeMillis() + 5000L;
+ while (System.currentTimeMillis() < deadline && !condition.getAsBoolean())
{
+ Thread.sleep(50);
+ }
+ }
}
