-
Notifications
You must be signed in to change notification settings - Fork 70
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
Changes from 7 commits
5b31ff9
842a222
58f3684
28e5fe0
3138a7a
859c9e6
f7dbd3d
c83d4f3
199ed5a
d3f707f
26e786d
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 |
---|---|---|
@@ -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; | ||
|
@@ -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"); | ||
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() { | ||
|
@@ -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|" | ||
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. Should remove all irrelevant clients 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. Fixed in d3f707f. 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. just all |jedis|lettuce|babushka 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. Nice catch, thank you. |
||
+ "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; | ||
} | ||
|
@@ -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")) { | ||
|
@@ -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); | ||
} | ||
|
@@ -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; | ||
} | ||
|
@@ -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 | ||
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. async -> sync? 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. Fixed in d3f707f. 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. sync 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. Fixed in d3f707f. |
||
LETTUCE("Lettuce"), // async | ||
BABUSHKA("Babushka"), // async | ||
ALL("All"); | ||
|
||
private String name; | ||
|
||
|
@@ -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); | ||
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. We don't need to wrap the results with timeouts 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. It is a protection against infinite hangs. If something happen, a CI run won't finish and will block running tests for other clients. |
||
} | ||
|
||
default <T> T waitForResult(Future<T> future, long timeout) { | ||
try { | ||
return future.get(timeout, TimeUnit.MILLISECONDS); | ||
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. same |
||
} catch (TimeoutException ignored) { | ||
return null; | ||
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. we don't want to ignore errors 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. 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); | ||
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. 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. 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. 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? 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. How would you use batching under the hood with this API? 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. Fixed in 3138a7a. |
||
|
||
String get(String key); | ||
} |
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.
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.
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.
Sorry, it was uncleaned reference to that client. It is already deleted.
Fixed in d3f707f.