-
Notifications
You must be signed in to change notification settings - Fork 13
Feature/sql healthcheck rework #3857
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,15 +6,13 @@ | |||||
| import com.bakdata.conquery.mode.NamespaceSetupData; | ||||||
| import com.bakdata.conquery.mode.cluster.InternalMapperFactory; | ||||||
| import com.bakdata.conquery.models.config.ConqueryConfig; | ||||||
| import com.bakdata.conquery.models.config.DatabaseConfig; | ||||||
| import com.bakdata.conquery.models.config.DatabaseConnection; | ||||||
| import com.bakdata.conquery.models.config.IdColumnConfig; | ||||||
| import com.bakdata.conquery.models.config.SqlConnectorConfig; | ||||||
| import com.bakdata.conquery.models.identifiable.ids.specific.DatasetId; | ||||||
| import com.bakdata.conquery.models.query.ExecutionManager; | ||||||
| import com.bakdata.conquery.models.worker.DatasetRegistry; | ||||||
| import com.bakdata.conquery.models.worker.LocalNamespace; | ||||||
| import com.bakdata.conquery.sql.DSLContextWrapper; | ||||||
| import com.bakdata.conquery.sql.DslContextFactory; | ||||||
| import com.bakdata.conquery.sql.conquery.SqlExecutionManager; | ||||||
| import com.bakdata.conquery.sql.conversion.NodeConversions; | ||||||
| import com.bakdata.conquery.sql.conversion.SqlConverter; | ||||||
|
|
@@ -37,16 +35,19 @@ public class LocalNamespaceHandler implements NamespaceHandler<LocalNamespace> { | |||||
| private final SqlDialectFactory dialectFactory; | ||||||
|
|
||||||
| @Override | ||||||
| public LocalNamespace createNamespace(NamespaceStorage namespaceStorage, MetaStorage metaStorage, DatasetRegistry<LocalNamespace> datasetRegistry, Environment environment) { | ||||||
| public LocalNamespace createNamespace( | ||||||
| NamespaceStorage namespaceStorage, | ||||||
| MetaStorage metaStorage, | ||||||
| DatasetRegistry<LocalNamespace> datasetRegistry, | ||||||
| Environment environment) { | ||||||
|
|
||||||
| NamespaceSetupData namespaceData = NamespaceHandler.createNamespaceSetup(namespaceStorage, config, internalMapperFactory, datasetRegistry, environment); | ||||||
|
|
||||||
| IdColumnConfig idColumns = config.getIdColumns(); | ||||||
| SqlConnectorConfig sqlConnectorConfig = config.getSqlConnectorConfig(); | ||||||
| DatabaseConfig databaseConfig = sqlConnectorConfig.getDatabaseConfig(namespaceStorage.getDataset()); | ||||||
| DatabaseConnection databaseConfig = sqlConnectorConfig.getDatabaseConfig(namespaceStorage.getDataset()); | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| DSLContextWrapper dslContextWrapper = DslContextFactory.create(databaseConfig, sqlConnectorConfig, environment.healthChecks()); | ||||||
| DSLContext dslContext = dslContextWrapper.getDslContext(); | ||||||
| DSLContext dslContext = databaseConfig.connect(sqlConnectorConfig); | ||||||
| SqlDialect sqlDialect = dialectFactory.createSqlDialect(databaseConfig.getDialect()); | ||||||
|
|
||||||
| boolean valid = dslContext.connectionResult(connection -> connection.isValid(1)); | ||||||
|
|
@@ -68,8 +69,7 @@ public LocalNamespace createNamespace(NamespaceStorage namespaceStorage, MetaSto | |||||
| namespaceData.preprocessMapper(), | ||||||
| namespaceStorage, | ||||||
| executionManager, | ||||||
| dslContextWrapper, | ||||||
| sqlStorageHandler, | ||||||
| dslContext, sqlStorageHandler, | ||||||
| namespaceData.jobManager(), | ||||||
| namespaceData.filterSearch(), | ||||||
| sqlEntityResolver | ||||||
|
|
||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,134 @@ | ||||||
| package com.bakdata.conquery.models.config; | ||||||
|
|
||||||
| import java.io.IOException; | ||||||
| import java.sql.SQLException; | ||||||
|
|
||||||
| import com.codahale.metrics.health.HealthCheckRegistry; | ||||||
| import com.fasterxml.jackson.annotation.JsonIgnore; | ||||||
| import com.google.common.base.Preconditions; | ||||||
| import com.zaxxer.hikari.HikariConfig; | ||||||
| import com.zaxxer.hikari.HikariDataSource; | ||||||
| import io.dropwizard.lifecycle.Managed; | ||||||
| import io.dropwizard.util.Duration; | ||||||
| import lombok.AllArgsConstructor; | ||||||
| import lombok.Builder; | ||||||
| import lombok.Data; | ||||||
| import lombok.NoArgsConstructor; | ||||||
| import lombok.ToString; | ||||||
| import lombok.extern.jackson.Jacksonized; | ||||||
| import lombok.extern.slf4j.Slf4j; | ||||||
| import org.jooq.DSLContext; | ||||||
| import org.jooq.conf.RenderOptionalKeyword; | ||||||
| import org.jooq.conf.RenderQuotedNames; | ||||||
| import org.jooq.conf.Settings; | ||||||
| import org.jooq.impl.DSL; | ||||||
|
|
||||||
| /** | ||||||
| * Connection properties for a SQL database. | ||||||
| * <p/> | ||||||
| * Currently supported are HANA and Prostgres databases, see {@link DatabaseConnection#dialect}. | ||||||
| */ | ||||||
| @Data | ||||||
| @Builder | ||||||
| @Jacksonized | ||||||
| @NoArgsConstructor | ||||||
| @AllArgsConstructor | ||||||
| @Slf4j | ||||||
| public class DatabaseConnection implements Managed { | ||||||
|
|
||||||
| private static final String DEFAULT_PRIMARY_COLUMN = "pid"; | ||||||
|
|
||||||
| /** | ||||||
| * SQL vendor specific dialect used to transform queries to SQL | ||||||
| */ | ||||||
| private Dialect dialect; | ||||||
|
|
||||||
| /** | ||||||
| * Username used to connect to the database. | ||||||
| */ | ||||||
| private String databaseUsername; | ||||||
|
|
||||||
|
|
||||||
| /** | ||||||
| * Password used to connect to the database. | ||||||
| */ | ||||||
| @ToString.Exclude | ||||||
| private String databasePassword; | ||||||
|
|
||||||
| /** | ||||||
| * Connections url in JDBC notation. | ||||||
| */ | ||||||
| private String jdbcConnectionUrl; | ||||||
|
|
||||||
| private Duration connectivityTimeout; | ||||||
|
|
||||||
| /** | ||||||
| * Name of the column which is shared among the table and all aggregations are grouped by. | ||||||
| */ | ||||||
| @Builder.Default | ||||||
| private String primaryColumn = DEFAULT_PRIMARY_COLUMN; | ||||||
|
|
||||||
| @JsonIgnore | ||||||
| private HikariDataSource dataSource; | ||||||
|
|
||||||
| @JsonIgnore | ||||||
| private HealthCheckRegistry healthCheckRegistry; | ||||||
|
Comment on lines
+71
to
+75
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. State bitte in einem Service halten, nicht in der Config. |
||||||
|
|
||||||
| @Override | ||||||
| public void start() throws Exception { | ||||||
| initializeDataSource(); | ||||||
| } | ||||||
|
Comment on lines
+77
to
+80
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trenne bitte die Config von dem Managed, so dass initializeDataSource gekapselt ist und nur einmal pro Connection aufgerufen werden kann |
||||||
|
|
||||||
| public void initializeDataSource() { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Zum Beispiel so
Suggested change
|
||||||
| HikariConfig hikariConfig = new HikariConfig(); | ||||||
| hikariConfig.setJdbcUrl(getJdbcConnectionUrl()); | ||||||
| hikariConfig.setUsername(getDatabaseUsername()); | ||||||
| hikariConfig.setPassword(getDatabasePassword()); | ||||||
|
|
||||||
| if (healthCheckRegistry != null) { | ||||||
| hikariConfig.setHealthCheckRegistry(healthCheckRegistry); | ||||||
| if (getConnectivityTimeout() != null) { | ||||||
| long connectivityTimeoutMs = getConnectivityTimeout().toMilliseconds(); | ||||||
| hikariConfig.addHealthCheckProperty("connectivityCheckTimeoutMs", Long.toString(connectivityTimeoutMs)); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| dataSource = new HikariDataSource(hikariConfig); | ||||||
|
|
||||||
| try { | ||||||
| log.debug("TEST connecting to {}", getJdbcConnectionUrl()); | ||||||
| if (dataSource.getConnection().isValid(100)) { | ||||||
| log.info("SUCCESS connecting to {}", getJdbcConnectionUrl()); | ||||||
| } | ||||||
| else { | ||||||
| log.error("FAILED connecting to {}. Connection did not become valid.", getJdbcConnectionUrl()); | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sind in der Url Credentials enthalten? |
||||||
| } | ||||||
| } | ||||||
| catch (SQLException exception) { | ||||||
| log.error("FAILED connecting to {}", getJdbcConnectionUrl(), exception); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| public DSLContext connect(SqlConnectorConfig connectorConfig) { | ||||||
| Preconditions.checkNotNull(this.dataSource, "dataSource has not been initialized yet."); | ||||||
|
|
||||||
| Settings settings = new Settings() | ||||||
| .withRenderFormatted(connectorConfig.isWithPrettyPrinting()) | ||||||
| // enforces all identifiers to be quoted if not explicitly unquoted via DSL.unquotedName() | ||||||
| // to prevent any lowercase/uppercase SQL dialect specific identifier naming issues | ||||||
| .withRenderQuotedNames(RenderQuotedNames.EXPLICIT_DEFAULT_QUOTED) | ||||||
| // always render "as" keyword for field aliases | ||||||
| .withRenderOptionalAsKeywordForFieldAliases(RenderOptionalKeyword.ON); | ||||||
|
|
||||||
| return DSL.using( | ||||||
| this.dataSource, | ||||||
| getDialect().getJooqDialect(), | ||||||
| settings | ||||||
| ); | ||||||
| } | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bitte diese Methode auch in eine ManagedDatasource packen |
||||||
|
|
||||||
| @Override | ||||||
| public void stop() throws IOException { | ||||||
| dataSource.close(); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,32 @@ | ||
| package com.bakdata.conquery.models.config; | ||
|
|
||
| import lombok.Getter; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.jooq.SQLDialect; | ||
|
|
||
| /** | ||
| * The dialect sets SQL vendor specific query transformation rules. | ||
| * <p/> | ||
| * There is no fallback dialect, so the dialect must fit the targeted database. | ||
| */ | ||
| @RequiredArgsConstructor | ||
| @Getter | ||
| public enum Dialect { | ||
|
|
||
| /** | ||
| * Dialect for PostgreSQL database | ||
| */ | ||
| POSTGRESQL(SQLDialect.POSTGRES, 63), | ||
| POSTGRESQL(SQLDialect.POSTGRES, 63, "SELECT 1"), | ||
| /** | ||
| * Dialect for SAP HANA database | ||
| */ | ||
| HANA(SQLDialect.DEFAULT, 127); | ||
| HANA(SQLDialect.DEFAULT, 127, "SELECT 1 FROM DUMMY"); | ||
|
|
||
| private final SQLDialect jooqDialect; | ||
|
|
||
| /** | ||
| * Set's the max length of database identifiers (column names, qualifiers, etc.). | ||
| */ | ||
| private final int nameMaxLength; | ||
|
|
||
| Dialect(SQLDialect jooqDialect, int nameMaxLength) { | ||
| this.jooqDialect = jooqDialect; | ||
| this.nameMaxLength = nameMaxLength; | ||
| } | ||
|
|
||
| private final String testConnection; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,14 +4,17 @@ | |
| import jakarta.validation.Valid; | ||
|
|
||
| import com.bakdata.conquery.models.datasets.Dataset; | ||
| import com.bakdata.conquery.models.identifiable.ids.specific.DatasetId; | ||
| import com.fasterxml.jackson.annotation.JsonIgnore; | ||
| import io.dropwizard.util.Duration; | ||
| import io.dropwizard.core.setup.Environment; | ||
| import io.dropwizard.lifecycle.Managed; | ||
| import io.dropwizard.validation.ValidationMethod; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.NoArgsConstructor; | ||
| import lombok.extern.jackson.Jacksonized; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| /** | ||
| * Configuration for SQL databases to send dataset queries to. | ||
|
|
@@ -25,7 +28,8 @@ | |
| @Jacksonized | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class SqlConnectorConfig { | ||
| @Slf4j | ||
| public class SqlConnectorConfig implements Managed { | ||
|
|
||
| private boolean enabled; | ||
|
|
||
|
|
@@ -37,15 +41,23 @@ public class SqlConnectorConfig { | |
| /** | ||
| * Keys must match the name of existing {@link Dataset}s. | ||
| */ | ||
| private Map<String, @Valid DatabaseConfig> databaseConfigs; | ||
| private Map<DatasetId, @Valid DatabaseConnection> databaseConfigs; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Funktioniert die Deserialisierung der Keys einfach so ohne Annotationen? |
||
|
|
||
| /** | ||
| * Timeout duration after which a database connection is considered unhealthy (defaults to connection timeout) | ||
| */ | ||
| private Duration connectivityCheckTimeout; | ||
|
|
||
| public DatabaseConfig getDatabaseConfig(Dataset dataset) { | ||
| return databaseConfigs.get(dataset.getName()); | ||
| public DatabaseConnection getDatabaseConfig(Dataset dataset) { | ||
| return databaseConfigs.get(dataset.getId()); | ||
| } | ||
|
|
||
|
|
||
| public void initialize(Environment environment) { | ||
| if(databaseConfigs == null || !enabled){ | ||
| return; | ||
| } | ||
|
|
||
| for (DatabaseConnection connection : databaseConfigs.values()) { | ||
| connection.setHealthCheckRegistry(environment.healthChecks()); | ||
| environment.lifecycle().manage(connection); | ||
| } | ||
| } | ||
|
|
||
| @JsonIgnore | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oder ein ConfiguredBundle, da kannst du checken ob SqlConnectorConfig gesetzt/enabled ist, Resources/Services registrieren für Injection, Healthchecks setzten...