feat: add async C and C++ bindings for TiKV RawKV by lance6716 · Pull Request #571 · tikv/client-rust

lance6716 · 2026-10-14T09:00:21Z

Summary

Add an asynchronous C ABI for TiKV RawKV operations, including request callbacks, result ownership, cancellation, shutdown, and a configurable gRPC response decoding limit.
Add a header-only C++17 wrapper with RAII request and client types.
Add metrics snapshot APIs, examples, and binding documentation.

Review status

This is a draft for reviewing the API and implementation. Inline review comments identify concurrency and cancellation issues that should be resolved before it is marked ready.

Testing

The binding includes Rust unit tests and C/C++ examples. The examples require a running TiKV cluster; no tests were rerun while creating this PR.

ti-chi-bot · 2026-10-14T09:00:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign bufferflies for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

coderabbitai · 2026-10-14T09:00:32Z

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

lance6716

Pre-PR review: the first two inline comments identify request-lifetime races to fix before marking this draft ready. The remaining comments cover cancellation semantics, tests, structure, and documentation.

lance6716 · 2026-10-14T09:01:40Z

+        .cancel_inflight
+        .load(Ordering::Acquire)
+    {
+        let request = unsafe { &*request_ptr };


This dereference can race with a fast completion. The C contract allows the callback to destroy the request; after that callback, finalize_request can release the last references before this submit thread reaches the block (when cancel_inflight is true). Please set the cancellation state before spawning, or hold a separate submit-side reference until all post-spawn access is finished.

lance6716 · 2026-10-14T09:01:41Z

+                .lock()
+                .unwrap_or_else(|poisoned| poisoned.into_inner())
+                .clone();
+            for request_addr in requests {


Cloning requests copies bare addresses without retaining the request states. A request may finish, be removed, and be destroyed before the loop dereferences its copied address, causing a use-after-free. The same pattern occurs in tikv_async_client_destroy. Please keep each request alive while setting its cancellation flag (or avoid traversing these pointers).

lance6716 · 2026-10-14T09:01:41Z

+                || keepalive.lifecycle.cancel_inflight.load(Ordering::Acquire)
+        };
+
+        let (status, output, error_message) = match result {


Cancellation is checked only after the underlying future finishes, so a write may already have taken effect when the callback reports CANCELLED. Also, the Ok(Err(error)) arm below reports ERROR even if cancellation was requested, whereas the C header promises CANCELLED for pending requests. Please define the intended logical-cancellation contract, document possible side effects/partial batches, and cover both success and error after cancellation.

lance6716 · 2026-10-14T09:01:41Z

+    wait_drain(&handle.core);
+}
+
+#[cfg(test)]


The current unit tests cover parsing, batch grouping, metrics, and result ownership, but not an accepted request's concurrent lifecycle. Before this ABI is ready, please add deterministic tests for destruction inside the callback, shutdown racing with completion, exactly-once callback delivery, and cancellation outcomes. A C/C++ compile-and-link smoke check in CI would also protect the public headers.

lance6716 · 2026-10-14T09:01:41Z

@@ -0,0 +1,2475 @@
+// Copyright (c) 2026 PingCAP, Inc.


This file currently combines ABI types, request lifetime management, RawKV operations, metrics, and tests in about 2,475 lines. Splitting the lifetime state machine and metrics code into focused modules would make the unsafe ownership invariants easier to audit and test.

lance6716 · 2026-10-14T09:01:41Z

@@ -0,0 +1,390 @@
+# TiKV Rust Client C Binding


Could we provide an English binding README and link to it from the repository's top-level README? That will make the new API discoverable and reviewable for upstream contributors. The build artifact list below should also include tikv_client_async.hpp, the main C++ wrapper.

lance6716 · 2026-10-14T09:01:41Z

+  }
+
+  /*
+   * Query whether the request callback has completed.


is_complete() can return true before the callback finishes: Rust sets completion.complete = true immediately before invoking it. Please describe this as terminal status availability; drain() is the operation that waits for callbacks to return.

lance6716

Additional observability review: these comments focus on what the metrics snapshot represents and what remains invisible at the C/C++ boundary.

lance6716 · 2026-10-14T09:06:13Z

+fn collect_metrics_snapshot() -> MetricsSnapshot {
+    let mut samples = Vec::new();
+
+    for family in prometheus::gather() {


This gathers the entire process-wide default registry, but MetricSample retains only the type label and the match below silently omits gauges, summaries, and other metric types. Two series that differ only in another label become indistinguishable to C/C++ consumers. Please either limit and document this as a snapshot of the known client-rust counter/histogram families, or preserve all labels and supported metric types in the ABI. A test with two series differing in a non-type label would make the contract clear.

lance6716 · 2026-10-14T09:06:14Z

+                || keepalive.lifecycle.cancel_inflight.load(Ordering::Acquire)
+        };
+
+        let (status, output, error_message) = match result {


From a monitoring perspective, this maps an underlying successful RPC to an FFI CANCELLED completion, while the existing tikv_request_total/duration metrics still count the RPC as successful. The binding also has no outcome metrics for rejected submits, cancellations, panics, or in-flight requests. Please consider binding-level outcome and in-flight metrics so operators can reconcile API outcomes with TiKV RPC activity.

lance6716 · 2026-10-14T09:06:14Z

+TTL 的批量 put/delete、真实到期，以及无 TTL key 在等待期间持续存在。测试
+使用最长 10 秒的轮询等待到期，不依赖固定时长的 sleep。
+
+`metrics_snapshot_demo` 会在同一进程内发起一次 raw put/get/delete，


Please document the scope of these values for monitoring consumers: prometheus::gather() is process-wide across client instances, and tikv_request_total counts underlying TiKV RPC attempts rather than one sample per C/C++ API call (retries and multi-phase batches can produce multiple samples). This distinction matters when using the snapshot for request rates and error ratios.

lance6716 · 2026-10-14T09:06:14Z

+                raw_get_duration_count = Some(sample.value);
+            }
+
+            if name == "tikv_request_duration_seconds"


The snapshot test confirms that an infinite bucket exists, but does not check its cumulative value, finite buckets, or histogram sum. Please assert those values with a known observation and test the intended label behavior; that would catch monitoring output regressions rather than only metric presence.

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has signed the dco. labels Oct 14, 2026

ti-chi-bot Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 14, 2026

lance6716 commented Oct 14, 2026

View reviewed changes

feat: add async C and C++ bindings for TiKV RawKV

29d9d90

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 force-pushed the lance6716/remaining-client-rust-changes branch from 00c9ea1 to 29d9d90 Compare October 14, 2026 09:13

lance6716 mentioned this pull request Oct 14, 2026

feat: add async C bindings for TiKV RawKV #572

Draft

Conversation

lance6716 commented Oct 14, 2026

Summary

Review status

Testing

Uh oh!

ti-chi-bot Bot commented Oct 14, 2026

Uh oh!

coderabbitai Bot commented Oct 14, 2026

Draft PR not reviewed

Uh oh!

lance6716 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lance6716 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant