Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR for #4693 - sessionList is very slow v7.5 on a cluster #4699

Merged
merged 3 commits into from
Jan 15, 2025
Merged

PR for #4693 - sessionList is very slow v7.5 on a cluster #4699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10cc5a7
Empty initial commit on gh-4693-session-leak [no ci]
at055612 Jan 15, 2025
ea64c6b
gh-4693 Add session list debug logging
at055612 Jan 15, 2025
21d5e01
gh-4693 Add maxInactiveInterval to control session expiry
at055612 Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 58 additions & 34 deletions stroom-app/src/main/java/stroom/app/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import stroom.config.app.AppConfig;
import stroom.config.app.Config;
import stroom.config.app.SecurityConfig;
import stroom.config.app.SessionConfig;
import stroom.config.app.SessionCookieConfig;
import stroom.config.app.StroomYamlUtil;
import stroom.config.global.impl.ConfigMapper;
import stroom.dropwizard.common.AdminServlets;
Expand Down Expand Up @@ -54,7 +56,9 @@
import stroom.util.logging.LambdaLoggerFactory;
import stroom.util.logging.LogUtil;
import stroom.util.shared.AbstractConfig;
import stroom.util.shared.ModelStringUtil;
import stroom.util.shared.ResourcePaths;
import stroom.util.time.StroomDuration;
import stroom.util.validation.ValidationModule;
import stroom.util.yaml.YamlUtil;

Expand All @@ -70,14 +74,14 @@
import jakarta.inject.Inject;
import jakarta.servlet.DispatcherType;
import jakarta.servlet.FilterRegistration;
import jakarta.servlet.SessionCookieConfig;
import jakarta.validation.ValidatorFactory;
import org.eclipse.jetty.server.session.SessionHandler;
import org.eclipse.jetty.servlets.CrossOriginFilter;

import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.EnumSet;
import java.util.Objects;

Expand Down Expand Up @@ -220,15 +224,14 @@ public void run(final Config configuration, final Environment environment) {
// We want Stroom to use the root path so we need to move Dropwizard's path.
environment.jersey().setUrlPattern(ResourcePaths.API_ROOT_PATH + "/*");

// Set up a session handler for Jetty
configureSessionHandling(environment);

// Ensure the session cookie that provides JSESSIONID is secure.
// Need to get it from ConfigMapper not AppConfig as ConfigMapper is now the source of
// truth for config.
// Need to get these config classed from ConfigMapper as the main appInjector is not created yet
// and configuration only holds the YAML view of the config, not the DB view.
final ConfigMapper configMapper = bootStrapInjector.getInstance(ConfigMapper.class);
final stroom.config.app.SessionCookieConfig sessionCookieConfig = configMapper.getConfigObject(
stroom.config.app.SessionCookieConfig.class);
final SessionCookieConfig sessionCookieConfig = configMapper.getConfigObject(SessionCookieConfig.class);
final SessionConfig sessionConfig = configMapper.getConfigObject(SessionConfig.class);

// Set up a session handler for Jetty
configureSessionHandling(environment, sessionConfig);
configureSessionCookie(environment, sessionCookieConfig);

// Configure Cross-Origin Resource Sharing.
Expand Down Expand Up @@ -273,11 +276,11 @@ public void run(final Config configuration, final Environment environment) {

private void showNodeInfo(final Config configuration) {
LOGGER.info(""
+ "\n********************************************************************************"
+ "\n Stroom home: " + homeDirProvider.get().toAbsolutePath().normalize()
+ "\n Stroom temp: " + tempDirProvider.get().toAbsolutePath().normalize()
+ "\n Node name: " + getNodeName(configuration.getYamlAppConfig())
+ "\n********************************************************************************");
+ "\n********************************************************************************"
+ "\n Stroom home: " + homeDirProvider.get().toAbsolutePath().normalize()
+ "\n Stroom temp: " + tempDirProvider.get().toAbsolutePath().normalize()
+ "\n Node name: " + getNodeName(configuration.getYamlAppConfig())
+ "\n********************************************************************************");
}

private void warnAboutDefaultOpenIdCreds(final Config configuration, final Injector injector) {
Expand All @@ -299,17 +302,17 @@ private void warnAboutDefaultOpenIdCreds(final Config configuration, final Injec
.getFullPathStr(AbstractOpenIdConfig.PROP_NAME_IDP_TYPE);

LOGGER.warn("\n" +
"\n -----------------------------------------------------------------------------" +
"\n " +
"\n WARNING!" +
"\n " +
"\n Using default and publicly available Open ID authentication credentials. " +
"\n This is insecure! These should only be used in test/demo environments. " +
"\n Set " + propPath + " to INTERNAL_IDP/EXTERNAL_IDP for production environments." +
"\n" +
"\n " + defaultOpenIdCredentials.getApiKey() +
"\n -----------------------------------------------------------------------------" +
"\n");
"\n -----------------------------------------------------------------------------" +
"\n " +
"\n WARNING!" +
"\n " +
"\n Using default and publicly available Open ID authentication credentials. " +
"\n This is insecure! These should only be used in test/demo environments. " +
"\n Set " + propPath + " to INTERNAL_IDP/EXTERNAL_IDP for production environments." +
"\n" +
"\n " + defaultOpenIdCredentials.getApiKey() +
"\n -----------------------------------------------------------------------------" +
"\n");
}
}

