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 @@ -46,31 +44,15 @@ public static void main(String[] args) {
// 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");
break;
case LETTUCE:
// run testClientSetGet on LETTUCE sync client
System.out.println("Run LETTUCE sync client");
Copy link
Contributor

Choose a reason for hiding this comment

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

it's an async client

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh I deleted the wrong reference. Restored.

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"), // sync
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,26 @@
package javababushka.benchmarks.clients;

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

/** 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 (Exception ignored) {
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 silently ignore exceptions? how would you know if there are issues with the benchmark?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an exception from a client, e.g. get operation failed. Should we care why?
Logging an exception reduces performance.

Copy link
Contributor

Choose a reason for hiding this comment

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

we should care that there were exceptions, because if Babushka suddenly throws exceptions under load, this is the first place we'll notice this.
This has happened in other languages.

If you want to avoid timeout logs, check that the exception isn't a timeout exception.

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

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.

}
}
}
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