Skip to content

Commit

Permalink
[#662] cli option to disable connection pool (#669)
Browse files Browse the repository at this point in the history
Introduce cli option to disable internal database connection pooling library. This can be used to use an external connection pool mechanism such as pgBouncer
  • Loading branch information
usmansaleem authored Nov 10, 2022
1 parent e03eb5f commit 381bc57
Show file tree
Hide file tree
Showing 13 changed files with 181 additions and 35 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
- Introduced cli option to specify Hikari configuration for pruning database connection [#661](https://github.com/ConsenSys/web3signer/issues/661)
- Better database pruning default values: Pruning enabled by default with `slashing-protection-pruning-epochs-to-keep = 250`, `slashing-protection-pruning-at-boot-enabled = false` and `slashing-protection-pruning-interval = 12`
- Improved performance for slashing protection import
- Introduced experimental cli option `--Xslashing-protection-db-connection-pool-enabled` to disable internal database connection
pool (Hikari) to allow using external database connection pool such as pgBouncer. `--slashing-protection-db-pool-configuration-file` and
`--slashing-protection-pruning-db-pool-configuration-file` can be reused to specify PG Datasource properties.
[#662](https://github.com/ConsenSys/web3signer/issues/662)

---

## 22.10.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public class SignerConfiguration {
private final Optional<String> slashingProtectionDbUrl;
private final String slashingProtectionDbUsername;
private final String slashingProtectionDbPassword;
private final boolean slashingProtectionDbConnectionPoolEnabled;
private final Optional<Path> slashingProtectionDbPoolConfigurationFile;
private final Optional<Map<String, String>> web3SignerEnvironment;
private final boolean enableSlashing;
Expand Down Expand Up @@ -85,6 +86,7 @@ public SignerConfiguration(
final Optional<String> slashingProtectionDbUrl,
final String slashingProtectionDbUsername,
final String slashingProtectionDbPassword,
final boolean slashingProtectionDbConnectionPoolEnabled,
final String mode,
final Optional<Map<String, String>> web3SignerEnvironment,
final Duration startupTimeout,
Expand Down Expand Up @@ -120,6 +122,7 @@ public SignerConfiguration(
this.slashingProtectionDbUrl = slashingProtectionDbUrl;
this.slashingProtectionDbUsername = slashingProtectionDbUsername;
this.slashingProtectionDbPassword = slashingProtectionDbPassword;
this.slashingProtectionDbConnectionPoolEnabled = slashingProtectionDbConnectionPoolEnabled;
this.mode = mode;
this.web3SignerEnvironment = web3SignerEnvironment;
this.startupTimeout = startupTimeout;
Expand Down Expand Up @@ -287,4 +290,8 @@ public boolean isKeyManagerApiEnabled() {
public Duration getStartupTimeout() {
return startupTimeout;
}

public boolean isSlashingProtectionDbConnectionPoolEnabled() {
return slashingProtectionDbConnectionPoolEnabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class SignerConfigurationBuilder {
Boolean.getBoolean("debugSubProcess") ? Duration.ofHours(1) : Duration.ofSeconds(30);
private boolean enableSlashing = false;
private String slashingProtectionDbUrl;
private boolean slashingProtectionDbConnectionPoolEnabled = true;
private Path slashingExportPath;
private Path slashingImportPath;
private boolean enableSlashingPruning = false;
Expand Down Expand Up @@ -166,6 +167,12 @@ public SignerConfigurationBuilder withSlashingProtectionDbPoolConfigurationFile(
return this;
}

public SignerConfigurationBuilder withSlashingProtectionDbConnectionPoolEnabled(
final boolean dbConnectionPoolEnabled) {
this.slashingProtectionDbConnectionPoolEnabled = dbConnectionPoolEnabled;
return this;
}

public SignerConfigurationBuilder withSlashingEnabled(final boolean enableSlashing) {
this.enableSlashing = enableSlashing;
return this;
Expand Down Expand Up @@ -277,6 +284,7 @@ public SignerConfiguration build() {
Optional.ofNullable(slashingProtectionDbUrl),
slashingProtectionDbUsername,
slashingProtectionDbPassword,
slashingProtectionDbConnectionPoolEnabled,
mode,
Optional.ofNullable(web3SignerEnvironment),
startupTimeout,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,15 @@ private String createEth2SlashingProtectionArgs() {
"eth2.slashing-protection-db-pool-configuration-file",
signerConfig.getSlashingProtectionDbPoolConfigurationFile()));
}

// enabled by default, explicitly set when false
if (!signerConfig.isSlashingProtectionDbConnectionPoolEnabled()) {
yamlConfig.append(
String.format(
YAML_BOOLEAN_FMT,
"eth2.Xslashing-protection-db-connection-pool-enabled",
signerConfig.isSlashingProtectionDbConnectionPoolEnabled()));
}
}

if (signerConfig.isSlashingProtectionPruningEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ private Collection<String> createEth2Args() {
params.add("--slashing-protection-db-pool-configuration-file");
params.add(signerConfig.getSlashingProtectionDbPoolConfigurationFile().toString());
}

// enabled by default, explicitly set when false
if (!signerConfig.isSlashingProtectionDbConnectionPoolEnabled()) {
params.add("--Xslashing-protection-db-connection-pool-enabled=false");
}
}

if (signerConfig.getSlashingExportPath().isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
public class DatabaseUtil {
public static final String USERNAME = "postgres";
public static final String PASSWORD = "postgres";
public static final boolean DB_CONNECTION_POOL_ENABLED = true;

public static TestDatabaseInfo create() {
try {
Expand All @@ -41,7 +42,11 @@ public static TestDatabaseInfo create() {
String.format("jdbc:postgresql://localhost:%d/postgres", db.getPort());
final Jdbi jdbi =
DbConnection.createConnection(
databaseUrl, DatabaseUtil.USERNAME, DatabaseUtil.PASSWORD, null);
databaseUrl,
DatabaseUtil.USERNAME,
DatabaseUtil.PASSWORD,
null,
DB_CONNECTION_POOL_ENABLED);
return new TestDatabaseInfo(db, jdbi, flyway);
} catch (IOException e) {
throw new UncheckedIOException("Unable to create embedded postgres database", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.apache.tuweni.bytes.Bytes32;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

public class SlashingProtectionAcceptanceTest extends AcceptanceTestBase {

Expand All @@ -52,14 +54,23 @@ public class SlashingProtectionAcceptanceTest extends AcceptanceTestBase {
"eth2_slashingprotection_permitted_signings",
"eth2_slashingprotection_prevented_signings");

void setupSigner(final Path testDirectory, final boolean enableSlashing) {
void setupSigner(final Path testDirectory) {
setupSigner(testDirectory, true, true);
}

void setupSigner(
final Path testDirectory,
final boolean enableSlashing,
final boolean slashingProtectionDbConnectionPoolEnabled) {
final SignerConfigurationBuilder builder =
new SignerConfigurationBuilder()
.withMetricsCategories("ETH2_SLASHING_PROTECTION")
.withMode("eth2")
.withSlashingEnabled(enableSlashing)
.withSlashingProtectionDbUsername(DB_USERNAME)
.withSlashingProtectionDbPassword(DB_PASSWORD)
.withSlashingProtectionDbConnectionPoolEnabled(
slashingProtectionDbConnectionPoolEnabled)
.withMetricsEnabled(true)
.withNetwork("minimal")
.withKeyStoreDirectory(testDirectory);
Expand All @@ -71,11 +82,12 @@ void setupSigner(final Path testDirectory, final boolean enableSlashing) {
startSigner(builder.build());
}

@Test
void canSignSameAttestationTwiceWhenSlashingIsEnabled(@TempDir Path testDirectory)
throws JsonProcessingException {
@ParameterizedTest
@ValueSource(booleans = {true, false})
void canSignSameAttestationTwiceWhenSlashingIsEnabled(
boolean dbConnectionPoolEnabled, @TempDir Path testDirectory) throws JsonProcessingException {

setupSigner(testDirectory, true);
setupSigner(testDirectory, true, dbConnectionPoolEnabled);

final Eth2SigningRequestBody request = createAttestationRequest(5, 6, UInt64.ZERO);

Expand All @@ -97,7 +109,7 @@ void canSignSameAttestationTwiceWhenSlashingIsEnabled(@TempDir Path testDirector
@Test
void cannotSignASecondAttestationForSameSlotWithDifferentSigningRoot(@TempDir Path testDirectory)
throws JsonProcessingException {
setupSigner(testDirectory, true);
setupSigner(testDirectory);

final Eth2SigningRequestBody initialRequest = createAttestationRequest(5, 6, UInt64.ZERO);

Expand All @@ -123,7 +135,7 @@ void cannotSignASecondAttestationForSameSlotWithDifferentSigningRoot(@TempDir Pa
@Test
void cannotSignSurroundedAttestationWhenSlashingEnabled(@TempDir Path testDirectory)
throws JsonProcessingException {
setupSigner(testDirectory, true);
setupSigner(testDirectory);

final Eth2SigningRequestBody initialRequest = createAttestationRequest(3, 6, UInt64.ONE);
final Response initialResponse =
Expand All @@ -141,7 +153,7 @@ void cannotSignSurroundedAttestationWhenSlashingEnabled(@TempDir Path testDirect
@Test
void cannotSignASurroundingAttestationWhenSlashingEnabled(@TempDir Path testDirectory)
throws JsonProcessingException {
setupSigner(testDirectory, true);
setupSigner(testDirectory);

final Eth2SigningRequestBody initialRequest = createAttestationRequest(3, 6, UInt64.ONE);
final Response initialResponse =
Expand All @@ -160,7 +172,7 @@ void cannotSignASurroundingAttestationWhenSlashingEnabled(@TempDir Path testDire
void canSignSameBlockTwiceWhenSlashingIsEnabled(@TempDir Path testDirectory)
throws JsonProcessingException {

setupSigner(testDirectory, true);
setupSigner(testDirectory);

final Eth2SigningRequestBody request =
createBlockRequest(
Expand All @@ -182,7 +194,7 @@ void canSignSameBlockTwiceWhenSlashingIsEnabled(@TempDir Path testDirectory)
@Test
void signingBlockWithDifferentSigningRootForPreviousSlotFailsWith412(@TempDir Path testDirectory)
throws JsonProcessingException {
setupSigner(testDirectory, true);
setupSigner(testDirectory);

final Eth2SigningRequestBody initialRequest =
createBlockRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ public class PicoCliSlashingProtectionParameters implements SlashingProtectionPa
arity = "1")
private long dbHealthCheckIntervalMilliseconds = 30000;

@Option(
names = "--Xslashing-protection-db-connection-pool-enabled",
description =
"Set to false to disable internal database connection pooling. Should only be disabled when using an external database connection pool. (Default: ${DEFAULT-VALUE})",
paramLabel = "<BOOL>",
arity = "1",
hidden = true)
private boolean dbConnectionPoolEnabled = true;

@Override
public boolean isEnabled() {
return enabled;
Expand Down Expand Up @@ -184,4 +193,9 @@ public long getDbHealthCheckTimeoutMilliseconds() {
public long getDbHealthCheckIntervalMilliseconds() {
return dbHealthCheckIntervalMilliseconds;
}

@Override
public boolean isDbConnectionPoolEnabled() {
return dbConnectionPoolEnabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Path;
import java.sql.SQLException;
import java.time.Duration;
import java.util.Properties;
import javax.sql.DataSource;
Expand All @@ -36,6 +37,7 @@
import org.jdbi.v3.core.argument.Arguments;
import org.jdbi.v3.core.mapper.ColumnMappers;
import org.jdbi.v3.core.transaction.SerializableTransactionRunner;
import org.postgresql.ds.PGSimpleDataSource;

public class DbConnection {
// https://jdbc.postgresql.org/documentation/head/connect.html#connection-parameters
Expand All @@ -46,8 +48,14 @@ public static Jdbi createConnection(
final String jdbcUrl,
final String username,
final String password,
final Path configurationFile) {
final DataSource datasource = createDataSource(jdbcUrl, username, password, configurationFile);
final Path configurationFile,
final boolean dbConnectionPoolEnabled) {
final DataSource datasource;
if (dbConnectionPoolEnabled) {
datasource = createHikariDataSource(jdbcUrl, username, password, configurationFile);
} else {
datasource = createPGDataSource(jdbcUrl, username, password, configurationFile);
}
final Jdbi jdbi = Jdbi.create(datasource);
configureJdbi(jdbi);
return jdbi;
Expand All @@ -57,10 +65,17 @@ public static Jdbi createPruningConnection(
final String jdbcUrl,
final String username,
final String password,
final Path configurationFile) {
final HikariDataSource datasource =
createDataSource(jdbcUrl, username, password, configurationFile);
datasource.setMaximumPoolSize(1); // we only need 1 connection in pool for pruning
final Path configurationFile,
final boolean dbConnectionPoolEnabled) {
final DataSource datasource;
if (dbConnectionPoolEnabled) {
final HikariDataSource hkDatasource =
createHikariDataSource(jdbcUrl, username, password, configurationFile);
hkDatasource.setMaximumPoolSize(1); // we only need 1 connection in pool for pruning
datasource = hkDatasource;
} else {
datasource = createPGDataSource(jdbcUrl, username, password, configurationFile);
}
final Jdbi jdbi = Jdbi.create(datasource);
configureJdbi(jdbi);
return jdbi;
Expand All @@ -77,7 +92,7 @@ public static void configureJdbi(final Jdbi jdbi) {
jdbi.setTransactionHandler(new SerializableTransactionRunner());
}

private static HikariDataSource createDataSource(
private static HikariDataSource createHikariDataSource(
final String jdbcUrl,
final String username,
final String password,
Expand All @@ -97,28 +112,74 @@ private static HikariDataSource createDataSource(
return new HikariDataSource(hikariConfig);
}

private static DataSource createPGDataSource(
final String jdbcUrl,
final String username,
final String password,
final Path propertiesFile) {
final PGSimpleDataSource pgSimpleDataSource = new PGSimpleDataSource();
pgSimpleDataSource.setURL(jdbcUrl);

if (!isEmpty(username)) {
pgSimpleDataSource.setUser(username);
}
if (!isEmpty(password)) {
pgSimpleDataSource.setPassword(password);
}

final Properties properties = loadPGConfigurationProperties(propertiesFile);
properties.forEach(
(k, v) -> {
try {
pgSimpleDataSource.setProperty(k.toString(), v.toString());
} catch (final SQLException e) {
throw new RuntimeException("Error setting Datasource Property.", e);
}
});

return pgSimpleDataSource;
}

@VisibleForTesting
static Properties loadHikariConfigurationProperties(final Path hikariConfigurationFile) {
final Properties hikariConfigurationProperties = new Properties();
if (hikariConfigurationFile != null) {
try (FileInputStream inputStream = new FileInputStream(hikariConfigurationFile.toFile())) {
hikariConfigurationProperties.load(inputStream);
static Properties loadHikariConfigurationProperties(final Path configurationFile) {
return addDefaultHikariDatasourceTimeoutProperty(
loadDatasourcePropertiesFile(configurationFile));
}

static Properties loadPGConfigurationProperties(final Path configurationFile) {
return addDefaultPGDatasourceTimeoutProperty(loadDatasourcePropertiesFile(configurationFile));
}

private static Properties loadDatasourcePropertiesFile(final Path configurationFile) {
final Properties datasourceConfigurationProperties = new Properties();
if (configurationFile != null) {
try (FileInputStream inputStream = new FileInputStream(configurationFile.toFile())) {
datasourceConfigurationProperties.load(inputStream);
} catch (final FileNotFoundException e) {
throw new UncheckedIOException(
"Hikari configuration file not found: " + hikariConfigurationFile, e);
"Datasource configuration file not found: " + configurationFile, e);
} catch (final IOException e) {
final String errorMessage =
String.format(
"Unexpected IO error while reading Hikari configuration file [%s]",
hikariConfigurationFile);
"Unexpected IO error while reading Datasource configuration file [%s]",
configurationFile);
throw new UncheckedIOException(errorMessage, e);
}
}
return datasourceConfigurationProperties;
}

private static Properties addDefaultHikariDatasourceTimeoutProperty(final Properties properties) {
if (!properties.containsKey("dataSource." + PG_SOCKET_TIMEOUT_PARAM)) {
properties.put("dataSource." + PG_SOCKET_TIMEOUT_PARAM, DEFAULT_PG_SOCKET_TIMEOUT_SECONDS);
}
return properties;
}

if (!hikariConfigurationProperties.containsKey("dataSource." + PG_SOCKET_TIMEOUT_PARAM)) {
hikariConfigurationProperties.put(
"dataSource." + PG_SOCKET_TIMEOUT_PARAM, DEFAULT_PG_SOCKET_TIMEOUT_SECONDS);
private static Properties addDefaultPGDatasourceTimeoutProperty(final Properties properties) {
if (!properties.containsKey(PG_SOCKET_TIMEOUT_PARAM)) {
properties.put(PG_SOCKET_TIMEOUT_PARAM, DEFAULT_PG_SOCKET_TIMEOUT_SECONDS);
}
return hikariConfigurationProperties;
return properties;
}
}
Loading

0 comments on commit 381bc57

Please sign in to comment.