[to #555] enable api v2 #575

iosmanthus · 2022-03-24T11:28:45Z

Signed-off-by: iosmanthus myosmanthustree@gmail.com

What problem does this PR solve?

attach API version to context to AbstractRegionStoreClient, RegionStoreClient, LockResolverClientV4.
simple API conf check.

Issue Number: close #555 , and tracked by #582

Check List for Tests

This PR has been tested by at least one of the following methods:

Unit test
- RawKV compatibility
- TxnKV compatibility
Integration test

Related changes

Need to cherry-pick the release branch: v3.3

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

zz-jason · 2022-03-25T08:39:31Z

src/main/java/org/tikv/common/region/AbstractRegionStoreClient.java

@@ -155,6 +156,42 @@ public boolean onStoreUnreachable(BackOffer backOffer) {
    return false;
  }

+  public ByteString buildRequestKey(ByteString key) {
+    switch (conf.getApiVersion()) {


can we get the cluster version and tikv configuration from PD? so that we don't need to add one more config in the java client and the java client can automatically adjust the encode/decode methods according to the cluster it is connected to.

By design, the client should not read the TiKV configuration from PD or themself. Because:

TiKV configurations could be different from each other due to the restart process.

TiDB use client-go, which should have the same behaviour as the client-java, however, TiDB would never use API v2 but use API v1 instead, due to they have a key prefix with m or t. Thus, client-go should not read the API version from the cluster components, instead, just set the version to 1.

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

codecov · 2022-03-28T11:15:04Z

Codecov Report

Merging #575 (629d4fa) into master (774ee50) will increase coverage by 0.38%.
The diff coverage is 79.72%.

@@             Coverage Diff              @@
##             master     #575      +/-   ##
============================================
+ Coverage     34.11%   34.50%   +0.38%     
- Complexity     1369     1411      +42     
============================================
  Files           270      278       +8     
  Lines         17244    17342      +98     
  Branches       1958     1970      +12     
============================================
+ Hits           5882     5983     +101     
+ Misses        10752    10747       -5     
- Partials        610      612       +2

Impacted Files	Coverage Δ
src/main/java/org/tikv/common/ConfigUtils.java	`0.00% <ø> (ø)`
src/main/java/org/tikv/common/StoreVersion.java	`69.76% <ø> (ø)`
...ikv/common/operation/iterator/RawScanIterator.java	`82.75% <ø> (ø)`
.../java/org/tikv/txn/AbstractLockResolverClient.java	`15.00% <0.00%> (-10.00%)`	⬇️
src/main/java/org/tikv/txn/Lock.java	`0.00% <0.00%> (ø)`
...c/main/java/org/tikv/txn/LockResolverClientV4.java	`4.54% <0.00%> (ø)`
...java/org/tikv/common/operation/KVErrorHandler.java	`39.47% <50.00%> (-3.77%)`	⬇️
...va/org/tikv/common/apiversion/RequestKeyCodec.java	`53.33% <53.33%> (ø)`
src/main/java/org/tikv/common/TiConfiguration.java	`67.37% <75.00%> (+0.50%)`	⬆️
src/main/java/org/tikv/common/TiSession.java	`70.23% <76.92%> (-0.06%)`	⬇️
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 774ee50...629d4fa. Read the comment docs.

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus · 2022-04-19T13:57:10Z

src/test/java/org/tikv/common/ApiVersionTest.java

+import org.tikv.common.TiConfiguration.ApiVersion;
+import org.tikv.raw.RawKVClient;
+
+public class ApiVersionTest {


@pingyu PTAL on this compatible test for API v2.

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

src/main/java/org/tikv/common/ConfigUtils.java

src/main/java/org/tikv/common/TiConfiguration.java

…aster' of github.com:tikv/client-java into api-v2 Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

…n api v2 Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

src/main/java/org/tikv/common/apiversion/CodecUtils.java

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

zhangyangyu

Much better! One thing I am curious is the usage of Pair. There are many places using it, like encodeRange. Why not directly use the Range type?

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus · 2022-05-31T16:02:44Z

The Range type will use the default comparator for the elements, it tends to be an actual range that use to compute if an element is between the min and max. The Pair type tells us that the return value is a tuple, nothing more.

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

ti-srebot · 2022-06-02T07:18:04Z

@xuanyu66, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. You are not a reviewer or committer or co-leader or leader.

src/main/java/org/tikv/common/PDClient.java

src/main/java/org/tikv/common/TiSession.java

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

ti-srebot · 2022-06-02T08:11:07Z

@zhangyangyu, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. You are not a reviewer or committer or co-leader or leader.

iosmanthus · 2022-06-02T08:24:53Z

/auto-merge

iosmanthus · 2022-06-02T08:25:10Z

/merge

ti-srebot · 2022-06-02T08:25:32Z

/run-all-tests

api-v2: enable api-v2

1c97d49

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

github-actions bot added the component/common label Mar 24, 2022

Merge branch 'master' of github.com:tikv/client-java into api-v2

0d485f2

zz-jason reviewed Mar 25, 2022

View reviewed changes

attach api version to context and fix lock resolver api version

f45d583

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

github-actions bot added the component/txnkv label Mar 25, 2022

iosmanthus added 3 commits March 28, 2022 14:48

./dev/javafmt

8ff27f2

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

fix api v2 config test

7bb666b

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

add topb test

790ffd2

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus mentioned this pull request Mar 29, 2022

TiKV API V2 Testing Tracking issue #582

Open

4 tasks

fix ingest interface for rawkv

c5e502e

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus force-pushed the api-v2 branch from e6bbb1a to c5e502e Compare April 6, 2022 07:30

iosmanthus added 5 commits April 12, 2022 11:27

fix ingest interface for rawkv

8a11a88

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

Merge branch 'master' of github.com:tikv/client-java into api-v2

ec2d127

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

Merge branch 'master' of github.com:tikv/client-java into api-v2

9b0363c

add api version test

f9f4cfe

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

add license header for new files

e5ff7e0

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus changed the title ~~[to #555] [WIP] enable api v2~~ [to #555] enable api v2 Apr 19, 2022

iosmanthus commented Apr 19, 2022

View reviewed changes

fix ApiVersionTest for older version of TiKV

3e63dc4

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

zz-jason reviewed Apr 22, 2022

View reviewed changes

src/main/java/org/tikv/common/ConfigUtils.java Show resolved Hide resolved

src/main/java/org/tikv/common/TiConfiguration.java Show resolved Hide resolved

iosmanthus added 8 commits May 13, 2022 00:53

Merge branch 'api-v2' of github.com:iosmanthus/client-java; branch 'm…

e44dee7

…aster' of github.com:tikv/client-java into api-v2 Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

fix kvproto wrong commit

04fa138

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

wip: add mock test for error handler for EpochNotMatch region error i…

66d07c4

…n api v2 Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

Merge branch 'master' of github.com:tikv/client-java into api-v2

153ec6f

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

add mock test for EpochNotMatch in API v2

6771d13

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

fix license header

b7c54f8

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

add some comments for epoch not match mock test

72eb371

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

git fire!: add tests for scanRegions

8443588

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus added 6 commits May 30, 2022 20:18

refactor: introduce RequestKeyCodec to reduce if

34a81a0

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

./dev/javafmt

a72a6cf

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

remove codec in TiConfiguration

de87cbb

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

remove decodeRegion in PDClient

6d43080

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

remove extra ;

ef71788

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

Merge branch 'api-v2' of github.com:iosmanthus/client-java into api-v2

d252888

sunxiaoguang reviewed May 30, 2022

View reviewed changes

src/main/java/org/tikv/common/apiversion/CodecUtils.java Show resolved Hide resolved

src/main/java/org/tikv/common/apiversion/CodecUtils.java Show resolved Hide resolved

add batch encode method for RequestKeyCodec

fec799c

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

zhangyangyu reviewed May 31, 2022

View reviewed changes

print stack trace for unstable tests

0a64f84

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus added 5 commits June 1, 2022 00:37

fix wrong base class for PDClientV2MockTest

cedc40d

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

wip: add tests for RequestKeyCodec

afd5858

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

./dev/javafmt

7c02154

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

complete RequestKeyCodecTest

0e0bd46

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

remove if in scanRegions

1db88aa

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

xuanyu66 previously approved these changes Jun 2, 2022

View reviewed changes

zhangyangyu reviewed Jun 2, 2022

View reviewed changes

src/main/java/org/tikv/common/PDClient.java Outdated Show resolved Hide resolved

zhangyangyu reviewed Jun 2, 2022

View reviewed changes

src/main/java/org/tikv/common/TiSession.java Outdated Show resolved Hide resolved

change Pair type to org.tikv.common.util.Pair

629d4fa

Signed-off-by: iosmanthus <myosmanthustree@gmail.com>

iosmanthus dismissed xuanyu66’s stale review via 629d4fa June 2, 2022 08:08

zhangyangyu approved these changes Jun 2, 2022

View reviewed changes

ti-srebot added the status/can-merge label Jun 2, 2022

iosmanthus merged commit 9ce52d3 into tikv:master Jun 2, 2022

iosmanthus deleted the api-v2 branch June 2, 2022 08:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[to #555] enable api v2 #575

[to #555] enable api v2 #575

iosmanthus commented Mar 24, 2022 •

edited

Loading

zz-jason Mar 25, 2022

iosmanthus Mar 28, 2022

codecov bot commented Mar 28, 2022 •

edited

Loading

iosmanthus Apr 19, 2022

pingyu Apr 20, 2022

zhangyangyu left a comment

iosmanthus commented May 31, 2022

ti-srebot commented Jun 2, 2022

ti-srebot commented Jun 2, 2022

iosmanthus commented Jun 2, 2022

iosmanthus commented Jun 2, 2022

ti-srebot commented Jun 2, 2022

[to #555] enable api v2 #575

[to #555] enable api v2 #575

Conversation

iosmanthus commented Mar 24, 2022 • edited Loading

What problem does this PR solve?

Check List for Tests

Related changes

zz-jason Mar 25, 2022

Choose a reason for hiding this comment

iosmanthus Mar 28, 2022

Choose a reason for hiding this comment

codecov bot commented Mar 28, 2022 • edited Loading

Codecov Report

iosmanthus Apr 19, 2022

Choose a reason for hiding this comment

pingyu Apr 20, 2022

Choose a reason for hiding this comment

zhangyangyu left a comment

Choose a reason for hiding this comment

iosmanthus commented May 31, 2022

ti-srebot commented Jun 2, 2022

ti-srebot commented Jun 2, 2022

iosmanthus commented Jun 2, 2022

iosmanthus commented Jun 2, 2022

ti-srebot commented Jun 2, 2022

iosmanthus commented Mar 24, 2022 •

edited

Loading

codecov bot commented Mar 28, 2022 •

edited

Loading