Support begin in read-only mode for Consensus Commit transactions

[brfrn169]

Description

This PR adds support for beginning a transaction in read-only mode for Consensus Commit transactions.

Note that this feature is being developed in the support-begin-in-read-only-mode feature branch.

Related issues and/or PRs

• Add beginReadOnly() method to transaction abstraction #2734

Changes made

Added some inline comments. Please take a look for the details.

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

Adds support for starting Consensus Commit transactions in read-only mode by introducing a readOnly flag throughout the transaction stack, updating handlers, snapshots, managers, and tests accordingly.

• Propagate a readOnly boolean to CrudHandler and suppress write-set and overlap verification when true
• Introduce beginReadOnly / startReadOnly methods in ConsensusCommitManager and wrap read-only transactions in ReadOnlyDistributedTransaction
• Update tests to cover read-only behavior and adjust constructors to accept the new flag

Reviewed Changes

Copilot reviewed 8 out of 8 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/ConsensusCommitIntegrationTestBase.java	Removed placeholder disabled tests for read-only mode
core/src/test/java/com/scalar/db/transaction/consensuscommit/CrudHandlerTest.java	Added unit tests for read-only get and scan paths
core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManagerTest.java	Added tests for beginReadOnly and startReadOnly, updated spies to expect read-only flag
core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java	Pass false for readOnly when creating two-phase transactions
core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java	Reordered and documented the read/get/scan/write/delete sets
core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java	Added readOnly field, suppressed read-set writes and overlap checks when in read-only mode
core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java	Added beginReadOnly / startReadOnly, extended executeTransaction to handle read-only paths

Comments suppressed due to low confidence (4)

integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitIntegrationTestBase.java:929

• The removed placeholder tests for read-only mode indicate missing integration coverage. Consider adding end-to-end tests that verify get and scan behavior under a read-only Consensus Commit transaction against actual storage to ensure the feature is fully validated.

@Disabled("Implement later")

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java:151

• [nitpick] Public API methods beginReadOnly and startReadOnly lack Javadoc. Adding Javadoc comments explaining that these methods start a snapshot-isolation read-only transaction would improve discoverability and maintain consistency with existing methods.

public DistributedTransaction beginReadOnly() {

core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java:59

• [nitpick] The boolean readOnly flag in this constructor can be ambiguous at call sites (it's not obvious what true or false means). Consider replacing it with an enum or builder pattern (e.g., .readOnly() vs .readWrite()) to make instantiation intent clearer.

public CrudHandler(
            storage,
            snapshot,
            tableMetadataManager,
            isIncludeMetadataEnabled,
            parallelExecutor,
            boolean readOnly) {

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java:431

• [nitpick] The executeTransaction method uses a boolean readOnly flag to choose between begin and beginReadOnly. You might simplify the API by splitting into two overloads (one for read/write, one for read-only) or passing an enum instead of a raw boolean to reduce the risk of passing the wrong mode.

private <R> R executeTransaction(
      ThrowableFunction<DistributedTransaction, R, TransactionException> throwableFunction,

[brfrn169]

Wrap the transaction object with ReadOnlyDistributedTransaction for read-only transactions.

[brfrn169]

As an optimization, in read-only mode, we don't need to add the result to the read set, since the read set is only used for write operations.

[brfrn169]

Also, we don't need to verify the overlap in read-only mode, since there are no write operations.

[komamitsu]

LGTM! 👍

[komamitsu]

👍

[feeblefakie]

LGTM! Thank you!

[Torch3333]

LGTM, thank you!