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

Reactive Cluster MGET is not running in parallel #2395

Closed
koisyu opened this issue Apr 21, 2023 · 5 comments
Closed

Reactive Cluster MGET is not running in parallel #2395

koisyu opened this issue Apr 21, 2023 · 5 comments
Labels
type: bug A general bug

Comments

@koisyu
Copy link
Contributor

koisyu commented Apr 21, 2023

Bug Report

RedisAdvancedClusterReactiveCommandsImpl.mget is not running in parallel

Current Behavior

If you run the code below, it will take a long time.

RedisClusterClient client = RedisClusterClient.create("redis://localhost");
RedisAdvancedClusterReactiveCommands<String, String> reactiveCommands = client.connect().reactive();
List<String> keys = IntStream.rangeClosed(1, 100).boxed().map(index -> "key-" + index).collect(Collectors.toList());
reactiveCommands.mget(keys.toArray(new String[]{})).collectList().block();

If I run the above code and check the Tcpdump, I can see that the requests and responses are alternating in order, like this.
That is, they are executed sequentially.

image

Expected behavior/code

I expect MGET commands to be executed in parallel.

image

Environment

  • Lettuce version(s): 6.1.9.RELEASE
  • Redis version: 4.0.14

Possible Solution

If the Flux#concat code is replaced with Flux#mergeSequential, the MGET commands are executed in parallel.

  • Flux#concat
    image
  • Flux#mergeSequential
    image

Additional context

@koisyu
Copy link
Contributor Author

koisyu commented Apr 21, 2023

I was worried that my explanation wasn't clear enough, so I added an example of the difference in repeated runs.
For example, if you run the code below, you can clearly see the difference.

RedisClusterClient client = RedisClusterClient.create("redis://localhost");
StatefulRedisClusterConnection<String, String> connect = client.connect();
RedisAdvancedClusterReactiveCommands<String, String> reactiveCommands = connect.reactive();
String[] keys = IntStream.rangeClosed(1, 500).boxed().map(index -> "key-" + index).toArray(String[]::new);
reactiveCommands.mget(keys).collectList().block();

long startTimeInMillis = System.currentTimeMillis();
for (int i = 0; i < 100; i++) {
    reactiveCommands.mget(keys).collectList().block();
}
long endTimeInMillis = System.currentTimeMillis();
System.out.println(String.format("Total time in millis : %s", (endTimeInMillis - startTimeInMillis)));

Here are the execution results of the code in both cases. The execution times vary slightly depending on the conditions, but the difference between the two is always significant.

  • Flux#concat
    • Total time in millis : 8289

image

  • Flux#mergeSequential
    • Total time in millis : 563

image

@mp911de mp911de added the type: bug A general bug label Apr 24, 2023
@mp911de mp911de changed the title RedisAdvancedClusterReactiveCommandsImpl.mget is not running in parallel Reactive Cluster MGET is not running in parallel Apr 24, 2023
mp911de pushed a commit that referenced this issue Apr 24, 2023
mp911de added a commit that referenced this issue Apr 24, 2023
Original pull request: #2396
mp911de pushed a commit that referenced this issue Apr 24, 2023
mp911de added a commit that referenced this issue Apr 24, 2023
Original pull request: #2396
@wenhaozhao
Copy link

@mp911de Can fix the same bug to version 6.1.x ?

@tishun tishun added the status: good-first-issue An issue that can only be worked on by brand new contributors label Jul 17, 2024
@tishun
Copy link
Collaborator

tishun commented Oct 24, 2024

@mp911de Can fix the same bug to version 6.1.x ?

Hey @wenhaozhao , is this still relevant or can we close this issue?

@tishun tishun added status: waiting-for-feedback We need additional information before we can continue and removed status: good-first-issue An issue that can only be worked on by brand new contributors labels Oct 24, 2024
@wenhaozhao
Copy link

@mp911de Can fix the same bug to version 6.1.x ?

Hey @wenhaozhao , is this still relevant or can we close this issue?

sure, please

tishun added a commit to tishun/lettuce-core that referenced this issue Oct 30, 2024
tishun added a commit that referenced this issue Oct 30, 2024
@tishun
Copy link
Collaborator

tishun commented Oct 30, 2024

sure, please

Lettuce 6.1.11.RELEASE should have the fix available

@tishun tishun closed this as completed Oct 30, 2024
@tishun tishun removed the status: waiting-for-feedback We need additional information before we can continue label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants