Skip to content

Conversation

@utkarshgupta137
Copy link
Contributor

@utkarshgupta137 utkarshgupta137 commented Jun 14, 2022

Pull Request check-list

  • Does $ tox pass with this change (including linting)?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?
  • Was the change added to CHANGES file?

Description of change

Since commands are split by slots, using a pipeline to send keys from the same node together provides a massive speedup, especially for the sync version.

Benchmarks using a batch of MGET 100 keys, MSET 100 keys, DEL 100 keys:

  • Sync (10 batches): 6.34s -> 0.26s
  • Async (100 batches): 9.34s -> 3.15s

Other changes:

  • allow passing target_nodes to pipeline commands
  • pass target_nodes for split commands
  • move READ_COMMANDS to commands/cluster to avoid import cycle
  • add types to list_or_args
- allow passing target_nodes to pipeline commands - move READ_COMMANDS to commands/cluster to avoid import cycle - add types to list_or_args
@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2022

Codecov Report

Merging #2230 (41016ff) into master (bedf3c8) will decrease coverage by 0.00%.
The diff coverage is 94.11%.

@@ Coverage Diff @@ ## master #2230 +/- ## ========================================== - Coverage 91.83% 91.82% -0.01%  ========================================== Files 108 108 Lines 27690 27685 -5 ========================================== - Hits 25429 25423 -6  - Misses 2261 2262 +1 
Impacted Files Coverage Δ
redis/cluster.py 90.08% <83.33%> (+0.08%) ⬆️
redis/asyncio/cluster.py 89.96% <85.71%> (+0.05%) ⬆️
redis/commands/__init__.py 100.00% <100.00%> (ø)
redis/commands/cluster.py 93.64% <100.00%> (-0.57%) ⬇️
redis/commands/helpers.py 87.36% <100.00%> (+0.27%) ⬆️
tests/test_cluster.py 96.79% <0.00%> (-0.12%) ⬇️
redis/asyncio/connection.py 83.85% <0.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bedf3c8...41016ff. Read the comment docs.

@dvora-h
Copy link
Collaborator

dvora-h commented Jun 27, 2022

@utkarshgupta137 Thanks! These things really improve the project.

@dvora-h dvora-h merged commit d7d4336 into redis:master Jun 27, 2022
@utkarshgupta137 utkarshgupta137 deleted the cluster_non_atomic_pipeline branch December 1, 2022 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants