From 56ce2b8e3fabd02441ec99077afa22b868b8e948 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 17 Jul 2019 14:12:26 +0200 Subject: [PATCH] Polish "Add metrics support for idle jdbc connections" See gh-17504 --- .../actuate/metrics/jdbc/DataSourcePoolMetrics.java | 3 +-- .../main/asciidoc/production-ready-features.adoc | 4 ++-- .../CommonsDbcp2DataSourcePoolMetadata.java | 11 +++++------ .../boot/jdbc/metadata/DataSourcePoolMetadata.java | 3 ++- .../jdbc/metadata/HikariDataSourcePoolMetadata.java | 13 +++++-------- .../jdbc/metadata/TomcatDataSourcePoolMetadata.java | 11 +++++------ .../AbstractDataSourcePoolMetadataTests.java | 5 ++++- .../CommonsDbcp2DataSourcePoolMetadataTests.java | 12 +----------- .../metadata/HikariDataSourcePoolMetadataTests.java | 13 +------------ .../metadata/TomcatDataSourcePoolMetadataTests.java | 12 +----------- 10 files changed, 27 insertions(+), 60 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/jdbc/DataSourcePoolMetrics.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/jdbc/DataSourcePoolMetrics.java index df75e70d1a..92cf4243b2 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/jdbc/DataSourcePoolMetrics.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/jdbc/DataSourcePoolMetrics.java @@ -38,7 +38,6 @@ import org.springframework.util.ConcurrentReferenceHashMap; * * @author Jon Schneider * @author Phillip Webb - * @author Artsiom Yudovin * @since 2.0.0 */ public class DataSourcePoolMetrics implements MeterBinder { @@ -67,9 +66,9 @@ public class DataSourcePoolMetrics implements MeterBinder { public void bindTo(MeterRegistry registry) { if (this.metadataProvider.getDataSourcePoolMetadata(this.dataSource) != null) { bindPoolMetadata(registry, "active", DataSourcePoolMetadata::getActive); + bindPoolMetadata(registry, "idle", DataSourcePoolMetadata::getIdle); bindPoolMetadata(registry, "max", DataSourcePoolMetadata::getMax); bindPoolMetadata(registry, "min", DataSourcePoolMetadata::getMin); - bindPoolMetadata(registry, "idle", DataSourcePoolMetadata::getIdle); } } diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc index c2d2385d46..297b0cf05c 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc @@ -2023,8 +2023,8 @@ is required. A `CacheMetricsRegistrar` bean is made available to make that proce ==== DataSource Metrics Auto-configuration enables the instrumentation of all available `DataSource` objects with a metric named `jdbc`. Data source instrumentation results in gauges representing the -currently active, maximum allowed, and minimum allowed connections in the pool. Each of -these gauges has a name that is prefixed by `jdbc`. +currently active, idle, maximum allowed, and minimum allowed connections in the pool. Each +of these gauges has a name that is prefixed by `jdbc`. Metrics are also tagged by the name of the `DataSource` computed based on the bean name. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadata.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadata.java index bd18ab3290..7262a45f1b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadata.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadata.java @@ -24,7 +24,6 @@ import org.apache.commons.dbcp2.BasicDataSource; * {@link DataSourcePoolMetadata} for an Apache Commons DBCP2 {@link DataSource}. * * @author Stephane Nicoll - * @author Artsiom Yudovin * @since 2.0.0 */ public class CommonsDbcp2DataSourcePoolMetadata extends AbstractDataSourcePoolMetadata { @@ -38,6 +37,11 @@ public class CommonsDbcp2DataSourcePoolMetadata extends AbstractDataSourcePoolMe return getDataSource().getNumActive(); } + @Override + public Integer getIdle() { + return getDataSource().getNumIdle(); + } + @Override public Integer getMax() { return getDataSource().getMaxTotal(); @@ -58,9 +62,4 @@ public class CommonsDbcp2DataSourcePoolMetadata extends AbstractDataSourcePoolMe return getDataSource().getDefaultAutoCommit(); } - @Override - public Integer getIdle() { - return getDataSource().getNumIdle(); - } - } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/DataSourcePoolMetadata.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/DataSourcePoolMetadata.java index a591fb63db..6e8a2e7e67 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/DataSourcePoolMetadata.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/DataSourcePoolMetadata.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2019 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. @@ -55,6 +55,7 @@ public interface DataSourcePoolMetadata { * if that information is not available. * @return the number of established but idle connections or {@code null} * @since 2.2.0 + * @see #getActive() */ default Integer getIdle() { return null; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadata.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadata.java index 088f03c201..70a7af12a2 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadata.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadata.java @@ -16,8 +16,6 @@ package org.springframework.boot.jdbc.metadata; -import java.util.Objects; - import javax.sql.DataSource; import com.zaxxer.hikari.HikariDataSource; @@ -29,7 +27,6 @@ import org.springframework.beans.DirectFieldAccessor; * {@link DataSourcePoolMetadata} for a Hikari {@link DataSource}. * * @author Stephane Nicoll - * @author Artsiom Yudovin * @since 2.0.0 */ public class HikariDataSourcePoolMetadata extends AbstractDataSourcePoolMetadata { @@ -50,12 +47,12 @@ public class HikariDataSourcePoolMetadata extends AbstractDataSourcePoolMetadata @Override public Integer getIdle() { - HikariPool pool = getHikariPool(); - if (Objects.nonNull(pool)) { - return pool.getIdleConnections(); + try { + return getHikariPool().getIdleConnections(); + } + catch (Exception ex) { + return null; } - - return null; } private HikariPool getHikariPool() { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadata.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadata.java index dc6db94276..fc5d0bc707 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadata.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadata.java @@ -23,7 +23,6 @@ import org.apache.tomcat.jdbc.pool.DataSource; * {@link DataSourcePoolMetadata} for a Tomcat DataSource. * * @author Stephane Nicoll - * @author Artsiom Yudovin * @since 2.0.0 */ public class TomcatDataSourcePoolMetadata extends AbstractDataSourcePoolMetadata { @@ -38,6 +37,11 @@ public class TomcatDataSourcePoolMetadata extends AbstractDataSourcePoolMetadata return (pool != null) ? pool.getActive() : 0; } + @Override + public Integer getIdle() { + return getDataSource().getNumIdle(); + } + @Override public Integer getMax() { return getDataSource().getMaxActive(); @@ -58,9 +62,4 @@ public class TomcatDataSourcePoolMetadata extends AbstractDataSourcePoolMetadata return getDataSource().isDefaultAutoCommit(); } - @Override - public Integer getIdle() { - return getDataSource().getNumIdle(); - } - } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/AbstractDataSourcePoolMetadataTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/AbstractDataSourcePoolMetadataTests.java index b53701ab1c..c8023ad258 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/AbstractDataSourcePoolMetadataTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/AbstractDataSourcePoolMetadataTests.java @@ -34,7 +34,8 @@ import static org.assertj.core.api.Assertions.assertThat; abstract class AbstractDataSourcePoolMetadataTests> { /** - * Return a data source metadata instance with a min size of 0 and max size of 2. + * Return a data source metadata instance with a min size of 0 and max size of 2. Idle + * connections are not reclaimed immediately. * @return the data source metadata */ protected abstract D getDataSourceMetadata(); @@ -70,6 +71,8 @@ abstract class AbstractDataSourcePoolMetadataTests) (connection) -> null); assertThat(getDataSourceMetadata().getIdle()).isEqualTo(Integer.valueOf(1)); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadataTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadataTests.java index e8ea011f80..2207eb82a6 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadataTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadataTests.java @@ -19,9 +19,6 @@ package org.springframework.boot.jdbc.metadata; import org.apache.commons.dbcp2.BasicDataSource; import org.junit.jupiter.api.Test; -import org.springframework.jdbc.core.ConnectionCallback; -import org.springframework.jdbc.core.JdbcTemplate; - import static org.assertj.core.api.Assertions.assertThat; /** @@ -86,9 +83,7 @@ class CommonsDbcp2DataSourcePoolMetadataTests BasicDataSource dataSource = createDataSource(); dataSource.setMinIdle(minSize); dataSource.setMaxTotal(maxSize); - - this.initPool(dataSource); - + dataSource.setMinEvictableIdleTimeMillis(5000); return new CommonsDbcp2DataSourcePoolMetadata(dataSource); } @@ -96,9 +91,4 @@ class CommonsDbcp2DataSourcePoolMetadataTests return initializeBuilder().type(BasicDataSource.class).build(); } - private void initPool(BasicDataSource dataSource) { - JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource); - jdbcTemplate.execute((ConnectionCallback) (connection) -> null); - } - } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadataTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadataTests.java index 77a8a1e5a1..4618f6406b 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadataTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadataTests.java @@ -18,16 +18,12 @@ package org.springframework.boot.jdbc.metadata; import com.zaxxer.hikari.HikariDataSource; -import org.springframework.jdbc.core.ConnectionCallback; -import org.springframework.jdbc.core.JdbcTemplate; - import static org.assertj.core.api.Assertions.assertThat; /** * Tests for {@link HikariDataSourcePoolMetadata}. * * @author Stephane Nicoll - * @author Artsiom Yudovin */ public class HikariDataSourcePoolMetadataTests extends AbstractDataSourcePoolMetadataTests { @@ -58,15 +54,8 @@ public class HikariDataSourcePoolMetadataTests HikariDataSource dataSource = initializeBuilder().type(HikariDataSource.class).build(); dataSource.setMinimumIdle(minSize); dataSource.setMaximumPoolSize(maxSize); - - this.initPool(dataSource); - + dataSource.setIdleTimeout(5000); return dataSource; } - private void initPool(HikariDataSource dataSource) { - JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource); - jdbcTemplate.execute((ConnectionCallback) (connection) -> null); - } - } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadataTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadataTests.java index fe4e3a5771..50713417e0 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadataTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadataTests.java @@ -18,9 +18,6 @@ package org.springframework.boot.jdbc.metadata; import org.apache.tomcat.jdbc.pool.DataSource; -import org.springframework.jdbc.core.ConnectionCallback; -import org.springframework.jdbc.core.JdbcTemplate; - import static org.assertj.core.api.Assertions.assertThat; /** @@ -57,19 +54,12 @@ public class TomcatDataSourcePoolMetadataTests DataSource dataSource = initializeBuilder().type(DataSource.class).build(); dataSource.setMinIdle(minSize); dataSource.setMaxActive(maxSize); + dataSource.setMinEvictableIdleTimeMillis(5000); // Avoid warnings dataSource.setInitialSize(minSize); dataSource.setMaxIdle(maxSize); - - this.initPool(dataSource); - return dataSource; } - private void initPool(DataSource dataSource) { - JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource); - jdbcTemplate.execute((ConnectionCallback) (connection) -> null); - } - }