Merge pull request #42937 from deki

* pr/42937:
  Polish "Add logger warning if Hikari datasource doesn't have pool suspension configured"
  Add logger warning if Hikari datasource doesn't have pool suspension configured

Closes gh-42937

Author: mhalbritter

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceCheckpointRestoreConfiguration.java

@@ -25,6 +25,7 @@
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.jdbc.HikariCheckpointRestoreLifecycle;
+import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
@@ -44,8 +45,9 @@ static class Hikari {
 
 		@Bean
 		@ConditionalOnMissingBean
-		HikariCheckpointRestoreLifecycle hikariCheckpointRestoreLifecycle(DataSource dataSource) {
-			return new HikariCheckpointRestoreLifecycle(dataSource);
+		HikariCheckpointRestoreLifecycle hikariCheckpointRestoreLifecycle(DataSource dataSource,
+				ConfigurableApplicationContext applicationContext) {
+			return new HikariCheckpointRestoreLifecycle(dataSource, applicationContext);
 		}
 
 	}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/HikariCheckpointRestoreLifecycle.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 the original author or authors.
+ * Copyright 2012-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.Lifecycle;
 import org.springframework.core.log.LogMessage;
 import org.springframework.util.Assert;
@@ -49,6 +50,7 @@
  *
  * @author Christoph Strobl
  * @author Andy Wilkinson
+ * @author Moritz Halbritter
  * @since 3.2.0
  */
 public class HikariCheckpointRestoreLifecycle implements Lifecycle {
@@ -71,15 +73,34 @@ public class HikariCheckpointRestoreLifecycle implements Lifecycle {
 
 	private final HikariDataSource dataSource;
 
+	private final ConfigurableApplicationContext applicationContext;
+
 	/**
 	 * Creates a new {@code HikariCheckpointRestoreLifecycle} that will allow the given
 	 * {@code dataSource} to participate in checkpoint-restore. The {@code dataSource} is
 	 * {@link DataSourceUnwrapper#unwrap unwrapped} to a {@link HikariDataSource}. If such
 	 * unwrapping is not possible, the lifecycle will have no effect.
 	 * @param dataSource the checkpoint-restore participant
+	 * @deprecated since 3.4.0 for removal in 3.6.0 in favor of
+	 * {@link #HikariCheckpointRestoreLifecycle(DataSource, ConfigurableApplicationContext)}
 	 */
+	@Deprecated(since = "3.4.0", forRemoval = true)
 	public HikariCheckpointRestoreLifecycle(DataSource dataSource) {
+		this(dataSource, null);
+	}
+
+	/**
+	 * Creates a new {@code HikariCheckpointRestoreLifecycle} that will allow the given
+	 * {@code dataSource} to participate in checkpoint-restore. The {@code dataSource} is
+	 * {@link DataSourceUnwrapper#unwrap unwrapped} to a {@link HikariDataSource}. If such
+	 * unwrapping is not possible, the lifecycle will have no effect.
+	 * @param dataSource the checkpoint-restore participant
+	 * @param applicationContext the application context
+	 * @since 3.4.0
+	 */
+	public HikariCheckpointRestoreLifecycle(DataSource dataSource, ConfigurableApplicationContext applicationContext) {
 		this.dataSource = DataSourceUnwrapper.unwrap(dataSource, HikariConfigMXBean.class, HikariDataSource.class);
+		this.applicationContext = applicationContext;
 		this.hasOpenConnections = (pool) -> {
 			ThreadPoolExecutor closeConnectionExecutor = (ThreadPoolExecutor) ReflectionUtils
 				.getField(CLOSE_CONNECTION_EXECUTOR, pool);
@@ -109,13 +130,21 @@ public void stop() {
 			logger.info("Suspending Hikari pool");
 			this.dataSource.getHikariPoolMXBean().suspendPool();
 		}
+		else {
+			if (this.applicationContext != null && !this.applicationContext.isClosed()) {
+				logger.warn(this.dataSource + " is not configured to allow pool suspension. "
+						+ "This will cause problems when the application is checkpointed. "
+						+ "Please configure allow-pool-suspension to fix this!");
+			}
+		}
 		closeConnections(Duration.ofMillis(this.dataSource.getConnectionTimeout() + 250));
 	}
 
 	private void closeConnections(Duration shutdownTimeout) {
 		logger.info("Evicting Hikari connections");
 		this.dataSource.getHikariPoolMXBean().softEvictConnections();
-		logger.debug("Waiting for Hikari connections to be closed");
+		logger.debug(LogMessage.format("Waiting %d seconds for Hikari connections to be closed",
+				shutdownTimeout.toSeconds()));
 		CompletableFuture<Void> allConnectionsClosed = CompletableFuture.runAsync(this::waitForConnectionsToClose);
 		try {
 			allConnectionsClosed.get(shutdownTimeout.toMillis(), TimeUnit.MILLISECONDS);

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/HikariCheckpointRestoreLifecycleTests.java

@@ -24,6 +24,8 @@
 import com.zaxxer.hikari.HikariDataSource;
 import org.junit.jupiter.api.Test;
 
+import org.springframework.context.ConfigurableApplicationContext;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatNoException;
@@ -47,7 +49,8 @@ class HikariCheckpointRestoreLifecycleTests {
 		config.setJdbcUrl("jdbc:hsqldb:mem:test-" + UUID.randomUUID());
 		config.setPoolName("lifecycle-tests");
 		this.dataSource = new HikariDataSource(config);
-		this.lifecycle = new HikariCheckpointRestoreLifecycle(this.dataSource);
+		this.lifecycle = new HikariCheckpointRestoreLifecycle(this.dataSource,
+				mock(ConfigurableApplicationContext.class));
 	}
 
 	@Test
@@ -88,7 +91,7 @@ void whenDataSourceIsClosedThenStartShouldThrow() {
 	@Test
 	void startHasNoEffectWhenDataSourceIsNotAHikariDataSource() {
 		HikariCheckpointRestoreLifecycle nonHikariLifecycle = new HikariCheckpointRestoreLifecycle(
-				mock(DataSource.class));
+				mock(DataSource.class), mock(ConfigurableApplicationContext.class));
 		assertThat(nonHikariLifecycle.isRunning()).isFalse();
 		nonHikariLifecycle.start();
 		assertThat(nonHikariLifecycle.isRunning()).isFalse();
@@ -97,7 +100,7 @@ void startHasNoEffectWhenDataSourceIsNotAHikariDataSource() {
 	@Test
 	void stopHasNoEffectWhenDataSourceIsNotAHikariDataSource() {
 		HikariCheckpointRestoreLifecycle nonHikariLifecycle = new HikariCheckpointRestoreLifecycle(
-				mock(DataSource.class));
+				mock(DataSource.class), mock(ConfigurableApplicationContext.class));
 		assertThat(nonHikariLifecycle.isRunning()).isFalse();
 		nonHikariLifecycle.stop();
 		assertThat(nonHikariLifecycle.isRunning()).isFalse();