Allow multiple calls to rollback()

[brfrn169]

Description

This PR updates the code to allow multiple calls to the rollback() method of transaction objects, making it more convenient for application developers.

Currently, if users call the rollback() method multiple times, an IllegalArgumentException is thrown, as shown below:

DistributedTransaction transaction = transactionManager.begin();

...

transaction.rollback();

transaction.rollback(); // Throw IllegalArgumentException

However, in certain situations—especially in modular or layered code—this behavior can be inconvenient for developers.

This PR addresses that issue by making repeated calls to rollback() safe.

Related issues and/or PRs

N/A

Changes made

• Updated StateManagedDistributedTransactionManager and StateManagedTwoPhaseCommitTransactionManager to allow multiple calls to the rollback() method.

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

[Copilot]

Pull Request Overview

This PR updates the transaction managers to allow multiple calls to rollback() (and abort()) without throwing exceptions if the transaction is already rolled back. Key changes include:

• Updating the tests in both StateManagedTwoPhaseCommitTransactionManagerTest and StateManagedDistributedTransactionManagerTest to verify that repeated calls to rollback() and abort() behave as expected.
• Adjusting the logic in rollback() and abort() in both transaction manager classes so that calling these methods on a transaction that is already rolled back returns silently, and only throws an IllegalStateException if the transaction has been committed.
• Changing the error message in CoreError to reflect that only a committed transaction now raises an exception on subsequent rollback/abort calls.

Reviewed Changes

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

Show a summary per file

File	Description
core/src/test/java/com/scalar/db/common/StateManagedTwoPhaseCommitTransactionManagerTest.java	Updated test names and assertions to verify non-exception behavior on repeated rollback/abort calls
core/src/test/java/com/scalar/db/common/StateManagedDistributedTransactionManagerTest.java	Mirrored the test updates for distributed transactions
core/src/main/java/com/scalar/db/common/error/CoreError.java	Changed error message constant to remove mention of rollback for already finalized transactions
core/src/main/java/com/scalar/db/common/StateManagedTwoPhaseCommitTransactionManager.java	Modified rollback() and abort() logic to allow multiple calls with appropriate exception handling
core/src/main/java/com/scalar/db/common/StateManagedDistributedTransactionManager.java	Similar changes as above to support multiple rollback/abort calls

Comments suppressed due to low confidence (3)

core/src/main/java/com/scalar/db/common/StateManagedTwoPhaseCommitTransactionManager.java:184

• Consider adding a clarifying comment here to document that multiple calls to rollback() are now allowed and that subsequent calls return silently when the status is ROLLED_BACK.

if (status == Status.ROLLED_BACK) {

core/src/main/java/com/scalar/db/common/StateManagedDistributedTransactionManager.java:147

• Consider adding a comment to indicate that if the transaction is already rolled back, further calls to rollback() will return silently, which supports the new behavior.

if (status == Status.ROLLED_BACK) {

core/src/main/java/com/scalar/db/common/error/CoreError.java:217

• Ensure that the updated error message 'TRANSACTION_ALREADY_COMMITTED' and its usage properly communicate that the exception is raised only when the transaction has been committed, not when it has been rolled back.

TRANSACTION_ALREADY_COMMITTED(

[feeblefakie]

LGTM! Thank you!

[komamitsu]

status seems to move to ROLLED_BACK even when rolling back the transaction fails. If users first receive an exception from rollback() and then try the second attempt, it will look like success while the transaction might not be rolled back. What do you think?

[brfrn169]

IMO, that’s okay. Even if rolling back a transaction fails, the transaction manager is expected to automatically and eventually roll back the transaction. So, I don’t think applications need to retry the rollback themselves. What do you think?

[komamitsu]

I see. Sounds good 👍

[Torch3333]

LGTM, thank you!

[komamitsu]

LGTM! 👍