Skip to content

Multi exec on cluster #3611

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

Merged

Conversation

robertosantamaria-scopely
Copy link
Contributor

@robertosantamaria-scopely robertosantamaria-scopely commented Apr 21, 2025

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • 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?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

This adds support for transactions in cluster clients. These transactions are based on multi/watch/exec, same as for standalone client, but are limited to a single hash slot.

@robertosantamaria-scopely robertosantamaria-scopely force-pushed the multi-exec-on-cluster branch 2 times, most recently from dee69d2 to 9937904 Compare April 22, 2025 08:06
@robertosantamaria-scopely
Copy link
Contributor Author

@elena-kolevska this needs workflow approval, thanks

Adds support for transactions based on multi/watch/exec on clusters.
Transactions in this mode are limited to a single hash slot.

Contributed-by: Scopely <[email protected]>
@petyaslavova petyaslavova added the feature New feature label Apr 29, 2025
@vladvildanov vladvildanov marked this pull request as ready for review May 6, 2025 09:40
@petyaslavova petyaslavova requested a review from Copilot May 8, 2025 15:08
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for transactions in cluster clients using multi/watch/exec commands (with the same slot limitation as standalone clients) and updates tests, documentation, and exception handling accordingly. Key changes include new tests for cluster transaction behaviors, additions to exception classes to better indicate transaction errors, and updated documentation reflecting the new transaction support in clusters.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_cluster_transaction.py Adds tests to validate transaction support in cluster mode.
tests/test_cluster.py Updates tests for pipeline behavior and multi delete operations.
redis/exceptions.py Introduces new exceptions (CrossSlotTransactionError, InvalidPipelineStack).
redis/client.py Updates Pipeline constructor with type annotations and minor refactoring.
docs/advanced_features.rst Updates documentation to describe transaction handling in clusters.
CHANGES Records support for transactions in ClusterPipeline.
.github/wordlist.txt Adds CAS to the wordlist.

@vladvildanov
Copy link
Collaborator

@robertosantamaria-scopely Hey! I would appreciate your review, since the PR was refactored

Copy link
Collaborator

@petyaslavova petyaslavova left a comment

Choose a reason for hiding this comment

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

LGTM.

@robertosantamaria-scopely
Copy link
Contributor Author

@robertosantamaria-scopely Hey! I would appreciate your review, since the PR was refactored

Thanks @vladvildanov ; I can't give a ✅ as the PR is on my name, but you have my 👍 for sure. Cheers!

@vladvildanov
Copy link
Collaborator

@robertosantamaria-scopely Thanks!

@vladvildanov vladvildanov merged commit 91be4a0 into redis:master May 12, 2025
69 of 70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants