Skip to content

Commit

Permalink
Validate vertx pool size options (#947)
Browse files Browse the repository at this point in the history
* Validate vertx pool size options. The deprecated worker pool size and new vertx worker pool size cli options are mutually exclusive.
  • Loading branch information
usmansaleem committed Nov 9, 2023
1 parent b001351 commit 4db92ab
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
subcommands = {HelpCommand.class},
footer = "Web3Signer is licensed under the Apache License 2.0")
public class Web3SignerBaseCommand implements BaseConfig, Runnable {

private static final int VERTX_WORKER_POOL_SIZE_DEFAULT = 20;
@Spec private CommandLine.Model.CommandSpec spec; // injected by picocli
public static final String KEY_STORE_CONFIG_FILE_SIZE_OPTION_NAME =
"--key-store-config-file-max-size";
Expand Down Expand Up @@ -207,9 +207,11 @@ public class Web3SignerBaseCommand implements BaseConfig, Runnable {
@Option(
names = "--vertx-worker-pool-size",
description =
"Configure the Vert.x worker pool size used for processing requests. (default: ${DEFAULT-VALUE})",
"Configure the Vert.x worker pool size used for processing requests. (default: "
+ VERTX_WORKER_POOL_SIZE_DEFAULT
+ ")",
paramLabel = INTEGER_FORMAT_HELP)
private int vertxWorkerPoolSize = 20;
private Integer vertxWorkerPoolSize = null;

@Deprecated
@Option(names = "--Xworker-pool-size", hidden = true)
Expand Down Expand Up @@ -321,10 +323,20 @@ public boolean keystoreParallelProcessingEnabled() {

@Override
public int getVertxWorkerPoolSize() {
// both values are not allowed on cli, they will be verified in validateArgs() ...
if (vertxWorkerPoolSize != null && deprecatedWorkerPoolSize != null) {
return -1;
}

if (vertxWorkerPoolSize != null) {
return vertxWorkerPoolSize;
}

if (deprecatedWorkerPoolSize != null) {
return deprecatedWorkerPoolSize;
}
return vertxWorkerPoolSize;

return VERTX_WORKER_POOL_SIZE_DEFAULT;
}

@Override
Expand Down Expand Up @@ -374,6 +386,12 @@ public void validateArgs() {
"--metrics-enabled option and --metrics-push-enabled option can't be used at the same "
+ "time. Please refer to CLI reference for more details about this constraint.");
}

if (vertxWorkerPoolSize != null && deprecatedWorkerPoolSize != null) {
throw new CommandLine.MutuallyExclusiveArgsException(
spec.commandLine(),
"--vertx-worker-pool-size option and --Xworker-pool-size option can't be used at the same time.");
}
}

public static class Web3signerMetricCategoryConverter extends MetricCategoryConverter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,60 @@ void awsWithoutModeDefaultsToSpecified() {
.contains("v1", "v2", "v3");
}

@Test
void vertxWorkerPoolSizeWithWorkerPoolSizeFailsToParse() {
String cmdline = validBaseCommandOptions();
cmdline +=
"--vertx-worker-pool-size=30 --Xworker-pool-size=40 eth2 --slashing-protection-enabled=false";

parser.registerSubCommands(new MockEth2SubCommand());
final int result = parser.parseCommandLine(cmdline.split(" "));

assertThat(result).isNotZero();
assertThat(commandError.toString())
.contains(
"Error parsing parameters: --vertx-worker-pool-size option and --Xworker-pool-size option can't be used at the same time.");
}

@Test
void vertxWorkerPoolSizeDefaultParsesSuccessfully() {
String cmdline = validBaseCommandOptions();
cmdline += "eth2 --slashing-protection-enabled=false";

MockEth2SubCommand mockEth2SubCommand = new MockEth2SubCommand();
parser.registerSubCommands(mockEth2SubCommand);
final int result = parser.parseCommandLine(cmdline.split(" "));

assertThat(result).isZero();
assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(20);
}

@Test
void vertxWorkerPoolSizeDeprecatedParsesSuccessfully() {
String cmdline = validBaseCommandOptions();
cmdline += "--Xworker-pool-size=40 eth2 --slashing-protection-enabled=false";

MockEth2SubCommand mockEth2SubCommand = new MockEth2SubCommand();
parser.registerSubCommands(mockEth2SubCommand);
final int result = parser.parseCommandLine(cmdline.split(" "));

assertThat(result).isZero();
assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(40);
}

@Test
void vertxWorkerPoolSizeParsesSuccessfully() {
String cmdline = validBaseCommandOptions();
cmdline += "--vertx-worker-pool-size=40 eth2 --slashing-protection-enabled=false";

MockEth2SubCommand mockEth2SubCommand = new MockEth2SubCommand();
parser.registerSubCommands(mockEth2SubCommand);
final int result = parser.parseCommandLine(cmdline.split(" "));

assertThat(result).isZero();
assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(40);
}

private <T> void missingOptionalParameterIsValidAndMeetsDefault(
final String paramToRemove, final Supplier<T> actualValueGetter, final T expectedValue) {

Expand All @@ -573,6 +627,10 @@ public static class MockEth2SubCommand extends Eth2SubCommand {
public Runner createRunner() {
return new NoOpRunner(config);
}

public BaseConfig getConfig() {
return config;
}
}

public static class NoOpRunner extends Runner {
Expand Down

0 comments on commit 4db92ab

Please sign in to comment.