Add sync API for transactional client

[odecode]

For issue #289 a synchronous API for transactional client.

I have programmed a fair bit but new to OSS and Rust. I consulted AI heavily for this to try and get an understanding of what this part of the project is about and how it works, but I tried using it a teacher rather than just "vibe coding".

Summary by CodeRabbit

• New Features
  • Exposed blocking synchronous transaction and snapshot APIs that provide full, blocking access to transactional and read-only operations alongside existing async interfaces.
• Bug Fixes
  • Clear user-facing error when attempting to use the synchronous client from within an active async runtime.
• Tests
  • Extensive integration tests covering sync client usage, transactions, snapshots, scans, edge cases, and context validation.

[ti-chi-bot]

Welcome @odecode!

It looks like this is your first PR to tikv/client-rust 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to tikv/client-rust. 😃

[Copilot]

Pull request overview

This PR adds a synchronous API wrapper for the TiKV transactional client to address issue #289. The implementation provides blocking alternatives to the existing async transaction APIs.

Changes:

• Introduced SyncTransactionClient, SyncTransaction, and SyncSnapshot structs that wrap their async counterparts
• Each sync type contains an Arc-wrapped Tokio runtime and uses block_on to execute async operations synchronously
• Added comprehensive integration tests mirroring the async test patterns

Reviewed changes

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

Show a summary per file

File	Description
src/transaction/sync_client.rs	New synchronous client wrapper providing blocking transaction operations
src/transaction/sync_transaction.rs	Synchronous transaction wrapper that blocks on async transaction methods
src/transaction/sync_snapshot.rs	Synchronous snapshot wrapper for read-only operations
src/transaction/mod.rs	Module declarations for new sync components
src/lib.rs	Public API exports for sync types
tests/sync_transaction_tests.rs	Comprehensive integration tests covering all sync APIs
tests/common/mod.rs	Added init_sync() helper for synchronous test setup

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[odecode]

Thank you for the review comments, I have made changes according to the comments

[Copilot]

Pull request overview

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

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[odecode]

I have made changes requested by the review comments

[Copilot]

Pull request overview

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

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[odecode]

Is it possible for me to request review from Copilot myself? Then I could address all its concerns and not bother you on every iteration.

EDIT: I figured out that I can ask Copilot to review uncommitted changes in my code editor, I will do that for the next round of changes if Copilot or you request more improvements

[Copilot]

Pull request overview

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

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a synchronous transaction API: new types SyncTransactionClient, SyncTransaction, SyncSnapshot, a NestedRuntimeError variant, a synchronous test initializer, and extensive integration tests. The sync types wrap existing async components and block on their futures via an internal Tokio runtime.

Changes

Cohort / File(s)	Summary
Error Handling src/common/errors.rs	Added NestedRuntimeError(String) to the public Error enum with a user-facing message about not using SyncTransactionClient inside a Tokio async runtime.
Crate Re-exports & Module Decl src/lib.rs, src/transaction/mod.rs	Publicly re-exported SyncSnapshot, SyncTransaction, and SyncTransactionClient; added mod declarations for sync implementations.
Sync Client Implementation src/transaction/sync_client.rs	New SyncTransactionClient that embeds an async Client and an Arc Tokio runtime; implements new, new_with_config, nested-runtime detection (check_nested_runtime), safe_block_on, synchronous wrappers (begin_*, snapshot, current_timestamp, gc, cleanup_locks, unsafe_destroy_range, cfg-gated scan_locks), Clone, and nested-runtime tests.
Sync Snapshot Implementation src/transaction/sync_snapshot.rs	New SyncSnapshot wrapping async Snapshot with blocking accessors (get, key_exists, batch_get, scan, scan_keys, scan_reverse, scan_keys_reverse) that call into the runtime.
Sync Transaction Implementation src/transaction/sync_transaction.rs	New SyncTransaction wrapping async Transaction; provides blocking transactional API (get, get_for_update, key_exists, batch ops, scans, put/insert/delete, batch_mutate, lock_keys, commit, rollback, send_heart_beat) delegating via safe_block_on.
Test Infrastructure tests/common/mod.rs	Added init_sync() helper to initialize test infra synchronously with a dedicated single-thread Tokio runtime (panics if called from an active async context).
Integration Test Suite tests/sync_transaction_tests.rs	Added comprehensive integration tests for sync client covering nested-runtime checks, timestamp retrieval, optimistic/pessimistic flows, CRUD, batch operations, snapshots, scans, locking, heartbeat, cloning, and edge cases.

Sequence Diagram(s)

