Skip to content

Conversation

@brfrn169
Copy link
Collaborator

Description

This PR renames some methods of CommitHandler to align with the Consensus Commit documentation.

Related issues and/or PRs

N/A

Changes made

  • Renamed some methods of CommitHandler.

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes.
  • I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
  • Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
  • Tests (unit, integration, etc.) have been added for the changes.
  • My changes generate no new warnings.
  • Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

N/A

Release notes

N/A

Copy link
Contributor

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 renames methods in the CommitHandler and related classes to align with the latest Consensus Commit documentation. The key changes include renaming from "prepare" to "prepareRecords" and "validate" to "validateRecords" across both production and test code, ensuring consistent method naming throughout the codebase.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java Updated method calls to use the new prepareRecords method.
core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitTest.java Renamed test methods and internal calls to match the new method names.
core/src/test/java/com/scalar/db/transaction/consensuscommit/ParallelExecutorTest.java Updated test method names and calls to parallelExecutor to use prepareRecords and validateRecords.
core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommit.java Modified implementation to call prepareRecords and validateRecords.
core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java Changed validate call to validateRecords.
core/src/main/java/com/scalar/db/transaction/consensuscommit/ParallelExecutor.java Updated method names for preparation and validation tasks.
core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java Changed commit flow to use the new prepareRecords and validateRecords methods and removed redundant code.

Copy link
Contributor

@Torch3333 Torch3333 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@komamitsu komamitsu left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@brfrn169
Copy link
Collaborator Author

@Torch3333 I'll merge this. Please take a look when you have time!

@brfrn169 brfrn169 merged commit 6ccaeec into master Jun 18, 2025
56 checks passed
@brfrn169 brfrn169 deleted the rename-methods-of-CommitHandler branch June 18, 2025 11:33
feeblefakie pushed a commit that referenced this pull request Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants