From 8c6c36c01e9d5feb5dc442c4fc82db25cb45f905 Mon Sep 17 00:00:00 2001 From: michaelpaliy Date: Sat, 15 May 2021 20:52:42 +0300 Subject: [PATCH] bugFix embrace spring flyway (#2246) * embrace spring flyway * embrace spring flyway - add comments --- .../config/config-mysql-grpc.properties | 10 +- docker/server/config/config-mysql.properties | 10 +- mysql-persistence/build.gradle | 2 +- .../mysql/config/MySQLConfiguration.java | 14 +- .../mysql/config/MySQLDataSourceProvider.java | 93 ---------- .../mysql/config/MySQLProperties.java | 160 ------------------ .../mysql/dao/MySQLExecutionDAOTest.java | 4 +- .../mysql/dao/MySQLMetadataDAOTest.java | 3 +- .../mysql/dao/MySQLQueueDAOTest.java | 3 +- .../mysql/util/MySQLDAOTestUtil.java | 56 +----- postgres-persistence/build.gradle | 2 +- .../config/PostgresConfiguration.java | 15 +- .../config/PostgresDataSourceProvider.java | 95 ----------- .../postgres/config/PostgresProperties.java | 159 ----------------- .../dao/PostgresExecutionDAOTest.java | 4 +- .../postgres/dao/PostgresMetadataDAOTest.java | 3 +- .../postgres/dao/PostgresQueueDAOTest.java | 3 +- .../postgres/util/PostgresDAOTestUtil.java | 61 +------ .../java/com/netflix/conductor/Conductor.java | 5 +- .../src/main/resources/application.properties | 1 - .../grpc/mysql/MySQLGrpcEndToEndTest.java | 11 +- .../postgres/PostgresGrpcEndToEndTest.java | 11 +- 22 files changed, 70 insertions(+), 655 deletions(-) delete mode 100644 mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLDataSourceProvider.java delete mode 100644 postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresDataSourceProvider.java diff --git a/docker/server/config/config-mysql-grpc.properties b/docker/server/config/config-mysql-grpc.properties index 39a366428a..2582b4d89f 100755 --- a/docker/server/config/config-mysql-grpc.properties +++ b/docker/server/config/config-mysql-grpc.properties @@ -17,13 +17,13 @@ conductor.grpc-server.enabled=true # Database persistence model. conductor.db.type=mysql -conductor.mysql.jdbcUrl=jdbc:mysql://mysql:3306/conductor -conductor.mysql.jdbcUsername=conductor -conductor.mysql.jdbcPassword=conductor +spring.datasource.url=jdbc:mysql://mysql:3306/conductor +spring.datasource.username=conductor +spring.datasource.password=conductor # Hikari pool sizes are -1 by default and prevent startup -conductor.mysql.connectionPoolMaxSize=10 -conductor.mysql.connectionPoolMinIdle=2 +spring.datasource.hikari.maximum-pool-size=10 +spring.datasource.hikari.minimum-idle=2 # Elastic search instance indexing is enabled. conductor.indexing.enabled=true diff --git a/docker/server/config/config-mysql.properties b/docker/server/config/config-mysql.properties index bddf362145..7600398307 100755 --- a/docker/server/config/config-mysql.properties +++ b/docker/server/config/config-mysql.properties @@ -4,13 +4,13 @@ conductor.grpc-server.enabled=false # Database persistence type. conductor.db.type=mysql -conductor.mysql.jdbcUrl=jdbc:mysql://mysql:3306/conductor -conductor.mysql.jdbcUsername=conductor -conductor.mysql.jdbcPassword=conductor +spring.datasource.url=jdbc:mysql://mysql:3306/conductor +spring.datasource.username=conductor +spring.datasource.password=conductor # Hikari pool sizes are -1 by default and prevent startup -conductor.mysql.connectionPoolMaxSize=10 -conductor.mysql.connectionPoolMinIdle=2 +spring.datasource.hikari.maximum-pool-size=10 +spring.datasource.hikari.minimum-idle=2 # Elastic search instance indexing is enabled. conductor.indexing.enabled=true diff --git a/mysql-persistence/build.gradle b/mysql-persistence/build.gradle index a5c1e0023c..6339edf527 100644 --- a/mysql-persistence/build.gradle +++ b/mysql-persistence/build.gradle @@ -25,7 +25,7 @@ dependencies { implementation "org.apache.commons:commons-lang3" implementation "mysql:mysql-connector-java" - implementation "com.zaxxer:HikariCP" + implementation "org.springframework.boot:spring-boot-starter-jdbc" implementation "org.flywaydb:flyway-core" testImplementation "org.testcontainers:mysql:${revTestContainer}" diff --git a/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLConfiguration.java b/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLConfiguration.java index 8d5e34b05a..a9d36f6dcc 100644 --- a/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLConfiguration.java +++ b/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLConfiguration.java @@ -21,32 +21,36 @@ import com.netflix.conductor.mysql.dao.MySQLQueueDAO; import javax.sql.DataSource; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; +import org.springframework.context.annotation.Import; @SuppressWarnings("SpringJavaInjectionPointsAutowiringInspection") @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties(MySQLProperties.class) @ConditionalOnProperty(name = "conductor.db.type", havingValue = "mysql") +// Import the DataSourceAutoConfiguration when mysql database is selected. +// By default the datasource configuration is excluded in the main module. +@Import(DataSourceAutoConfiguration.class) public class MySQLConfiguration { @Bean - public DataSource dataSource(MySQLProperties properties) { - return new MySQLDataSourceProvider(properties).getDataSource(); - } - - @Bean + @DependsOn({"flyway", "flywayInitializer"}) public MetadataDAO mySqlMetadataDAO(ObjectMapper objectMapper, DataSource dataSource, MySQLProperties properties) { return new MySQLMetadataDAO(objectMapper, dataSource, properties); } @Bean + @DependsOn({"flyway", "flywayInitializer"}) public ExecutionDAO mySqlExecutionDAO(ObjectMapper objectMapper, DataSource dataSource) { return new MySQLExecutionDAO(objectMapper, dataSource); } @Bean + @DependsOn({"flyway", "flywayInitializer"}) public QueueDAO mySqlQueueDAO(ObjectMapper objectMapper, DataSource dataSource) { return new MySQLQueueDAO(objectMapper, dataSource); } diff --git a/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLDataSourceProvider.java b/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLDataSourceProvider.java deleted file mode 100644 index 1836bc348e..0000000000 --- a/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLDataSourceProvider.java +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Copyright 2020 Netflix, Inc. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package com.netflix.conductor.mysql.config; - -import com.google.common.util.concurrent.ThreadFactoryBuilder; -import com.zaxxer.hikari.HikariConfig; -import com.zaxxer.hikari.HikariDataSource; -import org.flywaydb.core.Flyway; -import org.flywaydb.core.api.configuration.FluentConfiguration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.sql.DataSource; -import java.util.concurrent.ThreadFactory; - -public class MySQLDataSourceProvider { - - private static final Logger LOGGER = LoggerFactory.getLogger(MySQLDataSourceProvider.class); - - private final MySQLProperties properties; - - public MySQLDataSourceProvider(MySQLProperties properties) { - this.properties = properties; - } - - public DataSource getDataSource() { - HikariDataSource dataSource = null; - try { - dataSource = new HikariDataSource(createConfiguration()); - flywayMigrate(dataSource); - return dataSource; - } catch (final Throwable t) { - if (null != dataSource && !dataSource.isClosed()) { - dataSource.close(); - } - LOGGER.error("error migration DB", t); - throw t; - } - } - - private HikariConfig createConfiguration() { - HikariConfig hikariConfig = new HikariConfig(); - hikariConfig.setJdbcUrl(properties.getJdbcUrl()); - hikariConfig.setUsername(properties.getJdbcUsername()); - hikariConfig.setPassword(properties.getJdbcPassword()); - hikariConfig.setAutoCommit(false); - hikariConfig.setMaximumPoolSize(properties.getConnectionPoolMaxSize()); - hikariConfig.setMinimumIdle(properties.getConnectionPoolMinIdle()); - hikariConfig.setMaxLifetime(properties.getConnectionMaxLifetime().toMillis()); - hikariConfig.setIdleTimeout(properties.getConnectionIdleTimeout().toMillis()); - hikariConfig.setConnectionTimeout(properties.getConnectionTimeout().toMillis()); - hikariConfig.setTransactionIsolation(properties.getTransactionIsolationLevel()); - hikariConfig.setAutoCommit(properties.isAutoCommit()); - - ThreadFactory threadFactory = new ThreadFactoryBuilder() - .setDaemon(true) - .setNameFormat("hikari-mysql-%d") - .build(); - - hikariConfig.setThreadFactory(threadFactory); - return hikariConfig; - } - - // TODO Move this into a class that has complete lifecycle for the connection, i.e. startup and shutdown. - private void flywayMigrate(DataSource dataSource) { - boolean enabled = properties.isFlywayEnabled(); - if (!enabled) { - LOGGER.debug("Flyway migrations are disabled"); - return; - } - - String flywayTable = properties.getFlywayTable(); - LOGGER.debug("Using Flyway migration table '{}'", flywayTable); - - FluentConfiguration fluentConfiguration = Flyway.configure() - .table(flywayTable) - .dataSource(dataSource) - .placeholderReplacement(false); - - Flyway flyway = fluentConfiguration.load(); - flyway.migrate(); - } -} diff --git a/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLProperties.java b/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLProperties.java index ae9e543392..b026d4a5fa 100644 --- a/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLProperties.java +++ b/mysql-persistence/src/main/java/com/netflix/conductor/mysql/config/MySQLProperties.java @@ -22,70 +22,6 @@ @ConfigurationProperties("conductor.mysql") public class MySQLProperties { - /** - * The jdbc url to be used for connecting to the database - */ - private String jdbcUrl = "jdbc:mysql://localhost:3306/conductor"; - - /** - * The username to be used for connections - */ - private String jdbcUsername = "conductor"; - - /** - * The password to be used for connections - */ - private String jdbcPassword = "password"; - - /** - * Used to enable/disable flyway migrations - */ - private boolean flywayEnabled = true; - - /** - * Used to override the default flyway migration table - */ - private String flywayTable = "schema_version"; - - // The defaults are currently in line with the HikariConfig defaults, which are unfortunately private. - /** - * The maximum size that the connection pool is allowed to reach including idle and in-use connections - */ - private int connectionPoolMaxSize = -1; - - /** - * The minimum number of idle connections that the connection pool tries to maintain in the pool - */ - private int connectionPoolMinIdle = -1; - - /** - * The maximum lifetime of a connection (in minutes) in the pool - */ - @DurationUnit(ChronoUnit.MINUTES) - private Duration connectionMaxLifetime = Duration.ofMinutes(30); - - /** - * The maximum amount of time (in minutes) that a connection is allowed to sit idle in the pool - */ - @DurationUnit(ChronoUnit.MINUTES) - private Duration connectionIdleTimeout = Duration.ofMinutes(10); - - /** - * The maximum amount of time (in seconds) that a client will wait for a connection from the pool - */ - @DurationUnit(ChronoUnit.SECONDS) - private Duration connectionTimeout = Duration.ofSeconds(30); - - /** - * The transaction isolation level as specified in {@link Connection} - */ - private String transactionIsolationLevel = ""; - - //This is consistent with the current default when building the Hikari Client. - /** - * The auto-commit behavior of the connections in the pool - */ - private boolean autoCommit = false; /** * The time (in seconds) after which the in-memory task definitions cache will be refreshed @@ -93,102 +29,6 @@ public class MySQLProperties { private Duration taskDefCacheRefreshInterval = Duration.ofSeconds(60); - public String getJdbcUrl() { - return jdbcUrl; - } - - public void setJdbcUrl(String jdbcUrl) { - this.jdbcUrl = jdbcUrl; - } - - public String getJdbcUsername() { - return jdbcUsername; - } - - public void setJdbcUsername(String jdbcUsername) { - this.jdbcUsername = jdbcUsername; - } - - public String getJdbcPassword() { - return jdbcPassword; - } - - public void setJdbcPassword(String jdbcPassword) { - this.jdbcPassword = jdbcPassword; - } - - public boolean isFlywayEnabled() { - return flywayEnabled; - } - - public void setFlywayEnabled(boolean flywayEnabled) { - this.flywayEnabled = flywayEnabled; - } - - public String getFlywayTable() { - return flywayTable; - } - - public void setFlywayTable(String flywayTable) { - this.flywayTable = flywayTable; - } - - public int getConnectionPoolMaxSize() { - return connectionPoolMaxSize; - } - - public void setConnectionPoolMaxSize(int connectionPoolMaxSize) { - this.connectionPoolMaxSize = connectionPoolMaxSize; - } - - public int getConnectionPoolMinIdle() { - return connectionPoolMinIdle; - } - - public void setConnectionPoolMinIdle(int connectionPoolMinIdle) { - this.connectionPoolMinIdle = connectionPoolMinIdle; - } - - public Duration getConnectionMaxLifetime() { - return connectionMaxLifetime; - } - - public void setConnectionMaxLifetime(Duration connectionMaxLifetime) { - this.connectionMaxLifetime = connectionMaxLifetime; - } - - public Duration getConnectionIdleTimeout() { - return connectionIdleTimeout; - } - - public void setConnectionIdleTimeout(Duration connectionIdleTimeout) { - this.connectionIdleTimeout = connectionIdleTimeout; - } - - public Duration getConnectionTimeout() { - return connectionTimeout; - } - - public void setConnectionTimeout(Duration connectionTimeout) { - this.connectionTimeout = connectionTimeout; - } - - public String getTransactionIsolationLevel() { - return transactionIsolationLevel; - } - - public void setTransactionIsolationLevel(String transactionIsolationLevel) { - this.transactionIsolationLevel = transactionIsolationLevel; - } - - public boolean isAutoCommit() { - return autoCommit; - } - - public void setAutoCommit(boolean autoCommit) { - this.autoCommit = autoCommit; - } - public Duration getTaskDefCacheRefreshInterval() { return taskDefCacheRefreshInterval; } diff --git a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLExecutionDAOTest.java b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLExecutionDAOTest.java index f91faf479d..fe21d5107c 100644 --- a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLExecutionDAOTest.java +++ b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLExecutionDAOTest.java @@ -55,14 +55,12 @@ public class MySQLExecutionDAOTest extends ExecutionDAOTest { public void setup() { mySQLContainer = new MySQLContainer<>(DockerImageName.parse("mysql")).withDatabaseName(name.getMethodName()); mySQLContainer.start(); - testUtil = new MySQLDAOTestUtil(mySQLContainer, objectMapper, name.getMethodName()); + testUtil = new MySQLDAOTestUtil(mySQLContainer, objectMapper); executionDAO = new MySQLExecutionDAO(testUtil.getObjectMapper(), testUtil.getDataSource()); - testUtil.resetAllData(); } @After public void teardown() { - testUtil.resetAllData(); testUtil.getDataSource().close(); } diff --git a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLMetadataDAOTest.java b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLMetadataDAOTest.java index 7a167aa265..b024d5e952 100644 --- a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLMetadataDAOTest.java +++ b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLMetadataDAOTest.java @@ -71,14 +71,13 @@ public class MySQLMetadataDAOTest { public void setup() { mySQLContainer = new MySQLContainer<>(DockerImageName.parse("mysql")).withDatabaseName(name.getMethodName()); mySQLContainer.start(); - testUtil = new MySQLDAOTestUtil(mySQLContainer, objectMapper, name.getMethodName()); + testUtil = new MySQLDAOTestUtil(mySQLContainer, objectMapper); metadataDAO = new MySQLMetadataDAO(testUtil.getObjectMapper(), testUtil.getDataSource(), testUtil.getTestProperties()); } @After public void teardown() { - testUtil.resetAllData(); testUtil.getDataSource().close(); } diff --git a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLQueueDAOTest.java b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLQueueDAOTest.java index 114c9c2b96..4704ac6c6b 100644 --- a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLQueueDAOTest.java +++ b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/dao/MySQLQueueDAOTest.java @@ -70,13 +70,12 @@ public class MySQLQueueDAOTest { public void setup() { mySQLContainer = new MySQLContainer<>(DockerImageName.parse("mysql")).withDatabaseName(name.getMethodName()); mySQLContainer.start(); - testUtil = new MySQLDAOTestUtil(mySQLContainer, objectMapper, name.getMethodName()); + testUtil = new MySQLDAOTestUtil(mySQLContainer, objectMapper); queueDAO = new MySQLQueueDAO(testUtil.getObjectMapper(), testUtil.getDataSource()); } @After public void teardown() { - testUtil.resetAllData(); testUtil.getDataSource().close(); } diff --git a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/util/MySQLDAOTestUtil.java b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/util/MySQLDAOTestUtil.java index 09845dae49..c573412ce5 100644 --- a/mysql-persistence/src/test/java/com/netflix/conductor/mysql/util/MySQLDAOTestUtil.java +++ b/mysql-persistence/src/test/java/com/netflix/conductor/mysql/util/MySQLDAOTestUtil.java @@ -17,15 +17,9 @@ import com.zaxxer.hikari.HikariDataSource; import org.flywaydb.core.Flyway; import org.flywaydb.core.api.configuration.FluentConfiguration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.testcontainers.containers.MySQLContainer; import javax.sql.DataSource; -import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; import java.time.Duration; import static org.mockito.Mockito.mock; @@ -33,36 +27,26 @@ public class MySQLDAOTestUtil { - private static final Logger LOGGER = LoggerFactory.getLogger(MySQLDAOTestUtil.class); private final HikariDataSource dataSource; - private final MySQLProperties properties; + private final MySQLProperties properties = mock(MySQLProperties.class); private final ObjectMapper objectMapper; - public MySQLDAOTestUtil(MySQLContainer mySQLContainer, ObjectMapper objectMapper, String dbName) { - properties = mock(MySQLProperties.class); - when(properties.getJdbcUrl()).thenReturn(mySQLContainer.getJdbcUrl() - + "?useSSL=false&useUnicode=true&useJDBCCompliantTimezoneShift=true&useLegacyDatetimeCode=false&serverTimezone=UTC"); - when(properties.getJdbcUsername()).thenReturn(mySQLContainer.getUsername()); - when(properties.getJdbcPassword()).thenReturn(mySQLContainer.getPassword()); - when(properties.getTaskDefCacheRefreshInterval()).thenReturn(Duration.ofSeconds(60)); - //createDatabase(mySQLContainer, dbName); + public MySQLDAOTestUtil(MySQLContainer mySQLContainer, ObjectMapper objectMapper) { + this.objectMapper = objectMapper; - this.dataSource = getDataSource(properties); - } - private HikariDataSource getDataSource(MySQLProperties properties) { - HikariDataSource dataSource = new HikariDataSource(); - dataSource.setJdbcUrl(properties.getJdbcUrl()); - dataSource.setUsername(properties.getJdbcUsername()); - dataSource.setPassword(properties.getJdbcPassword()); + this.dataSource = new HikariDataSource(); + dataSource.setJdbcUrl(mySQLContainer.getJdbcUrl()); + dataSource.setUsername(mySQLContainer.getUsername()); + dataSource.setPassword(mySQLContainer.getPassword()); dataSource.setAutoCommit(false); + when(properties.getTaskDefCacheRefreshInterval()).thenReturn(Duration.ofSeconds(60)); + // Prevent DB from getting exhausted during rapid testing dataSource.setMaximumPoolSize(8); flywayMigrate(dataSource); - - return dataSource; } private void flywayMigrate(DataSource dataSource) { @@ -87,26 +71,4 @@ public ObjectMapper getObjectMapper() { return objectMapper; } - public void resetAllData() { - LOGGER.info("Resetting data for test"); - try (Connection connection = dataSource.getConnection()) { - try (ResultSet rs = connection.prepareStatement("SHOW TABLES").executeQuery(); - PreparedStatement keysOn = connection.prepareStatement("SET FOREIGN_KEY_CHECKS=1")) { - try (PreparedStatement keysOff = connection.prepareStatement("SET FOREIGN_KEY_CHECKS=0")) { - keysOff.execute(); - while (rs.next()) { - String table = rs.getString(1); - try (PreparedStatement ps = connection.prepareStatement("TRUNCATE TABLE " + table)) { - ps.execute(); - } - } - } finally { - keysOn.execute(); - } - } - } catch (SQLException ex) { - LOGGER.error(ex.getMessage(), ex); - throw new RuntimeException(ex); - } - } } diff --git a/postgres-persistence/build.gradle b/postgres-persistence/build.gradle index 22cad03c1a..99dad0fd3d 100644 --- a/postgres-persistence/build.gradle +++ b/postgres-persistence/build.gradle @@ -24,7 +24,7 @@ dependencies { implementation "org.apache.commons:commons-lang3" implementation "org.postgresql:postgresql" - implementation "com.zaxxer:HikariCP" + implementation "org.springframework.boot:spring-boot-starter-jdbc" implementation "org.flywaydb:flyway-core" testImplementation "org.testcontainers:postgresql:${revTestContainer}" diff --git a/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresConfiguration.java b/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresConfiguration.java index 32dfea5ef2..615aceca01 100644 --- a/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresConfiguration.java +++ b/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresConfiguration.java @@ -21,33 +21,44 @@ import com.netflix.conductor.postgres.dao.PostgresQueueDAO; import javax.sql.DataSource; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.autoconfigure.flyway.FlywayConfigurationCustomizer; +import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; +import org.springframework.context.annotation.Import; @SuppressWarnings("SpringJavaInjectionPointsAutowiringInspection") @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties(PostgresProperties.class) @ConditionalOnProperty(name = "conductor.db.type", havingValue = "postgres") +// Import the DataSourceAutoConfiguration when postgres database is selected. +// By default the datasource configuration is excluded in the main module. +@Import(DataSourceAutoConfiguration.class) public class PostgresConfiguration { @Bean - public DataSource dataSource(PostgresProperties config) { - return new PostgresDataSourceProvider(config).getDataSource(); + public FlywayConfigurationCustomizer flywayConfigurationCustomizer() { + // override the default location. + return configuration -> configuration.locations("classpath:db/migration_postgres"); } @Bean + @DependsOn({"flyway", "flywayInitializer"}) public MetadataDAO postgresMetadataDAO(ObjectMapper objectMapper, DataSource dataSource, PostgresProperties properties) { return new PostgresMetadataDAO(objectMapper, dataSource, properties); } @Bean + @DependsOn({"flyway", "flywayInitializer"}) public ExecutionDAO postgresExecutionDAO(ObjectMapper objectMapper, DataSource dataSource) { return new PostgresExecutionDAO(objectMapper, dataSource); } @Bean + @DependsOn({"flyway", "flywayInitializer"}) public QueueDAO postgresQueueDAO(ObjectMapper objectMapper, DataSource dataSource) { return new PostgresQueueDAO(objectMapper, dataSource); } diff --git a/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresDataSourceProvider.java b/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresDataSourceProvider.java deleted file mode 100644 index 7159cfcd23..0000000000 --- a/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresDataSourceProvider.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright 2020 Netflix, Inc. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package com.netflix.conductor.postgres.config; - -import com.google.common.util.concurrent.ThreadFactoryBuilder; -import com.zaxxer.hikari.HikariConfig; -import com.zaxxer.hikari.HikariDataSource; -import org.flywaydb.core.Flyway; -import org.flywaydb.core.api.configuration.FluentConfiguration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.sql.DataSource; -import java.nio.file.Paths; -import java.util.concurrent.ThreadFactory; - -public class PostgresDataSourceProvider { - - private static final Logger LOGGER = LoggerFactory.getLogger(PostgresDataSourceProvider.class); - - private final PostgresProperties properties; - - public PostgresDataSourceProvider(PostgresProperties properties) { - this.properties = properties; - } - - public DataSource getDataSource() { - HikariDataSource dataSource = null; - try { - dataSource = new HikariDataSource(createConfiguration()); - flywayMigrate(dataSource); - return dataSource; - } catch (final Throwable t) { - if (null != dataSource && !dataSource.isClosed()) { - dataSource.close(); - } - LOGGER.error("error migration DB", t); - throw t; - } - } - - private HikariConfig createConfiguration() { - HikariConfig hikariConfig = new HikariConfig(); - hikariConfig.setJdbcUrl(properties.getJdbcUrl()); - hikariConfig.setUsername(properties.getJdbcUsername()); - hikariConfig.setPassword(properties.getJdbcPassword()); - hikariConfig.setAutoCommit(false); - hikariConfig.setMaximumPoolSize(properties.getConnectionPoolMaxSize()); - hikariConfig.setMinimumIdle(properties.getConnectionPoolMinIdle()); - hikariConfig.setMaxLifetime(properties.getConnectionMaxLifetime().toMillis()); - hikariConfig.setIdleTimeout(properties.getConnectionIdleTimeout().toMillis()); - hikariConfig.setConnectionTimeout(properties.getConnectionTimeout().toMillis()); - hikariConfig.setTransactionIsolation(properties.getTransactionIsolationLevel()); - hikariConfig.setAutoCommit(properties.isAutoCommit()); - - ThreadFactory tf = new ThreadFactoryBuilder() - .setDaemon(true) - .setNameFormat("hikari-postgres-%d") - .build(); - - hikariConfig.setThreadFactory(tf); - return hikariConfig; - } - - // TODO Move this into a class that has complete lifecycle for the connection, i.e. startup and shutdown. - private void flywayMigrate(DataSource dataSource) { - boolean enabled = properties.isFlywayEnabled(); - if (!enabled) { - LOGGER.debug("Flyway migrations are disabled"); - return; - } - - String flywayTable = properties.getFlywayTable(); - LOGGER.debug("Using Flyway migration table '{}'", flywayTable); - - FluentConfiguration fluentConfiguration = Flyway.configure() - .table(flywayTable) - .locations(Paths.get("db", "migration_postgres").toString()) - .dataSource(dataSource) - .placeholderReplacement(false); - - Flyway flyway = fluentConfiguration.load(); - flyway.migrate(); - } -} diff --git a/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresProperties.java b/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresProperties.java index 6d0dba6468..8df1c9c5fc 100644 --- a/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresProperties.java +++ b/postgres-persistence/src/main/java/com/netflix/conductor/postgres/config/PostgresProperties.java @@ -22,70 +22,6 @@ @ConfigurationProperties("conductor.postgres") public class PostgresProperties { - /** - * The jdbc url to be used for connecting to the database - */ - private String jdbcUrl = "jdbc:postgresql://localhost:5432/conductor"; - - /** - * The username to be used for connections - */ - private String jdbcUsername = "conductor"; - - /** - * The password to be used for connections - */ - private String jdbcPassword = "password"; - - /** - * Used to enable/disable flyway migrations - */ - private boolean flywayEnabled = true; - - /** - * Used to override the default flyway migration table - */ - private String flywayTable = "schema_version"; - - // The defaults are currently in line with the HikariConfig defaults, which are unfortunately private. - /** - * The maximum size that the connection pool is allowed to reach including idle and in-use connections - */ - private int connectionPoolMaxSize = -1; - - /** - * The minimum number of idle connections that the connection pool tries to maintain in the pool - */ - private int connectionPoolMinIdle = -1; - - /** - * The maximum lifetime (in minutes) of a connection in the pool - */ - @DurationUnit(ChronoUnit.MINUTES) - private Duration connectionMaxLifetime = Duration.ofMinutes(30); - - /** - * The maximum amount of time (in minutes) that a connection is allowed to sit idle in the pool - */ - @DurationUnit(ChronoUnit.MINUTES) - private Duration connectionIdleTimeout = Duration.ofMinutes(10); - - /** - * The maximum amount of time (in seconds) that a client will wait for a connection from the pool - */ - @DurationUnit(ChronoUnit.SECONDS) - private Duration connectionTimeout = Duration.ofSeconds(30); - - /** - * The transaction isolation level as specified in {@link Connection} - */ - private String transactionIsolationLevel = ""; - - // This is consistent with the current default when building the Hikari Client. - /** - * The auto-commit behavior of the connections in the pool - */ - private boolean autoCommit = false; /** * The time in seconds after which the in-memory task definitions cache will be refreshed @@ -93,101 +29,6 @@ public class PostgresProperties { @DurationUnit(ChronoUnit.SECONDS) private Duration taskDefCacheRefreshInterval = Duration.ofSeconds(60); - public String getJdbcUrl() { - return jdbcUrl; - } - - public void setJdbcUrl(String jdbcUrl) { - this.jdbcUrl = jdbcUrl; - } - - public String getJdbcUsername() { - return jdbcUsername; - } - - public void setJdbcUsername(String jdbcUsername) { - this.jdbcUsername = jdbcUsername; - } - - public String getJdbcPassword() { - return jdbcPassword; - } - - public void setJdbcPassword(String jdbcPassword) { - this.jdbcPassword = jdbcPassword; - } - - public boolean isFlywayEnabled() { - return flywayEnabled; - } - - public void setFlywayEnabled(boolean flywayEnabled) { - this.flywayEnabled = flywayEnabled; - } - - public String getFlywayTable() { - return flywayTable; - } - - public void setFlywayTable(String flywayTable) { - this.flywayTable = flywayTable; - } - - public int getConnectionPoolMaxSize() { - return connectionPoolMaxSize; - } - - public void setConnectionPoolMaxSize(int connectionPoolMaxSize) { - this.connectionPoolMaxSize = connectionPoolMaxSize; - } - - public int getConnectionPoolMinIdle() { - return connectionPoolMinIdle; - } - - public void setConnectionPoolMinIdle(int connectionPoolMinIdle) { - this.connectionPoolMinIdle = connectionPoolMinIdle; - } - - public Duration getConnectionMaxLifetime() { - return connectionMaxLifetime; - } - - public void setConnectionMaxLifetime(Duration connectionMaxLifetime) { - this.connectionMaxLifetime = connectionMaxLifetime; - } - - public Duration getConnectionIdleTimeout() { - return connectionIdleTimeout; - } - - public void setConnectionIdleTimeout(Duration connectionIdleTimeout) { - this.connectionIdleTimeout = connectionIdleTimeout; - } - - public Duration getConnectionTimeout() { - return connectionTimeout; - } - - public void setConnectionTimeout(Duration connectionTimeout) { - this.connectionTimeout = connectionTimeout; - } - - public String getTransactionIsolationLevel() { - return transactionIsolationLevel; - } - - public void setTransactionIsolationLevel(String transactionIsolationLevel) { - this.transactionIsolationLevel = transactionIsolationLevel; - } - - public boolean isAutoCommit() { - return autoCommit; - } - - public void setAutoCommit(boolean autoCommit) { - this.autoCommit = autoCommit; - } public Duration getTaskDefCacheRefreshInterval() { return taskDefCacheRefreshInterval; diff --git a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresExecutionDAOTest.java b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresExecutionDAOTest.java index 77f4241c50..70b5ecbf96 100644 --- a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresExecutionDAOTest.java +++ b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresExecutionDAOTest.java @@ -56,17 +56,15 @@ public void setup() { postgreSQLContainer = new PostgreSQLContainer<>(DockerImageName.parse("postgres")).withDatabaseName(name.getMethodName().toLowerCase()); postgreSQLContainer.start(); - testPostgres = new PostgresDAOTestUtil(postgreSQLContainer, objectMapper, name.getMethodName().toLowerCase()); + testPostgres = new PostgresDAOTestUtil(postgreSQLContainer, objectMapper); executionDAO = new PostgresExecutionDAO( testPostgres.getObjectMapper(), testPostgres.getDataSource() ); - testPostgres.resetAllData(); } @After public void teardown() { - testPostgres.resetAllData(); testPostgres.getDataSource().close(); } diff --git a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresMetadataDAOTest.java b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresMetadataDAOTest.java index 138a0c40d4..03b60fb7c2 100644 --- a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresMetadataDAOTest.java +++ b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresMetadataDAOTest.java @@ -72,14 +72,13 @@ public void setup() { postgreSQLContainer = new PostgreSQLContainer<>(DockerImageName.parse("postgres")).withDatabaseName(name.getMethodName().toLowerCase()); postgreSQLContainer.start(); - testUtil = new PostgresDAOTestUtil(postgreSQLContainer, objectMapper, name.getMethodName().toLowerCase()); + testUtil = new PostgresDAOTestUtil(postgreSQLContainer, objectMapper); metadataDAO = new PostgresMetadataDAO(testUtil.getObjectMapper(), testUtil.getDataSource(), testUtil.getTestProperties()); } @After public void teardown() { - testUtil.resetAllData(); testUtil.getDataSource().close(); } diff --git a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresQueueDAOTest.java b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresQueueDAOTest.java index a4daca434b..d56653972a 100644 --- a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresQueueDAOTest.java +++ b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/dao/PostgresQueueDAOTest.java @@ -71,13 +71,12 @@ public void setup() { postgreSQLContainer = new PostgreSQLContainer<>(DockerImageName.parse("postgres")).withDatabaseName(name.getMethodName().toLowerCase()); postgreSQLContainer.start(); - testUtil = new PostgresDAOTestUtil(postgreSQLContainer, objectMapper, name.getMethodName().toLowerCase()); + testUtil = new PostgresDAOTestUtil(postgreSQLContainer, objectMapper); queueDAO = new PostgresQueueDAO(testUtil.getObjectMapper(), testUtil.getDataSource()); } @After public void teardown() { - testUtil.resetAllData(); testUtil.getDataSource().close(); } diff --git a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/util/PostgresDAOTestUtil.java b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/util/PostgresDAOTestUtil.java index 20182aa042..e491ead409 100644 --- a/postgres-persistence/src/test/java/com/netflix/conductor/postgres/util/PostgresDAOTestUtil.java +++ b/postgres-persistence/src/test/java/com/netflix/conductor/postgres/util/PostgresDAOTestUtil.java @@ -17,16 +17,10 @@ import com.zaxxer.hikari.HikariDataSource; import org.flywaydb.core.Flyway; import org.flywaydb.core.api.configuration.FluentConfiguration; -import org.postgresql.ds.PGSimpleDataSource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.testcontainers.containers.PostgreSQLContainer; import javax.sql.DataSource; import java.nio.file.Paths; -import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.SQLException; import java.time.Duration; import static org.mockito.Mockito.mock; @@ -34,45 +28,25 @@ public class PostgresDAOTestUtil { - private static final Logger LOGGER = LoggerFactory.getLogger(PostgresDAOTestUtil.class); private final HikariDataSource dataSource; private final PostgresProperties properties = mock(PostgresProperties.class); private final ObjectMapper objectMapper; - private final DataSource initializationDataSource; - public PostgresDAOTestUtil(PostgreSQLContainer postgreSQLContainer, ObjectMapper objectMapper, String dbName) { - PGSimpleDataSource dataSource = new PGSimpleDataSource(); - dataSource.setURL(postgreSQLContainer.getJdbcUrl()); - dataSource.setDatabaseName(dbName); - dataSource.setUser(postgreSQLContainer.getUsername()); - dataSource.setPassword(postgreSQLContainer.getPassword()); - this.initializationDataSource = dataSource; + public PostgresDAOTestUtil(PostgreSQLContainer postgreSQLContainer, ObjectMapper objectMapper) { this.objectMapper = objectMapper; - when(properties.getJdbcUrl()).thenReturn(postgreSQLContainer.getJdbcUrl()); - when(properties.getJdbcUsername()).thenReturn(postgreSQLContainer.getUsername()); - when(properties.getJdbcPassword()).thenReturn(postgreSQLContainer.getPassword()); - when(properties.isFlywayEnabled()).thenReturn(true); - when(properties.getTaskDefCacheRefreshInterval()).thenReturn(Duration.ofSeconds(60)); - - this.dataSource = getDataSource(properties); - } - - private HikariDataSource getDataSource(PostgresProperties properties) { - - HikariDataSource dataSource = new HikariDataSource(); - dataSource.setJdbcUrl(properties.getJdbcUrl()); - dataSource.setUsername(properties.getJdbcUsername()); - dataSource.setPassword(properties.getJdbcPassword()); + this.dataSource = new HikariDataSource(); + dataSource.setJdbcUrl(postgreSQLContainer.getJdbcUrl()); + dataSource.setUsername(postgreSQLContainer.getUsername()); + dataSource.setPassword(postgreSQLContainer.getPassword()); dataSource.setAutoCommit(false); - // Prevent DB from getting exhausted during rapid testing dataSource.setMaximumPoolSize(8); - flywayMigrate(dataSource); + when(properties.getTaskDefCacheRefreshInterval()).thenReturn(Duration.ofSeconds(60)); - return dataSource; + flywayMigrate(dataSource); } private void flywayMigrate(DataSource dataSource) { @@ -98,26 +72,5 @@ public ObjectMapper getObjectMapper() { return objectMapper; } - public static void dropDb(DataSource ds, String dbName) { - exec(ds, dbName, "DROP", "IF EXISTS"); - } - - private static void exec(DataSource ds, String dbName, String prefix, String suffix) { - - try (Connection connection = ds.getConnection()) { - String stmt = String.format("%s DATABASE %s %s", prefix, suffix, dbName); - try (PreparedStatement ps = connection.prepareStatement(stmt)) { - ps.executeUpdate(); - } - } catch (SQLException ex) { - LOGGER.error(ex.getMessage(), ex); - throw new RuntimeException(ex); - } - } - public void resetAllData() { - LOGGER.info("Resetting data for test"); - dropDb(initializationDataSource, "conductor"); - flywayMigrate(dataSource); - } } diff --git a/server/src/main/java/com/netflix/conductor/Conductor.java b/server/src/main/java/com/netflix/conductor/Conductor.java index ce93e6db91..a2d5980da6 100644 --- a/server/src/main/java/com/netflix/conductor/Conductor.java +++ b/server/src/main/java/com/netflix/conductor/Conductor.java @@ -16,13 +16,16 @@ import org.slf4j.LoggerFactory; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; import org.springframework.core.io.FileSystemResource; import org.springframework.util.StringUtils; import java.io.IOException; import java.util.Properties; -@SpringBootApplication +// Prevents from the datasource beans to be loaded, AS they are needed only for specific databases. +// In case that SQL database is selected this class will be imported back in the appropriate database persistence module. +@SpringBootApplication(exclude = DataSourceAutoConfiguration.class) public class Conductor { private static final Logger log = LoggerFactory.getLogger(Conductor.class); diff --git a/server/src/main/resources/application.properties b/server/src/main/resources/application.properties index 05ef8470c9..364bfbf6d3 100644 --- a/server/src/main/resources/application.properties +++ b/server/src/main/resources/application.properties @@ -13,7 +13,6 @@ spring.application.name=conductor springdoc.api-docs.path=/api-docs -spring.flyway.enabled=false conductor.db.type=memory diff --git a/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/mysql/MySQLGrpcEndToEndTest.java b/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/mysql/MySQLGrpcEndToEndTest.java index 72d793e891..5af65fa177 100644 --- a/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/mysql/MySQLGrpcEndToEndTest.java +++ b/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/mysql/MySQLGrpcEndToEndTest.java @@ -25,12 +25,11 @@ @TestPropertySource(properties = { "conductor.db.type=mysql", "conductor.grpc-server.port=8094", - "conductor.mysql.jdbcUrl=jdbc:tc:mysql:///conductor", // "tc" prefix starts the MySql container - "conductor.mysql.jdbcUsername=root", - "conductor.mysql.jdbcPassword=root", - "conductor.mysql.connectionPoolMaxSize=8", - "conductor.mysql.connectionPoolMinIdle=300000", - "spring.flyway.enabled=false" + "spring.datasource.url=jdbc:tc:mysql:///conductor", // "tc" prefix starts the MySql container + "spring.datasource.username=root", + "spring.datasource.password=root", + "spring.datasource.hikari.maximum-pool-size=8", + "spring.datasource.hikari.minimum-idle=300000" }) public class MySQLGrpcEndToEndTest extends AbstractGrpcEndToEndTest { diff --git a/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/postgres/PostgresGrpcEndToEndTest.java b/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/postgres/PostgresGrpcEndToEndTest.java index 0bd65d32ef..d5e365531b 100644 --- a/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/postgres/PostgresGrpcEndToEndTest.java +++ b/test-harness/src/test/java/com/netflix/conductor/test/integration/grpc/postgres/PostgresGrpcEndToEndTest.java @@ -25,12 +25,11 @@ @TestPropertySource(properties = { "conductor.db.type=postgres", "conductor.grpc-server.port=8098", - "conductor.postgres.jdbcUrl=jdbc:tc:postgresql:///conductor", // "tc" prefix starts the Postgres container - "conductor.postgres.jdbcUsername=postgres", - "conductor.postgres.jdbcPassword=postgres", - "conductor.postgres.connectionPoolMaxSize=8", - "conductor.postgres.connectionPoolMinIdle=300000", - "spring.flyway.enabled=false" + "spring.datasource.url=jdbc:tc:postgresql:///conductor", // "tc" prefix starts the Postgres container + "spring.datasource.username=postgres", + "spring.datasource.password=postgres", + "spring.datasource.hikari.maximum-pool-size=8", + "spring.datasource.hikari.minimum-idle=300000" }) public class PostgresGrpcEndToEndTest extends AbstractGrpcEndToEndTest {