sequenceDiagram
    participant U as User Code
    participant SC as SyncTransactionClient
    participant RTcheck as check_nested_runtime()
    participant SB as safe_block_on()
    participant TRT as Tokio Runtime
    participant AC as Async TransactionClient

    U->>SC: call sync method (e.g., begin_optimistic)
    SC->>RTcheck: detect active Tokio runtime?
    alt nested runtime detected
        RTcheck-->>SC: Err(NestedRuntimeError)
        SC-->>U: return Err(NestedRuntimeError)
    else no nested runtime
        RTcheck-->>SC: Ok(())
        SC->>SB: create future for async operation
        SB->>TRT: block_on(future)
        TRT->>AC: execute async operation
        AC-->>TRT: result
        TRT-->>SB: return result
        SB-->>SC: Result<T>
        SC-->>U: return Result<T>
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hop between async and sync today,
I check runtimes lest I stray,
I block with care and wrap each call,
So transactions stand both proud and tall,
A tiny rabbit cheers this play!

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add sync API for transactional client' directly and clearly summarizes the main change: introducing a synchronous API wrapper around the existing async transactional client.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into master

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

src/common/errors.rs (1)

131-136: Minor: trailing {0} in error message lacks separator and may be unnecessary.

The {0} is appended directly after the period with no space/newline. Currently NestedRuntimeError is always constructed with String::new(), making {0} a no-op. If this is intended for future contextual detail, add a space or conditional separator; otherwise, consider removing it or switching to a unit-like variant without the String payload.

Option A: Remove the String payload if not needed

-    /// Attempted to create or use the sync client (including calling its methods) from within a Tokio async runtime context
-    #[error(
-        "Nested Tokio runtime detected: cannot use SyncTransactionClient from within an async context. \
-Use the async TransactionClient instead, or create and use SyncTransactionClient outside of any Tokio runtime.{0}"
-    )]
-    NestedRuntimeError(String),
+    /// Attempted to create or use the sync client (including calling its methods) from within a Tokio async runtime context
+    #[error(
+        "Nested Tokio runtime detected: cannot use SyncTransactionClient from within an async context. \
+Use the async TransactionClient instead, or create and use SyncTransactionClient outside of any Tokio runtime."
+    )]
+    NestedRuntimeError,

Option B: Add a space before `{0}` if context detail is planned

-Use the async TransactionClient instead, or create and use SyncTransactionClient outside of any Tokio runtime.{0}"
+Use the async TransactionClient instead, or create and use SyncTransactionClient outside of any Tokio runtime. {0}"

src/transaction/sync_client.rs (1)

81-91: Consider documenting or exposing runtime configuration.

new_multi_thread() with default settings creates a runtime with one thread per CPU core. This is potentially heavyweight if multiple SyncTransactionClient instances are created (each gets its own runtime).

A few options to consider for the future:

• Allow passing in an existing Arc<Runtime> for sharing across clients.
• Use worker_threads(2) or similar to limit thread count since this is a sync wrapper and the runtime primarily drives gRPC I/O.

Not blocking — the current approach works correctly and the Clone impl shares the runtime via Arc, so cloned clients are lightweight.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[odecode]

added extra test coverage for the sync snapshot public api surface, and error handling to prevent panic when sync client is called from within an async runtime.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/transaction/sync_snapshot.rs`:
- Around line 18-70: SyncSnapshot methods call self.runtime.block_on(...)
directly which will panic if invoked inside an active Tokio runtime; add a
private helper on SyncSnapshot (e.g., safe_block_on_snapshot) that mirrors
sync_client::safe_block_on by checking
tokio::runtime::Handle::try_current().is_ok() and returning
Err(Error::NestedRuntimeError) when a current runtime exists, otherwise invoking
self.runtime.block_on; replace direct self.runtime.block_on(...) calls in the
public methods get, key_exists, batch_get, scan, scan_keys, scan_reverse, and
scan_keys_reverse to use this helper so nested runtime usage is guarded.

In `@src/transaction/sync_transaction.rs`:
- Around line 20-126: All SyncTransaction and SyncSnapshot methods currently
call self.runtime.block_on(...) and can panic if invoked inside an existing
Tokio runtime; make the existing check_nested_runtime/safe_block_on helpers
accessible (move to a shared module or change to pub(crate)), then replace every
direct self.runtime.block_on(self.inner.<method>(...)) call in SyncTransaction
and SyncSnapshot (e.g., get, get_for_update, key_exists, batch_get,
batch_get_for_update, scan, scan_keys, scan_reverse, scan_keys_reverse, put,
insert, delete, batch_mutate, lock_keys, commit, rollback, send_heart_beat) with
the safe helper usage (run the nested-runtime check and call safe_block_on
around the inner async call) so callers get a NestedRuntimeError instead of a
panic.

[pingyu]

Rest LGTM.

[pingyu]

LGTM~

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-02-15 14:35:34.298373352 +0000 UTC m=+109187.313066812: ☑️ agreed by pingyu.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pingyu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [pingyu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[pingyu]

cc @iosmanthus

[pingyu]

The build failure will be fixed by #532 .