-
Couldn't load subscription status.
- Fork 40
Support batch operation in Transaction API #3082
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
Conversation
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.
Pull Request Overview
This PR adds support for batch operations in the Transaction API, enabling multiple operations to be executed together in a single batch call. The implementation includes support for both read operations (Get, Scan) and mutation operations (Put, Insert, Upsert, Update, Delete) across various transaction implementations.
Key Changes:
- Introduced a new
batch()method in theCrudOperableinterface that accepts a list of operations and returns batch results - Implemented batch operation support across all transaction types (distributed, two-phase commit, JDBC, and single CRUD)
- Added
BatchResultinterface andBatchResultImplclass to encapsulate operation results - Removed unnecessary consistency settings from test utility methods
Reviewed Changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/com/scalar/db/api/CrudOperable.java | Added batch() method signature and BatchResult interface to the core API |
| core/src/main/java/com/scalar/db/common/BatchResultImpl.java | New implementation class for batch operation results |
| core/src/main/java/com/scalar/db/common/AbstractDistributedTransaction.java | Implemented batch() and mutate() methods with operation dispatching logic |
| core/src/main/java/com/scalar/db/common/AbstractTwoPhaseCommitTransaction.java | Implemented batch() and mutate() methods for two-phase commit transactions |
| core/src/main/java/com/scalar/db/common/ReadOnlyDistributedTransaction.java | Added validation to prevent mutations in read-only batch operations |
| core/src/main/java/com/scalar/db/util/ScalarDbUtils.java | Updated utility to handle Operation type instead of just Mutation |
| core/src/main/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionManager.java | Implemented batch() with single operation restriction |
| integration-test/src/main/java/com/scalar/db/api/TwoPhaseCommitTransactionIntegrationTestBase.java | Added comprehensive integration tests for batch operations and removed unnecessary consistency settings |
| integration-test/src/main/java/com/scalar/db/api/DistributedTransactionIntegrationTestBase.java | Added integration tests for batch operations in distributed transactions |
| core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitTest.java | Added unit tests for batch operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Summary of ChangesHello @brfrn169, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Transaction API by introducing a Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This PR adds support for batch operations in the Transaction API. The changes are extensive, touching many interfaces and their implementations to add the batch method. The core logic is centralized in abstract classes like AbstractDistributedTransaction, which is a good design choice. The integration tests are also updated to cover the new functionality. My review includes a few suggestions to improve the Javadoc clarity and the immutability of the new BatchResultImpl class.
core/src/main/java/com/scalar/db/common/AbstractDistributedTransaction.java
Show resolved
Hide resolved
core/src/main/java/com/scalar/db/common/AbstractTwoPhaseCommitTransaction.java
Show resolved
Hide resolved
...rc/main/java/com/scalar/db/common/ActiveTransactionManagedDistributedTransactionManager.java
Show resolved
Hide resolved
core/src/main/java/com/scalar/db/common/ReadOnlyDistributedTransaction.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java
Outdated
Show resolved
Hide resolved
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.
LGTM! 👍
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.
LGTM! Thank you!
Left one question for clarification. PTAL!
...rc/main/java/com/scalar/db/common/ActiveTransactionManagedDistributedTransactionManager.java
Show resolved
Hide resolved
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.
LGTM, thank you!
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.
LGTM! Thank you!
Description
This PR adds support for batch operations that perform multiple operations in the Transaction API.
Related issues and/or PRs
N/A
Changes made
BatchResultinterface andBatchResultImplclass to encapsulate operation resultsChecklist
Additional notes (optional)
N/A
Release notes
Added support for batch operations that perform multiple operations in the Transaction API.