Expand Down Expand Up @@ -344,35 +347,56 @@ private void validateAppConfig(final Config config, final Path configFile) {

if (result.hasErrors() && appConfig.isHaltBootOnConfigValidationFailure()) {
LOGGER.error("Application configuration is invalid. Stopping Stroom. To run Stroom with invalid " +
"configuration, set {} to false, however this is not advised!",
"configuration, set {} to false, however this is not advised!",
appConfig.getFullPathStr(AppConfig.PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE));
System.exit(1);
}
}

private static void configureSessionHandling(final Environment environment) {
SessionHandler sessionHandler = new SessionHandler();
private void configureSessionHandling(final Environment environment,
final SessionConfig sessionConfig) {

final SessionHandler sessionHandler = new SessionHandler();
// We need to give our session cookie a name other than JSESSIONID, otherwise it might
// clash with other services running on the same domain.
sessionHandler.setSessionCookie(SESSION_COOKIE_NAME);
long maxInactiveIntervalSecs = NullSafe.getOrElse(
sessionConfig.getMaxInactiveInterval(),
StroomDuration::getDuration,
Duration::toSeconds,
-1L);
if (maxInactiveIntervalSecs > Integer.MAX_VALUE) {
maxInactiveIntervalSecs = -1;
}
LOGGER.info("Setting session maxInactiveInterval to {} secs ({})",
ModelStringUtil.formatCsv(maxInactiveIntervalSecs),
(maxInactiveIntervalSecs > 0
? Duration.ofSeconds(maxInactiveIntervalSecs).toString()
: String.valueOf(maxInactiveIntervalSecs)));

// If we don't let sessions expire then the Map of HttpSession in SessionListListener
// will grow and grow
sessionHandler.setMaxInactiveInterval((int) maxInactiveIntervalSecs);

environment.servlets().setSessionHandler(sessionHandler);
environment.jersey().register(SessionFactoryProvider.class);
}

private static void configureSessionCookie(final Environment environment,
final stroom.config.app.SessionCookieConfig config) {
private void configureSessionCookie(final Environment environment,
final SessionCookieConfig sessionCookieConfig) {
// Ensure the session cookie that provides JSESSIONID is secure.
final SessionCookieConfig sessionCookieConfig = environment
final jakarta.servlet.SessionCookieConfig servletSessionCookieConfig = environment
.getApplicationContext()
.getServletContext()
.getSessionCookieConfig();
sessionCookieConfig.setSecure(config.isSecure());
sessionCookieConfig.setHttpOnly(config.isHttpOnly());
servletSessionCookieConfig.setSecure(sessionCookieConfig.isSecure());
servletSessionCookieConfig.setHttpOnly(sessionCookieConfig.isHttpOnly());
// TODO : Add `SameSite=Strict` when supported by JEE
}

private static void configureCors(io.dropwizard.core.setup.Environment environment) {
FilterRegistration.Dynamic cors = environment.servlets().addFilter("CORS", CrossOriginFilter.class);
final FilterRegistration.Dynamic cors = environment.servlets()
.addFilter("CORS", CrossOriginFilter.class);
cors.addMappingForUrlPatterns(EnumSet.allOf(DispatcherType.class), true, "/*");
cors.setInitParameter(CrossOriginFilter.ALLOWED_METHODS_PARAM, "GET,PUT,POST,DELETE,OPTIONS,PATCH");
cors.setInitParameter(CrossOriginFilter.ALLOWED_ORIGINS_PARAM, "*");
Expand Down
32 changes: 22 additions & 10 deletions stroom-config/stroom-config-app/src/main/java/stroom/config/app/AppConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public class AppConfig extends AbstractConfig implements IsStroomConfig {
public static final String PROP_NAME_SECURITY = "security";
public static final String PROP_NAME_SERVICE_DISCOVERY = "serviceDiscovery";
public static final String PROP_NAME_SESSION_COOKIE = "sessionCookie";
public static final String PROP_NAME_SESSION = "session";
public static final String PROP_NAME_SOLR = "solr";
public static final String PROP_NAME_STATE = "state";
public static final String PROP_NAME_STATISTICS = "statistics";
Expand Down Expand Up @@ -161,6 +162,7 @@ public class AppConfig extends AbstractConfig implements IsStroomConfig {
private final SecurityConfig securityConfig;
private final ServiceDiscoveryConfig serviceDiscoveryConfig;
private final SessionCookieConfig sessionCookieConfig;
private final SessionConfig sessionConfig;
private final SolrConfig solrConfig;
private final StateConfig stateConfig;
private final StatisticsConfig statisticsConfig;
Expand Down Expand Up @@ -211,6 +213,7 @@ public AppConfig() {
new SecurityConfig(),
new ServiceDiscoveryConfig(),
new SessionCookieConfig(),
new SessionConfig(),
new SolrConfig(),
new StateConfig(),
new StatisticsConfig(),
Expand Down Expand Up @@ -260,6 +263,7 @@ public AppConfig(@JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE)
@JsonProperty(PROP_NAME_SECURITY) final SecurityConfig securityConfig,
@JsonProperty(PROP_NAME_SERVICE_DISCOVERY) final ServiceDiscoveryConfig serviceDiscoveryConfig,
@JsonProperty(PROP_NAME_SESSION_COOKIE) final SessionCookieConfig sessionCookieConfig,
@JsonProperty(PROP_NAME_SESSION) final SessionConfig sessionConfig,
@JsonProperty(PROP_NAME_SOLR) final SolrConfig solrConfig,
@JsonProperty(PROP_NAME_STATE) final StateConfig stateConfig,
@JsonProperty(PROP_NAME_STATISTICS) final StatisticsConfig statisticsConfig,
Expand Down Expand Up @@ -305,6 +309,7 @@ public AppConfig(@JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE)
this.securityConfig = securityConfig;
this.serviceDiscoveryConfig = serviceDiscoveryConfig;
this.sessionCookieConfig = sessionCookieConfig;
this.sessionConfig = sessionConfig;
this.solrConfig = solrConfig;
this.stateConfig = stateConfig;
this.statisticsConfig = statisticsConfig;
Expand All @@ -317,11 +322,12 @@ public AppConfig(@JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE)

@AssertTrue(
message = "stroom." + PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE + " is set to false. If there is " +
"invalid configuration the system may behave in unexpected ways. This setting is not advised.",
"invalid configuration the system may behave in unexpected ways. This setting is not advised.",
payload = ValidationSeverity.Warning.class)
@JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE)
@JsonPropertyDescription("If true, Stroom will halt on start up if any errors are found in the YAML " +
"configuration file. If false, the errors will simply be logged. Setting this to false is not advised.")
"configuration file. If false, the errors will simply be logged." +
"Setting this to false is not advised.")
public boolean isHaltBootOnConfigValidationFailure() {
return haltBootOnConfigValidationFailure;
}
Expand Down Expand Up @@ -363,8 +369,9 @@ public ClusterLockConfig getClusterLockConfig() {

@JsonProperty(PROP_NAME_COMMON_DB_DETAILS)
@JsonPropertyDescription("Defines a set of common database connection details to use if no connection details " +
"are defined for a service area in stroom, e.g. core or config. This means you can have all service " +
"areas running in a single database, have each in their own database or a mixture.")
"are defined for a service area in stroom, e.g. core or config. This means you can " +
"have all service areas running in a single database, have each in their own " +
"database or a mixture.")
public CommonDbConfig getCommonDbConfig() {
return commonDbConfig;
}
Expand Down Expand Up @@ -448,10 +455,10 @@ public NodeConfig getNodeConfig() {
}

@JsonPropertyDescription("This is the base endpoint of the node for all inter-node communications, " +
"i.e. all cluster management and node info calls. " +
"This endpoint will typically be hidden behind a firewall and not be publicly available. " +
"The address must be resolvable from all other nodes in the cluster. " +
"This does not need to be set for a single node cluster.")
"i.e. all cluster management and node info calls. " +
"This endpoint will typically be hidden behind a firewall and not be " +
"publicly available. The address must be resolvable from all other nodes " +
"in the cluster. This does not need to be set for a single node cluster.")
@JsonProperty(PROP_NAME_NODE_URI)
public NodeUriConfig getNodeUri() {
return nodeUri;
Expand Down Expand Up @@ -479,7 +486,7 @@ public PropertyServiceConfig getPropertyServiceConfig() {
}

@JsonPropertyDescription("This is public facing URI of stroom which may be different from the local host if " +
"behind a proxy")
"behind a proxy")
@JsonProperty(PROP_NAME_PUBLIC_URI)
public PublicUriConfig getPublicUri() {
return publicUri;
Expand Down Expand Up @@ -537,6 +544,11 @@ public SessionCookieConfig getSessionCookieConfig() {
return sessionCookieConfig;
}

@JsonProperty(PROP_NAME_SESSION)
public SessionConfig getSessionConfig() {
return sessionConfig;
}

@JsonProperty(PROP_NAME_STATE)
@JsonPropertyDescription("Configuration for the stroom state service")
public StateConfig getStateConfig() {
Expand All @@ -555,7 +567,7 @@ public UiConfig getUiConfig() {
}

@JsonPropertyDescription("This is the URI where the UI is hosted if different to the public facing URI of the " +
"server, e.g. during development or some other deployments")
"server, e.g. during development or some other deployments")
@JsonProperty(PROP_NAME_UI_URI)
public UiUriConfig getUiUri() {
return uiUri;
Expand Down
51 changes: 51 additions & 0 deletions stroom-config/stroom-config-app/src/main/java/stroom/config/app/SessionConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package stroom.config.app;

import stroom.util.config.annotations.RequiresRestart;
import stroom.util.config.annotations.RequiresRestart.RestartScope;
import stroom.util.shared.AbstractConfig;
import stroom.util.shared.IsStroomConfig;
import stroom.util.time.StroomDuration;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyDescription;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;


@JsonPropertyOrder(alphabetic = true)
public class SessionConfig extends AbstractConfig implements IsStroomConfig {

public static final String PROP_NAME_MAX_INACTIVE_INTERVAL = "maxInactiveInterval";

public static final StroomDuration DEFAULT_MAX_INACTIVE_INTERVAL = StroomDuration.ofDays(1);

private final StroomDuration maxInactiveInterval;

public SessionConfig() {
this.maxInactiveInterval = DEFAULT_MAX_INACTIVE_INTERVAL;
}

@SuppressWarnings("unused")
@JsonCreator
public SessionConfig(
@JsonProperty(PROP_NAME_MAX_INACTIVE_INTERVAL) final StroomDuration maxInactiveInterval) {
this.maxInactiveInterval = maxInactiveInterval;
}

@RequiresRestart(RestartScope.UI)
@JsonProperty(PROP_NAME_MAX_INACTIVE_INTERVAL)
@JsonPropertyDescription("The maximum time interval between the last access of a HTTP session and " +
"it being considered expired. Set to null for sessions that never expire, " +
"however this will causes sessions to be held and build up in memory indefinitely, " +
"so is best avoided.")
public StroomDuration getMaxInactiveInterval() {
return maxInactiveInterval;
}

@Override
public String toString() {
return "SessionConfig{" +
"maxInactiveInterval=" + maxInactiveInterval +
'}';
}
}
2 changes: 2 additions & 0 deletions stroom-config/stroom-config-app/src/test/resources/stroom/config/app/expected.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,8 @@ appConfig:
servicesPort: 8080
zookeeperBasePath: "/stroom-services"
zookeeperUrl: "localhost:2181"
session:
maxInactiveInterval: "P1D"
sessionCookie:
httpOnly: true
secure: true
Expand Down
Loading