Skip to content
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

Add benchmarking application. No test clients yet. #629

5 changes: 4 additions & 1 deletion java/benchmarks/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ dependencies {
implementation 'io.lettuce:lettuce-core:6.2.6.RELEASE'
implementation 'commons-cli:commons-cli:1.5.0'
implementation group: 'org.apache.commons', name: 'commons-lang3', version: '3.13.0'
implementation group: 'org.apache.commons', name: 'commons-math3', version: '3.5'
implementation group: 'com.google.code.gson', name: 'gson', version: '2.10.1'

}

// Apply a specific Java toolchain to ease working on different environments.
Expand All @@ -32,7 +35,7 @@ application {
mainClass = 'javababushka.benchmarks.BenchmarkingApp'
}

tasks.withType(Test) {
tasks.withType(Test) {
testLogging {
exceptionFormat "full"
events "started", "skipped", "passed", "failed"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package javababushka.benchmarks;

import java.io.FileWriter;
import java.io.IOException;
import java.util.Arrays;
import java.util.Optional;
import java.util.stream.Stream;
Expand Down Expand Up @@ -37,40 +35,24 @@ public static void main(String[] args) {
runConfiguration = verifyOptions(line);
} catch (ParseException exp) {
// oops, something went wrong
System.err.println("Parsing failed. Reason: " + exp.getMessage());
System.err.println("Parsing failed. Reason: " + exp.getMessage());
}

for (ClientName client : runConfiguration.clients) {
switch (client) {
case JEDIS:
// run testClientSetGet on JEDIS sync client
System.out.println("Run JEDIS sync client");
break;
case JEDIS_ASYNC:
// run testClientSetGet on JEDIS pseudo-async client
System.out.println("Run JEDIS pseudo-async client");
// run testClientSetGet on JEDIS pseudo-sync client
System.out.println("Run JEDIS async client");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you log Jedis as async? there's no such thing, and we shouldn't create it.
Babushka & Lettuce are async, Jedis is sync. that's all we need.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it was uncleaned reference to that client. It is already deleted.
Fixed in d3f707f.

break;
case LETTUCE:
// run testClientSetGet on LETTUCE sync client
System.out.println("Run LETTUCE sync client");
break;
case LETTUCE_ASYNC:
// run testClientSetGet on LETTUCE async client
System.out.println("Run LETTUCE async client");
break;
case BABUSHKA_ASYNC:
case BABUSHKA:
System.out.println("Babushka async not yet configured");
break;
}
}

if (runConfiguration.resultsFile.isPresent()) {
try {
runConfiguration.resultsFile.get().close();
} catch (IOException ioException) {
System.out.println("Error closing results file: " + ioException.getLocalizedMessage());
}
}
}

private static Options getOptions() {
Expand All @@ -96,15 +78,21 @@ private static Options getOptions() {
Option.builder("clients")
.hasArg(true)
.desc(
"one of:"
+ " all|jedis|jedis_async|lettuce|lettuce_async|babushka_async|all_async|all_sync"
+ " [all]")
"one of: all|jedis|jedis_async|lettuce|lettuce_async|"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should remove all irrelevant clients

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d3f707f.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just all |jedis|lettuce|babushka
(if you also want all sync / all async that's fine, but I don't see the benefit there)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, thank you.
Fixed in d3f707f.

+ "babushka|babushka_async|all_async|all_sync")
.build());
options.addOption(Option.builder("host").hasArg(true).desc("Hostname [localhost]").build());
options.addOption(Option.builder("port").hasArg(true).desc("Port number [6379]").build());
options.addOption(
Option.builder("clientCount").hasArg(true).desc("Number of clients to run [1]").build());
options.addOption(Option.builder("tls").hasArg(false).desc("TLS [false]").build());
options.addOption(
Option.builder("clusterModeEnabled")
.hasArg(false)
.desc("Is cluster-mode enabled, other standalone mode is used [false]")
.build());
options.addOption(
Option.builder("debugLogging").hasArg(false).desc("Verbose logs [false]").build());

return options;
}
Expand All @@ -123,16 +111,11 @@ private static RunConfiguration verifyOptions(CommandLine line) throws ParseExce
}

if (line.hasOption("resultsFile")) {
String resultsFileName = line.getOptionValue("resultsFile");
try {
runConfiguration.resultsFile = Optional.of(new FileWriter(resultsFileName));
} catch (IOException ioException) {
throw new ParseException(
"Unable to write to resultsFile ("
+ resultsFileName
+ "): "
+ ioException.getMessage());
}
runConfiguration.resultsFile = Optional.ofNullable(line.getOptionValue("resultsFile"));
}

if (line.hasOption("dataSize")) {
runConfiguration.dataSize = parseIntListOption(line.getOptionValue("dataSize"));
}

