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

Batch and pipeline multiple Redis calls together #4508

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Woellchen
Copy link
Contributor

What problem are we solving?

When using Redis store there are a lot of single commands sent to Redis sequentially. This causes many round trips and network IO wait.
For example for directories this produces a GET command for each child in the directory. Imagine a folder with a million files, it would sequentially execute 1 million GET commands.

How are we solving the problem?

Using the client-side pipelining feature to send all commands and get all their results in a single roundtrip.
This is implemented for redis2 stores. If you like the idea I can go ahead and change the other redis stores as well.

How is the PR tested?

Checks

  • I have added unit tests if possible.
  • I will add related wiki document changes and link to this PR after merging.

@Woellchen Woellchen marked this pull request as draft May 26, 2023 18:16
@agipson33
Copy link
Contributor

As a Redis v7 user with SeaweedFS configured to use redis_cluster3, I would love to see these improvements and have many directories with thousands of files to test performance improvements against.


// Clean expired keys
var expiredEntries []*filer.Entry
for _, entry := range entries {
Copy link
Collaborator

@chrislusf chrislusf May 31, 2023

Choose a reason for hiding this comment

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

what if the entries in the first batch are all expired?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants