Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TiKV Rust Client RFC #7

Open
wants to merge 36 commits into
base: master
from
Open
Changes from 2 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
7419c9b
TiKV Rust Client
sunxiaoguang Oct 25, 2018
64aad0b
Add more about API usage
sunxiaoguang Oct 31, 2018
57cc178
Fix some gramma mistakes
sunxiaoguang Nov 2, 2018
e82ac4e
Fix indentation
sunxiaoguang Nov 2, 2018
43ed161
Fixed a typo
sunxiaoguang Nov 5, 2018
5368649
Do not move key for get, scan and delete methods
sunxiaoguang Nov 8, 2018
0cfcaff
Remove KeyRange and use RangeBounds trait instead
sunxiaoguang Nov 9, 2018
aea8d85
Use builder style interface for raw kv
sunxiaoguang Nov 14, 2018
510791f
Use impl Trait whenever possible
sunxiaoguang Nov 14, 2018
bf8ddab
Update example code
sunxiaoguang Nov 15, 2018
5cb6085
Update crate name to 'tikv-client'
sunxiaoguang Nov 15, 2018
584e88a
Add example code to create Config with security
sunxiaoguang Nov 15, 2018
b239698
Add transaction isolation level example
sunxiaoguang Nov 15, 2018
7dd712d
Merge branch 'master' into tikv-client-rust
Hoverbear Nov 15, 2018
78d6100
Clarify that Raw/Transaction APIs are mutually exclusive
sunxiaoguang Nov 16, 2018
cbfceb5
Do not use trait in APIs
sunxiaoguang Nov 16, 2018
9215f8c
Fix formatting issue with wrong indention
sunxiaoguang Nov 16, 2018
6b22b3e
Fix indention and threading model
sunxiaoguang Nov 18, 2018
9b892a3
Use concrete future type for transactional api
sunxiaoguang Nov 18, 2018
c8b0167
Merge branch 'master' into tikv-client-rust
Hoverbear Nov 19, 2018
80457b2
Change example to reflect changes of Config
sunxiaoguang Nov 19, 2018
b68e34b
Remove `Programming Model` section and make it TBD
sunxiaoguang Nov 20, 2018
bd05fde
Fix a typo
sunxiaoguang Nov 21, 2018
e3c080c
Format text to make lines fit into 80 characters
sunxiaoguang Nov 22, 2018
04569e8
provide a C-compatible binding for future clients
sunxiaoguang Nov 22, 2018
e692e80
Merge branch 'master' into tikv-client-rust
Hoverbear Dec 20, 2018
1a7c67f
Fix lints
Hoverbear Dec 20, 2018
e4ef825
add schedule limit design (#13)
disksing Dec 24, 2018
aa950ff
Unified Log Format RFC (#18)
breeswish Jan 7, 2019
5eb89c3
RFC: PD Simulator (#20)
rleungx Jan 28, 2019
252eb33
Update RFC to match code
Hoverbear Feb 20, 2019
9d2e4c3
Merge branch 'master' into tikv-client-rust
Hoverbear Feb 21, 2019
41d029d
Fix lints
Hoverbear Feb 25, 2019
2c77911
Merge branch 'tikv-client-rust' of github.com:sunxiaoguang/rfcs into …
Hoverbear Feb 25, 2019
bbb504c
Merge branch 'master' into tikv-client-rust
Hoverbear May 9, 2019
1ff37ef
Merge branch 'master' into tikv-client-rust
Hoverbear Jul 29, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,386 @@
# Summary

Introduce a full featured, official TiKV (and PD) Rust client. It is intended to be used as a reference implementation, or to provide C-compatible binding for future clients.

# Motivation

Currently, users of TiKV must use [TiDB's Go Client](https://github.com/pingcap/tidb/blob/master/store/tikv/client.go), which is not well packaged or documented. We would like to ensure that users can easily use TiKV and PD without needing to use TiDB.

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

Suggested change
Currently, users of TiKV must use [TiDB's Go Client](https://github.com/pingcap/tidb/blob/master/store/tikv/client.go), which is not well packaged or documented. We would like to ensure that users can easily use TiKV and PD without needing to use TiDB.
Currently, TiKV users must use [TiKV's Go Client provided by TiDB](https://github.com/pingcap/tidb/blob/master/store/tikv/client.go), which is not well packaged or documented. We would like to ensure that users can easily use TiKV and PD without needing to use TiDB.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

@lilin90 Any comments?

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 15, 2018

Member

Both are OK. But I think we can add the Java client now: https://github.com/tikv/client-java


We think this would help encourage community participation in the TiKV project and associated libraries. During talks with several potential corporate users we discovered that there was an interest in using TiKV to resolve concerns such as caching and raw key-value stores.

# Detailed design
## Supported targets
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@crazycoder1988

We will target the `stable` channel of Rust starting in the Rust 2018 edition. We choose to begin with Rust 2018 so we do not need to concern ourselves with an upgrade path.

We will also support the most recent `nightly` version of Rust, but users should not feel the need to reach for stable unless they are already using it.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

"Reach for nightly" 😁

This comment has been minimized.

Copy link
@sunxiaoguang

## Naming

The [Rust API Guidelines](https://rust-lang-nursery.github.io/api-guidelines/naming.html) do not perscribe any particular crate name convention.
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@sticnarf

sticnarf Nov 21, 2018

Suggested change
The [Rust API Guidelines](https://rust-lang-nursery.github.io/api-guidelines/naming.html) do not perscribe any particular crate name convention.
The [Rust API Guidelines](https://rust-lang-nursery.github.io/api-guidelines/naming.html) do not prescribe any particular crate name convention.

Fix a typo

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 21, 2018

Author

Fixed, Thanks a lot


We choose to name the crate `tikv_client` to conform to the constraints presented to us by the Rust compiler. Cargo permits `tikv-client` and `tikv_client`, but `rustc` does not permit `tikv-client` so we choose to use `tikv_client` to reduce mental overhead.
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

We chose tikv-client.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

@Hoverbear PTAL about this line. I'm not sure if I made it clear.


Choosing to seperate `tikv` and `client` helps potentially unfamiliar users to immediately understand the intent of the package. `tikvclient`, while understandable, is not immediately parsable by a human.

All structures and functions will otherwise follow the [Rust API Guidelines](https://rust-lang-nursery.github.io/api-guidelines/), some of which will be enforced by `clippy`.

## Installation

To utilize the client programmatically, users will be able to add the `tikv-client` crate to their `Cargo.toml`'s dependencies. Then they must use the crate with `use tikv_client;`. Unfortunately due to Rust’s naming conventions this inconsistency is in place.

To utilize the command line client, users will be able to install the binary via `cargo install tikv-client`. They will then be able to access the client through the binary `tikv-client`. If they wish for a different name they can alias it in their shell.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

Maybe we should leave this for a future RFC to prevent this one from becoming huge. Besides we cannot effectively start on this until after the library is done.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

This CLI section was removed.

This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

What's the scope of a command line client? Is it like tikv-ctl or something else?

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 14, 2018

Member

@sunxiaoguang I think we talked about leaving the CLI client to another RFC, do you agree? If so lets remove this.

This comment has been minimized.

Copy link
@gregwebs

gregwebs Nov 14, 2018

tikv-ctl means administrative operations of TiKV.
I think it is best to work on that independently from this data API to the extent it is possible.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

Sure. Let me remove the tikv-ctl part.


## Usage

## Two types of APIs

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 15, 2018

Member

If I recall correctly we need to advise users that they are mutually exclusive, and they must choose one or the other. I think that is worth mentioning in this section.

This comment has been minimized.

Copy link
@sunxiaoguang
TiKV provides two types of APIs for developers:
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@crazycoder1988
- The Raw Key-Value API

If your application scenario does not need distributed transactions or MVCC (Multi-Version Concurrency Control) and only need to guarantee the atomicity towards one key, you can use the Raw Key-Value API.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@rleungx

rleungx Nov 2, 2018

Member
Suggested change
If your application scenario does not need distributed transactions or MVCC (Multi-Version Concurrency Control) and only need to guarantee the atomicity towards one key, you can use the Raw Key-Value API.
If your application scenario does not need distributed transactions or MVCC (Multi-Version Concurrency Control) and only needs to guarantee the atomicity towards one key, you can use the Raw Key-Value API.

This comment has been minimized.

Copy link
@sunxiaoguang

- The Transactional Key-Value API

If your application scenario requires distributed ACID transactions and the atomicity of multiple keys within a transaction, you can use the Transactional Key-Value API.

Generally the Raw Key-Value API has higher throughput and lower latency compare to the Transactional Key-Value API. If distributed ACID transactions is not required, Raw Key-Value API is preferred over Transactional Key-Value API for better performance and ease of use.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@lilin90

lilin90 Nov 2, 2018

Member
Suggested change
Generally the Raw Key-Value API has higher throughput and lower latency compare to the Transactional Key-Value API. If distributed ACID transactions is not required, Raw Key-Value API is preferred over Transactional Key-Value API for better performance and ease of use.
Generally, the Raw Key-Value API has higher throughput and lower latency compared to the Transactional Key-Value API. If distributed ACID transactions are not required, Raw Key-Value API is preferred over Transactional Key-Value API for better performance and ease of use.

This comment has been minimized.

Copy link
@sunxiaoguang

The client provides two types of APIs in two separate modules for developers to choose from.

### The common data types

- Key: raw binary data
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

Better to leave the actual Rust type as well. If it is already a type name, please surround it with ``.

- Value: raw binary data
- KvPair: Key-value pair type
- KeyRange: Half-open interval of keys
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

Please define "Half-open"

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

Sorry that I forgot to remove KeyRange here. It's been removed and changed to standard RangeBounds trait so it can represent both half-open and closed intervals after edition.

- Config: Configuration for client

### Try the Raw Key-Value API
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member
Suggested change
### Try the Raw Key-Value API
### Raw Key-Value API Basic Usage

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

Since this is an RFC we are more focused on "What using this looks and feels like" over teaching users how to use it. We must think beyond the simple cases. :)


To use the Raw Key-Value API, take the following steps:

1. Create an instance of Config to specify endpoints of PD (Placement Driver) and optional security config

```rust
let config = Config::new(vec!["127.0.0.1:2379"]);
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

A security config is mentioned but the example does not include one

This comment has been minimized.

Copy link
@sunxiaoguang
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

I would prefer something like:

let config = Config::default()
    .pd_address(&[...])
    .tls(...);

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 14, 2018

Member

pd_address is a required field at this time, TiKV won't work without it, so it's better to have it as a required argument.

```

2. Create a Raw Key-Value client.

```rust
let client = RawClient::new(&config);
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

Maybe RawKvClient is better because it is actually not a raw client, but a raw (KV) client.

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 14, 2018

Member

I think tikv_client::RawClient already implies the Kv. Users can choose to rename it when they import it if they are dealing with multiple Client types. You may notice for example Hyper doesn't have HyperClient or HttpClient, it just has Client

```

3. Call the Raw Key-Value client methods to access the data on TiKV. The Raw Key-Value API contains following methods

```rust
fn get<K, C>(&self, key: K, cf: C) -> KvFuture<Value>
where
K: Into<Key>,
C: Into<Option<String>>;
fn batch_get<I, K, C>(&self, keys: I, cf: C) -> KvFuture<Vec<KvPair>>
where
I: IntoIterator<Item = K>,
K: Into<Key>,
C: Into<Option<String>>;
fn put<P, C>(&self, pair: P, cf: C) -> KvFuture<()>
where
P: Into<KvPair>,
C: Into<Option<String>>;
fn batch_put<I, P, C>(&self, pairs: I, cf: C) -> KvFuture<()>
where
I: IntoIterator<Item = P>,
P: Into<KvPair>,
C: Into<Option<String>>;
fn delete<K, C>(&self, key: K, cf: C) -> KvFuture<()>
where
K: Into<Key>,
C: Into<Option<String>>;
fn batch_delete<I, K, C>(&self, keys: I, cf: C) -> KvFuture<()>
where
I: IntoIterator<Item = K>,
K: Into<Key>,
C: Into<Option<String>>;
fn scan<R, C>(&self, range: R, limit: u32, key_only: bool, cf: C) -> KvFuture<Vec<KvPair>>
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

I wonder if it is easier for users if we stick to key/range being the first param and CF always being the second. 🤔

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 6, 2018

Author

Compare to other parameters, the cf parameter is optional and is it a good idea to have an optional parameter in the middle? Although it is an inappropriate example, but C++ default arguments have to come after normal arguments.

BTW: I have another comment on this signature. Having a KvPair seems to make sense because it might be returned from get. But KeyRange is always input arguments, we can easily break it into two arguments at cost of one more argument. Maybe it's better to just remove this type?

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 8, 2018

Member

It might make sense. 🤔

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 9, 2018

Author

I removed KeyRange type and use RangeBounds for scan and batch_scan. It makes better sense to me now. PTAL @Hoverbear

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 9, 2018

Author

I looked at HashMap interface and the insert method of HashMap takes two arguments for key and value. Maybe we should do similar thing to Raw interface put method?

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 15, 2018

Member

I was wondering this as well. Maybe @gregwebs has some feedback?

where
R: Into<KeyRange>,
C: Into<Option<String>>;
fn batch_scan<I, R, C>(
&self,
ranges: I,
each_limit: u32,
key_only: bool,
cf: C,
) -> KvFuture<Vec<KvPair>>
where
I: IntoIterator<Item = R>,
R: Into<KeyRange>,
C: Into<Option<String>>;
fn delete_range<R, C>(&self, range: R, cf: C) -> KvFuture<()>
where
R: Into<KeyRange>,
C: Into<Option<String>>;
```

#### Usage example of the Raw Key-Value API

```rust
extern crate futures;
extern crate tikv_client;
use futures::future::Future;
use tikv_client::raw::Client;
use tikv_client::*;
fn main() {
let config = Config::new(vec!["127.0.0.1:3379"]);
let raw = raw::RawClient::new(&config)
.wait()
.expect("Could not connect to tikv");
let key: Key = b"Company".to_vec().into();
let value: Value = b"PingCAP".to_vec().into();
raw.put((Clone::clone(&key), Clone::clone(&value)), None)
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

It would be very nice to avoid the clones here.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 1, 2018

Author

I also had a feeling that moving params makes user's life harder. But API Guideline says it is preferable to let caller decide where to copy and place data. So does it really make sense to follow the guideline here?

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 1, 2018

Member

Hm, you're right. I think in the case of put it's okay... I note that HashMap::get takes a borrow though: https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.get

I think in this case mutations, eg put, which actually go and send the value (and thus almost certainly need to do memory work) should take. But maybe we can make the searching/get based functions accept just a borrow? Similar to HashMap or HashSet?

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 2, 2018

Author

The gRPC part is going to move whatever parameters passed in. In addition the nature of asynchronous programming model requires moving and sending parameters to event loop thread, so it doesn't matter if it's mutation or not the arguments are going to be moved anyway. However I do feel this particular API guideline applies to regular libraries better then the scenario we are facing. So breaking this rule probably is a better choice as it makes our users' life easier.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 8, 2018

Author

I changed it to AsRef and it does look better. Please take a look at the new commit. @Hoverbear

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 8, 2018

Member

@siddontang What do you think about this?

.wait()
.expect("Could not put kv pair to tikv");
println!("Successfully put {:?}:{:?} to tikv", key, value);
let value = raw
.get(Clone::clone(&key), None)
.wait()
.expect("Could not get value");
println!("Found val: {:?} for key: {:?}", value, key);
raw.delete(Clone::clone(&key), None)
.wait()
.expect("Could not delete value");
println!("Key: {:?} deleted", key);
raw.get(key, None)
.wait()
.expect_err("Get returned value for not existing key");
}
```

The result is like:

```bash
Successfully put Key([67, 111, 109, 112, 97, 110, 121]):Value([80, 105, 110, 103, 67, 65, 80]) to tikv
Found val: Value([80, 105, 110, 103, 67, 65, 80]) for key: Key([67, 111, 109, 112, 97, 110, 121])
Key: Key([67, 111, 109, 112, 97, 110, 121]) deleted
```

Raw Key-Value client is a client of the TiKV server and only supports the GET/BATCH_GET/PUT/BATCH_PUT/DELETE/BATCH_DELETE/SCAN/BATCH_SCAN/DELETE_RANGE commands. The Raw Key-Value client can be safely and concurrently accessed by multiple threads. Therefore, for one process, one client is enough generally.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 15, 2018

Member

"...a client of the TiKV server" I feel this is obvious. Maybe it can be removed.

This comment has been minimized.

Copy link
@sunxiaoguang

### Try the Transactional Key-Value API
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

The previous section is called "Usage ... " and this section might follow the same style as well


The Transactional Key-Value API is more complicated than the Raw Key-Value API. Some transaction related concepts are listed as follows.

- Client

Like the Raw Key-Value client, a Client is client to a TiKV cluster.

- Snapshot

A Snapshot is the state of a Client at a particular point of time, which provides some readonly methods. The multiple times read from a same Snapshot is guaranteed consistent.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

"multiple reads of the same snapshot..."

This comment has been minimized.

Copy link
@sunxiaoguang

- Transaction

Like the Transaction in SQL, a Transaction symbolizes a series of read and write operations performed within the Client. Internally, a Transaction consists of a Snapshot for reads, and a buffer for all writes. The default isolation level of a Transaction is Snapshot Isolation.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

Since we're talking about a default we should detail how to configure it

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

Sure, let me add it.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

Added an optional step in example.

This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 15, 2018

Member
Suggested change
Like the Transaction in SQL, a Transaction symbolizes a series of read and write operations performed within the Client. Internally, a Transaction consists of a Snapshot for reads, and a buffer for all writes. The default isolation level of a Transaction is Snapshot Isolation.
Like Transactions in SQL, a Transaction in TiKV symbolizes a series of read and write operations performed within the service. Internally, a Transaction consists of a Snapshot for reads, and a buffer for all writes. The default isolation level of a Transaction is Snapshot Isolation.

This comment has been minimized.

Copy link
@sunxiaoguang

To use the Transactional Key-Value API, take the following steps:

1. Create an instance of Config to specify endpoints of PD (Placement Driver) and optional security config

```rust
let config = Config::new(vec!["127.0.0.1:2379"]);
```

2. Create a Transactional Key-Value client.

```rust
let client = TxnClient::new(&config);
```
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@lilin90

lilin90 Nov 2, 2018

Member

Please also indent the code blocks in step 1&2 like that in step 5.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 2, 2018

Author

Fixed, but I'm not sure if I'm doing it right, please double check it.


4. (Optional) Modify data using a Transaction.

The lifecycle of a Transaction is: _begin → {get, set, delete, scan} → {commit, rollback}_.

5. Call the Transactional Key-Value API's methods to access the data on TiKV. The Transactional Key-Value API contains the following methods:

```rust
fn begin(&self) -> KvFuture<Transaction>;
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

What is KvFuture?

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

It's added to common data types section now. Thanks

fn get<K>(&self, key: K) -> KvFuture<Value>
where
K: Into<Key>;
fn set<P>(&mut self, pair: P) -> KvFuture<()>
where
P: Into<KvPair>;
fn delete<K>(&mut self, key: K) -> KvFuture<()>
where
K: Into<Key>;
fn seek<K>(&self, key: K) -> KvFuture<Scanner>
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@crazycoder1988

crazycoder1988 Nov 14, 2018

how to do reverse seek and specify upper bound & lower bound?

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

There is a reverse_seek method that resembles the Go Client.

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 15, 2018

Author

BTW: Just updated it to use RangeBounds as well. Since ScanRequest supports has end_keys now. Using RangeBounds makes better sense.

where
K: Into<Key>; Begin() -> Txn
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

Oh, is this valid?

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 5, 2018

Author

Hm, not sure if I misunderstood anything here. If you mean if it compiles, then the answer is true. I compiled the sample code with nightly toolchain.

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 8, 2018

Member

I meant K: Into<Key>; Begin() -> Txn seems like it might be invalid syntax to me. Maybe it's a copy/paste mistake?

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 8, 2018

Author

Oh, I see what you mean now. The Begin() -> Txn was a typo, I probably copied it from golang document and forgot to delete it. It's removed now.

fn commit(self) -> KvFuture<()>;
fn rollback(self) -> KvFuture<()>;
```

### Usage example of the Transactional Key-Value API

```rust
extern crate futures;
extern crate tikv_client;
use futures::{Async, Future, Stream};
use tikv_client::transaction::{Client, Mutator, Retriever, TxnClient};
use tikv_client::*;
fn puts<P, I>(client: &TxnClient, pairs: P)
where
P: IntoIterator<Item = I>,
I: Into<KvPair>,
{
let mut txn = client.begin().wait().expect("Could not begin transaction");
let _: Vec<()> = pairs
.into_iter()
.map(Into::into)
.map(|p| {
txn.set(p).wait().expect("Could not set key value pair");
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

Maybe another an example that uses something like futures::join_all? I think many users won't want to call .wait() here.

This comment has been minimized.

Copy link
@sunxiaoguang
}).collect();
txn.commit().wait().expect("Could not commit transaction");
}
fn get(client: &TxnClient, key: Key) -> Value {
let txn = client.begin().wait().expect("Could not begin transaction");
txn.get(key).wait().expect("Could not get value")
}
fn scan(client: &TxnClient, start: Key, limit: usize) {
let txn = client.begin().wait().expect("Could not begin transaction");
let mut scanner = txn.seek(start).wait().expect("Could not seek to start key");
let mut limit = limit;
loop {
if limit == 0 {
break;
}
match scanner.poll() {
Ok(Async::Ready(None)) => return,
Ok(Async::Ready(Some(pair))) => {
limit -= 1;
println!("{:?}", pair);
}
_ => break,
}
}
}
fn dels<P>(client: &TxnClient, pairs: P)
where
P: IntoIterator<Item = Key>,
{
let mut txn = client.begin().wait().expect("Could not begin transaction");
let _: Vec<()> = pairs
.into_iter()
.map(|p| {
txn.delete(p).wait().expect("Could not delete key");
}).collect();
txn.commit().wait().expect("Could not commit transaction");
}
fn main() {
let config = Config::new(vec!["127.0.0.1:3379"]);
let txn = TxnClient::new(&config)
.wait()
.expect("Could not connect to tikv");
// set
let key1: Key = b"key1".to_vec().into();
let value1: Value = b"value1".to_vec().into();
let key2: Key = b"key2".to_vec().into();
let value2: Value = b"value2".to_vec().into();
puts(&txn, vec![(key1, value1), (key2, value2)]);
// get
let key1: Key = b"key1".to_vec().into();
let value1 = get(&txn, Clone::clone(&key1));
println!("{:?}", (key1, value1));
// scan
let key1: Key = b"key1".to_vec().into();
scan(&txn, key1, 10);
// delete
let key1: Key = b"key1".to_vec().into();
let key2: Key = b"key2".to_vec().into();
dels(&txn, vec![key1, key2]);
}
```

The result is like:

```bash
(Key([107, 101, 121, 49]), Value([118, 97, 108, 117, 101, 49]))
(Key([107, 101, 121, 49]), Value([118, 97, 108, 117, 101, 49]))
(Key([107, 101, 121, 49]), Value([118, 97, 108, 117, 101, 49]))
```

## Programming model

The client instance is thread safe and all the interfaces return futures so users can use the client in either asynchronous or synchronous way. A dedicated event loop thread is created at per client instance basis to drive the reactor to make progress.
This conversation was marked as resolved by Hoverbear

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

created at a`

Suggested change
The client instance is thread safe and all the interfaces return futures so users can use the client in either asynchronous or synchronous way. A dedicated event loop thread is created at per client instance basis to drive the reactor to make progress.
The client instance is thread safe and all the interfaces return futures so users can use the client in either asynchronous or synchronous way. A dedicated event loop thread is created at a per client instance basis to drive the reactor to make progress.

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Oct 31, 2018

Member

Is it possible to attach to another event loop?

This comment has been minimized.

Copy link
@sunxiaoguang

sunxiaoguang Nov 5, 2018

Author

Not right now. Maybe it is intimidating for users to know eventloop?

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Nov 8, 2018

Member

I think it is, let's leave this for the future. :)


## Tooling

The `tikv_client` crate will be tested with Travis CI using Rust's standard testing framework. We will also include benchmarking with criterion in the future. For public functions which process user input, we will seek to use fuzz testing such as `quickcheck` to find subtle bugs.
This conversation was marked as resolved by sunxiaoguang

This comment has been minimized.

Copy link
@rleungx

rleungx Nov 2, 2018

Member
Suggested change
The `tikv_client` crate will be tested with Travis CI using Rust's standard testing framework. We will also include benchmarking with criterion in the future. For public functions which process user input, we will seek to use fuzz testing such as `quickcheck` to find subtle bugs.
The `tikv_client` crate will be tested with Travis CI using Rust's standard testing framework. We will also include benchmark with criterion in the future. For public functions which process user input, we will seek to use fuzz testing such as `quickcheck` to find subtle bugs.

This comment has been minimized.

Copy link
@sunxiaoguang

The CI will validate all code is warning free, passes `rustfmt`, and passes a `clippy` check without lint warnings.

All code that reaches the `master` branch should not output errors when the following commands are run:

```shell
cargo fmt --all -- --check
cargo clippy --all -- -D clippy
cargo test --all -- --nocapture
cargo bench --all -- --test
```

# Drawbacks

Choosing not to create a Rust TiKV client would mean the current state of clients remains the same.

It is likely that in the future we would end up creating a client in some other form due to customer demand.

# Alternatives

## Package the Go client

Choosing to do this would likely be considerably much less work. The code is already written, so most of the work would be documenting and packaging. Unfortunately, Go does not share the same performance characteristics and FFI capabilities as Rust, so it is a poor core binding for future libraries. Due to the limited abstractions available in Go (it does not have a Linear Type System) we may not be able to create the semantic abstractions possible in a Rust client, reducing the quality of implementations referencing the client.

## Choose another language

We can choose another language such as C, C++, or Python.

A C client would be the most portable and allow future users and customers to bind to the library as they wish. This quality is maintained in Rust, so it is not an advantage for C. Choosing to implement this client in C or C++ means we must take extra steps to support multiple packaging systems, string libraries, and other choices which would not be needed in languages like Ruby, Node.js, or Python.

Choosing to use Python or Ruby for this client would likely be considerably less work than C/C++ as there is a reduced error surface. These languages do not offer good FFI bindings, and often require starting up a language runtime. We suspect that if we implement a C/C++/Rust client, future dynamic language libraries will be able to bind to the Rust client, allowing them to be written quickly and easily.

# Unresolved questions

There are some concerns about integration testing. While we can use the mock `Pd` available in TiKV to mock PD, we do not currently have something similar for TiKV. We suspect implementing a mock TiKV will be the easiest method.

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.