if (line.hasOption("concurrentTasks")) {
Expand All @@ -148,22 +131,7 @@ private static RunConfiguration verifyOptions(CommandLine line) throws ParseExce
e -> {
switch (e) {
case ALL:
return Stream.of(
ClientName.JEDIS,
ClientName.JEDIS_ASYNC,
// ClientName.BABUSHKA_ASYNC,
ClientName.LETTUCE,
ClientName.LETTUCE_ASYNC);
case ALL_ASYNC:
return Stream.of(
ClientName.JEDIS_ASYNC,
// ClientName.BABUSHKA_ASYNC,
ClientName.LETTUCE_ASYNC);
case ALL_SYNC:
return Stream.of(
ClientName.JEDIS,
// ClientName.BABUSHKA_ASYNC,
ClientName.LETTUCE);
return Stream.of(ClientName.JEDIS, ClientName.BABUSHKA, ClientName.LETTUCE);
default:
return Stream.of(e);
}
Expand All @@ -184,6 +152,8 @@ private static RunConfiguration verifyOptions(CommandLine line) throws ParseExce
}

runConfiguration.tls = line.hasOption("tls");
runConfiguration.clusterModeEnabled = line.hasOption("clusterModeEnabled");
runConfiguration.debugLogging = line.hasOption("debugLogging");

return runConfiguration;
}
Expand All @@ -195,23 +165,23 @@ private static int[] parseIntListOption(String line) throws ParseException {
if (lineValue.startsWith("[") && lineValue.endsWith("]")) {
lineValue = lineValue.substring(1, lineValue.length() - 1);
}

// trim whitespace
lineValue = lineValue.trim();

// check if it's the correct format
if (!lineValue.matches("\\d+(\\s+\\d+)?")) {
if (!lineValue.matches("\\d+(\\s+\\d+)*")) {
throw new ParseException("Invalid option: " + line);
}
// split the string into a list of integers
return Arrays.stream(lineValue.split("\\s+")).mapToInt(Integer::parseInt).toArray();
}

public enum ClientName {
JEDIS("Jedis"),
JEDIS_ASYNC("Jedis async"),
LETTUCE("Lettuce"),
LETTUCE_ASYNC("Lettuce async"),
BABUSHKA_ASYNC("Babushka async"),
ALL("All"),
ALL_SYNC("All sync"),
ALL_ASYNC("All async");
JEDIS("Jedis"), // async
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async -> sync?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d3f707f.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sync

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d3f707f.

LETTUCE("Lettuce"), // async
BABUSHKA("Babushka"), // async
ALL("All");

private String name;

Expand All @@ -231,30 +201,31 @@ public boolean isEqual(String other) {

public static class RunConfiguration {
public String configuration;
public Optional<FileWriter> resultsFile;
public Optional<String> resultsFile;
public int[] dataSize;
public int[] concurrentTasks;
public ClientName[] clients;
public String host;
public int port;
public int[] clientCount;
public boolean tls;
public boolean clusterModeEnabled;
public boolean debugLogging = false;

public RunConfiguration() {
configuration = "Release";
resultsFile = Optional.empty();
dataSize = new int[] {100, 4000};
concurrentTasks = new int[] {100, 1000};
concurrentTasks = new int[] {1, 10, 100, 1000};
clients =
new ClientName[] {
// ClientName.BABUSHKA_ASYNC,
ClientName.JEDIS, ClientName.JEDIS_ASYNC, ClientName.LETTUCE, ClientName.LETTUCE_ASYNC
ClientName.ALL,
};
host = "localhost";
port = 6379;
clientCount = new int[] {1};
tls = false;
clusterModeEnabled = false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package javababushka.benchmarks.clients;

import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

/** A Redis client with async capabilities */
public interface AsyncClient<T> extends Client {

long DEFAULT_TIMEOUT_MILLISECOND = 1000;

Future<T> asyncSet(String key, String value);

Future<String> asyncGet(String key);

default <T> T waitForResult(Future<T> future) {
return waitForResult(future, DEFAULT_TIMEOUT_MILLISECOND);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to wrap the results with timeouts

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a protection against infinite hangs. If something happen, a CI run won't finish and will block running tests for other clients.
Please confirm that I can remove it.

}

default <T> T waitForResult(Future<T> future, long timeout) {
try {
return future.get(timeout, TimeUnit.MILLISECONDS);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

} catch (TimeoutException ignored) {
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't want to ignore errors

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d3f707f.

} catch (ExecutionException e) {
throw new RuntimeException("Client error", e);
} catch (InterruptedException e) {
if (Thread.currentThread().isInterrupted()) {
// restore interrupt
Thread.interrupted();
}
throw new RuntimeException("The thread was interrupted", e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package javababushka.benchmarks.clients;

import javababushka.benchmarks.utils.ConnectionSettings;

/** A Redis client interface */
public interface Client {
void connectToRedis(ConnectionSettings connectionSettings);
shachlanAmazon marked this conversation as resolved.
Show resolved Hide resolved

default void closeConnection() {}

String getName();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package javababushka.benchmarks.clients;

/** A Redis client with sync capabilities */
public interface SyncClient extends Client {
void set(String key, String value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the previous PR, this isn't how to get the best performance from a synchronous client. You'd batch commands in order to get the maximum throughput.

#540 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a common APi for the testing clients. We can batch commands under the hood. Otherwize we need to refactor the benchmarking to do different approaches for sync and async clients. Should we?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you use batching under the hood with this API?
Sending 1000 get commands per one get method call will skew the statistics.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 3138a7a.


String get(String key);
}
Loading