Fix behavior of group commit with one-phase commit

[brfrn169]

Description

When I implemented the one-phase commit optimization, I somehow thought that we couldn’t support enabling both group commit and one-phase commit. However, on second thought, I realized that we actually can. This PR addresses that.

Related issues and/or PRs

N/A

Changes made

• Updated the logic to cancel group commit in the following situations:
  • When canOnePhaseCommit() throws an Exception
  • When a one-phase commit is performed

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

Fixed a bug where group commit did not work correctly with one-phase commit.

[gemini-code-assist]

Summary of Changes

Hello @brfrn169, I'm Gemini Code Assist¹! 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 resolves a perceived incompatibility between the group commit and one-phase commit optimizations, allowing them to function together. The core change involves updating the commit handler to ensure proper state management and cleanup within the group commit mechanism when one-phase commit is utilized, thereby enhancing the robustness and efficiency of the transaction system.

Highlights

• Enabling Concurrent Optimizations: This PR enables the simultaneous use of both 'group commit' and 'one-phase commit' optimizations, which were previously considered mutually exclusive. This allows for potentially improved transaction performance and flexibility.
• Group Commit Integration for One-Phase Commit: The CommitHandlerWithGroupCommit class has been updated to correctly integrate with one-phase commit. Specifically, it now ensures that a transaction is properly removed from the group commit queue when a one-phase commit attempt is made, even if it fails.
• Comprehensive Test Suite Update: Numerous previously disabled tests related to one-phase commit within a group commit context have been re-enabled. New assertions have been added to these tests to verify the correct interaction and cleanup behavior of the group commit mechanism during one-phase commit operations.

Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and 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 to provide feedback.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[Copilot]

Pull Request Overview

This PR enables support for using both group commit and one-phase commit together.

• Removed disabled tests and implemented one-phase commit scenarios in the group‐commit test suite
• Introduced a createCommitHandlerWithOnePhaseCommit helper in base tests and aligned test names
• Added cancellation logic for group commits in the CommitHandlerWithGroupCommit class

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommitTest.java	Re-enabled and implemented one-phase commit tests; removed @Disabled annotations and added overrides
core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerTest.java	Added createCommitHandlerWithOnePhaseCommit in base tests and updated test method naming
core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommit.java	Overrode canOnePhaseCommit and onePhaseCommitRecords to cancel group commit on errors

Comments suppressed due to low confidence (2)

core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommitTest.java:229

• This method is missing the @test annotation, so JUnit will not execute it. Please add @test above the method.

      onePhaseCommitRecords_WhenNoMutationExceptionThrown_ShouldThrowCommitConflictException()

core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommitTest.java:240

• This method is missing the @test annotation, so it won't be run by the test runner. Please add @test above it.

      onePhaseCommitRecords_WhenRetriableExecutionExceptionThrown_ShouldThrowCommitConflictException()

[gemini-code-assist]

Code Review

This pull request enables one-phase commit optimization alongside group commit. The changes in CommitHandlerWithGroupCommit ensure that group commit reservations are properly cancelled when a transaction proceeds with a one-phase commit, and the test suite verifies this new behavior.

[komamitsu]

LGTM! 👍

I can't remember why I didn't notice the previous PR treated the combination of the coordinator group commit feature and one-phase commit prohibited in the review...

[brfrn169]

I can't remember why I didn't notice the previous PR treated the combination of the coordinator group commit feature and one-phase commit prohibited in the review...

@komamitsu Maybe the idea that we couldn’t support one-phase commit with SSR was stuck in our heads, and we ended up mixing up SSR and group commit. 😅

[komamitsu]

we ended up mixing up SSR and group commit

Yeah, I also guess so.

[Torch3333]

LGTM, thank you!

[feeblefakie]

LGTM! Thank you!