From 94979903d1247f2499f63676b2a8327f315d74c4 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Fri, 16 Dec 2022 00:32:30 +0800 Subject: [PATCH 1/7] fix batch get Signed-off-by: shiyuhang <1136742008@qq.com> --- src/main/java/org/tikv/txn/KVClient.java | 8 ++- src/test/java/org/tikv/txn/BatchGetTest.java | 76 ++++++++++++++++++++ src/test/java/org/tikv/txn/TXNTest.java | 2 +- 3 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/tikv/txn/BatchGetTest.java diff --git a/src/main/java/org/tikv/txn/KVClient.java b/src/main/java/org/tikv/txn/KVClient.java index dfa9b8b2962..4962e868934 100644 --- a/src/main/java/org/tikv/txn/KVClient.java +++ b/src/main/java/org/tikv/txn/KVClient.java @@ -50,6 +50,7 @@ public class KVClient implements AutoCloseable { private final RegionStoreClientBuilder clientBuilder; private final TiConfiguration conf; private final ExecutorService executorService; + private Set resolvedLocks = new HashSet<>(); public KVClient(TiConfiguration conf, RegionStoreClientBuilder clientBuilder, TiSession session) { Objects.requireNonNull(conf, "conf is null"); @@ -223,6 +224,10 @@ private List doSendBatchGetInBatchesWithRetry( if (oldRegion.equals(currentRegion)) { RegionStoreClient client = clientBuilder.build(batch.getRegion()); + // set resolvedLocks for the new client + if (!resolvedLocks.isEmpty()) { + client.addResolvedLocks(version, resolvedLocks); + } try { return client.batchGet(backOffer, batch.getKeys(), version); } catch (final TiKVException e) { @@ -230,7 +235,8 @@ private List doSendBatchGetInBatchesWithRetry( clientBuilder.getRegionManager().invalidateRegion(batch.getRegion()); logger.warn("ReSplitting ranges for BatchGetRequest", e); - // retry + // get resolved locks and retry + resolvedLocks = client.getResolvedLocks(version); return doSendBatchGetWithRefetchRegion(backOffer, batch, version); } } else { diff --git a/src/test/java/org/tikv/txn/BatchGetTest.java b/src/test/java/org/tikv/txn/BatchGetTest.java new file mode 100644 index 00000000000..7a77beb04d3 --- /dev/null +++ b/src/test/java/org/tikv/txn/BatchGetTest.java @@ -0,0 +1,76 @@ +package org.tikv.txn; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; + +import com.google.protobuf.ByteString; +import java.util.Arrays; +import java.util.List; +import org.junit.Test; +import org.tikv.common.BytePairWrapper; +import org.tikv.common.ByteWrapper; +import org.tikv.common.exception.KeyException; +import org.tikv.common.util.BackOffer; +import org.tikv.common.util.ConcreteBackOffer; +import org.tikv.kvproto.Kvrpcpb.KvPair; + +public class BatchGetTest extends TXNTest { + + @Test + public void BatchGetResolveLockTest() throws Exception { + long lockTTL = 20000L; + long startTS = session.getTimestamp().getVersion(); + String key1 = "batchGetResolveLockTestKey1"; + String key2 = "batchGetResolveLockTestKey2"; + String val1 = "val1"; + String val2 = "val2"; + + new Thread( + () -> { + try (TwoPhaseCommitter twoPhaseCommitter = + new TwoPhaseCommitter(session, startTS, lockTTL)) { + byte[] primaryKey = key1.getBytes("UTF-8"); + byte[] secondary = key2.getBytes("UTF-8"); + // prewrite primary key + twoPhaseCommitter.prewritePrimaryKey( + ConcreteBackOffer.newCustomBackOff(5000), primaryKey, val1.getBytes("UTF-8")); + List pairs = + Arrays.asList(new BytePairWrapper(secondary, val2.getBytes("UTF-8"))); + // prewrite secondary key + twoPhaseCommitter.prewriteSecondaryKeys(primaryKey, pairs.iterator(), 5000); + + // get commitTS + long commitTS = session.getTimestamp().getVersion(); + Thread.sleep(5000); + // commit primary key + twoPhaseCommitter.commitPrimaryKey( + ConcreteBackOffer.newCustomBackOff(5000), primaryKey, commitTS); + // commit secondary key + List keys = Arrays.asList(new ByteWrapper(secondary)); + twoPhaseCommitter.commitSecondaryKeys(keys.iterator(), commitTS, 5000); + } catch (Exception e) { + KeyException keyException = (KeyException) e.getCause().getCause(); + assertNotSame("", keyException.getKeyErr().getCommitTsExpired().toString()); + } + }) + .start(); + + // wait 2PC get commitTS + Thread.sleep(2000); + try (KVClient kvClient = session.createKVClient()) { + long version = session.getTimestamp().getVersion(); + ByteString k1 = ByteString.copyFromUtf8(key1); + ByteString k2 = ByteString.copyFromUtf8(key2); + + BackOffer backOffer = ConcreteBackOffer.newCustomBackOff(5000); + List kvPairs = kvClient.batchGet(backOffer, Arrays.asList(k1, k2), version); + // Since TiKV v4.0.0 write locked key will not block read. it is supported by Min Commit + // Timestamp + assertEquals(ByteString.copyFromUtf8(val1), kvPairs.get(0).getValue()); + assertEquals(ByteString.copyFromUtf8(val2), kvPairs.get(1).getValue()); + } + + // wait 2PC commit finish + Thread.sleep(10000); + } +} diff --git a/src/test/java/org/tikv/txn/TXNTest.java b/src/test/java/org/tikv/txn/TXNTest.java index 92af0383da1..386ad8182e0 100644 --- a/src/test/java/org/tikv/txn/TXNTest.java +++ b/src/test/java/org/tikv/txn/TXNTest.java @@ -41,7 +41,7 @@ public class TXNTest extends BaseTxnKVTest { static final int DEFAULT_TTL = 10; - private TiSession session; + public TiSession session; RegionStoreClient.RegionStoreClientBuilder builder; @Before From 97dcb41571df56584ad554383d806ec35c8b11a4 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Fri, 16 Dec 2022 00:40:44 +0800 Subject: [PATCH 2/7] add license and fix it Signed-off-by: shiyuhang <1136742008@qq.com> --- src/test/java/org/tikv/txn/BatchGetTest.java | 26 ++++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/tikv/txn/BatchGetTest.java b/src/test/java/org/tikv/txn/BatchGetTest.java index 7a77beb04d3..493cc94a713 100644 --- a/src/test/java/org/tikv/txn/BatchGetTest.java +++ b/src/test/java/org/tikv/txn/BatchGetTest.java @@ -1,7 +1,23 @@ +/* + * Copyright 2022 TiKV Project Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + package org.tikv.txn; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotSame; import com.google.protobuf.ByteString; import java.util.Arrays; @@ -9,7 +25,6 @@ import org.junit.Test; import org.tikv.common.BytePairWrapper; import org.tikv.common.ByteWrapper; -import org.tikv.common.exception.KeyException; import org.tikv.common.util.BackOffer; import org.tikv.common.util.ConcreteBackOffer; import org.tikv.kvproto.Kvrpcpb.KvPair; @@ -48,9 +63,7 @@ public void BatchGetResolveLockTest() throws Exception { // commit secondary key List keys = Arrays.asList(new ByteWrapper(secondary)); twoPhaseCommitter.commitSecondaryKeys(keys.iterator(), commitTS, 5000); - } catch (Exception e) { - KeyException keyException = (KeyException) e.getCause().getCause(); - assertNotSame("", keyException.getKeyErr().getCommitTsExpired().toString()); + } catch (Exception ignore) { } }) .start(); @@ -69,8 +82,5 @@ public void BatchGetResolveLockTest() throws Exception { assertEquals(ByteString.copyFromUtf8(val1), kvPairs.get(0).getValue()); assertEquals(ByteString.copyFromUtf8(val2), kvPairs.get(1).getValue()); } - - // wait 2PC commit finish - Thread.sleep(10000); } } From a891ea5e900c73181faa01cb82e5d7e02e0e1bc3 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Fri, 16 Dec 2022 01:05:43 +0800 Subject: [PATCH 3/7] Fix IT Signed-off-by: shiyuhang <1136742008@qq.com> --- src/test/java/org/tikv/txn/BatchGetTest.java | 22 +++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/tikv/txn/BatchGetTest.java b/src/test/java/org/tikv/txn/BatchGetTest.java index 493cc94a713..5547a6cd92f 100644 --- a/src/test/java/org/tikv/txn/BatchGetTest.java +++ b/src/test/java/org/tikv/txn/BatchGetTest.java @@ -18,6 +18,7 @@ package org.tikv.txn; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; import com.google.protobuf.ByteString; import java.util.Arrays; @@ -25,6 +26,7 @@ import org.junit.Test; import org.tikv.common.BytePairWrapper; import org.tikv.common.ByteWrapper; +import org.tikv.common.exception.KeyException; import org.tikv.common.util.BackOffer; import org.tikv.common.util.ConcreteBackOffer; import org.tikv.kvproto.Kvrpcpb.KvPair; @@ -39,7 +41,14 @@ public void BatchGetResolveLockTest() throws Exception { String key2 = "batchGetResolveLockTestKey2"; String val1 = "val1"; String val2 = "val2"; + String val1_update = "val1_update"; + String val2_update = "val2_update"; + // put key1 and key2 + putKV(key1, val1); + putKV(key2, val2); + + // run 2PC background new Thread( () -> { try (TwoPhaseCommitter twoPhaseCommitter = @@ -48,9 +57,11 @@ public void BatchGetResolveLockTest() throws Exception { byte[] secondary = key2.getBytes("UTF-8"); // prewrite primary key twoPhaseCommitter.prewritePrimaryKey( - ConcreteBackOffer.newCustomBackOff(5000), primaryKey, val1.getBytes("UTF-8")); + ConcreteBackOffer.newCustomBackOff(5000), + primaryKey, + val1_update.getBytes("UTF-8")); List pairs = - Arrays.asList(new BytePairWrapper(secondary, val2.getBytes("UTF-8"))); + Arrays.asList(new BytePairWrapper(secondary, val2_update.getBytes("UTF-8"))); // prewrite secondary key twoPhaseCommitter.prewriteSecondaryKeys(primaryKey, pairs.iterator(), 5000); @@ -63,13 +74,16 @@ public void BatchGetResolveLockTest() throws Exception { // commit secondary key List keys = Arrays.asList(new ByteWrapper(secondary)); twoPhaseCommitter.commitSecondaryKeys(keys.iterator(), commitTS, 5000); - } catch (Exception ignore) { + } catch (Exception e) { + KeyException keyException = (KeyException) e.getCause().getCause(); + assertNotSame("", keyException.getKeyErr().getCommitTsExpired().toString()); } }) .start(); // wait 2PC get commitTS Thread.sleep(2000); + // batch get key1 and key2 try (KVClient kvClient = session.createKVClient()) { long version = session.getTimestamp().getVersion(); ByteString k1 = ByteString.copyFromUtf8(key1); @@ -81,6 +95,8 @@ public void BatchGetResolveLockTest() throws Exception { // Timestamp assertEquals(ByteString.copyFromUtf8(val1), kvPairs.get(0).getValue()); assertEquals(ByteString.copyFromUtf8(val2), kvPairs.get(1).getValue()); + // wait 2PC finish + Thread.sleep(10000); } } } From 24b9285ec0dcfcf81f796bdc7542542e4b6877a9 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Fri, 16 Dec 2022 01:36:07 +0800 Subject: [PATCH 4/7] Fix IT Signed-off-by: shiyuhang <1136742008@qq.com> --- src/test/java/org/tikv/txn/BatchGetTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/tikv/txn/BatchGetTest.java b/src/test/java/org/tikv/txn/BatchGetTest.java index 5547a6cd92f..cbdff1b3920 100644 --- a/src/test/java/org/tikv/txn/BatchGetTest.java +++ b/src/test/java/org/tikv/txn/BatchGetTest.java @@ -36,7 +36,6 @@ public class BatchGetTest extends TXNTest { @Test public void BatchGetResolveLockTest() throws Exception { long lockTTL = 20000L; - long startTS = session.getTimestamp().getVersion(); String key1 = "batchGetResolveLockTestKey1"; String key2 = "batchGetResolveLockTestKey2"; String val1 = "val1"; @@ -51,6 +50,7 @@ public void BatchGetResolveLockTest() throws Exception { // run 2PC background new Thread( () -> { + long startTS = session.getTimestamp().getVersion(); try (TwoPhaseCommitter twoPhaseCommitter = new TwoPhaseCommitter(session, startTS, lockTTL)) { byte[] primaryKey = key1.getBytes("UTF-8"); @@ -95,6 +95,7 @@ public void BatchGetResolveLockTest() throws Exception { // Timestamp assertEquals(ByteString.copyFromUtf8(val1), kvPairs.get(0).getValue()); assertEquals(ByteString.copyFromUtf8(val2), kvPairs.get(1).getValue()); + System.out.println(kvPairs); // wait 2PC finish Thread.sleep(10000); } From fe3763b3956de5138ce22c408bbf619e036fd24c Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Fri, 16 Dec 2022 11:39:17 +0800 Subject: [PATCH 5/7] trigger ci Signed-off-by: shiyuhang <1136742008@qq.com> From cbd7223062787d82763e5e70ec86c9576fa32d71 Mon Sep 17 00:00:00 2001 From: shiyuhang <1136742008@qq.com> Date: Tue, 20 Dec 2022 17:30:56 +0800 Subject: [PATCH 6/7] use emptyset to avoid memory waste Signed-off-by: shiyuhang <1136742008@qq.com> --- src/main/java/org/tikv/txn/KVClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/tikv/txn/KVClient.java b/src/main/java/org/tikv/txn/KVClient.java index 4962e868934..e8c83c54463 100644 --- a/src/main/java/org/tikv/txn/KVClient.java +++ b/src/main/java/org/tikv/txn/KVClient.java @@ -50,7 +50,7 @@ public class KVClient implements AutoCloseable { private final RegionStoreClientBuilder clientBuilder; private final TiConfiguration conf; private final ExecutorService executorService; - private Set resolvedLocks = new HashSet<>(); + private Set resolvedLocks = Collections.emptySet(); public KVClient(TiConfiguration conf, RegionStoreClientBuilder clientBuilder, TiSession session) { Objects.requireNonNull(conf, "conf is null"); From 2f897b3cf575ac68eeb38414041b7a8067323bae Mon Sep 17 00:00:00 2001 From: Xiang Zhang Date: Tue, 20 Dec 2022 17:12:10 +0800 Subject: [PATCH 7/7] Delete stale-checker.yml (#681) Signed-off-by: zhangyangyu Signed-off-by: shiyuhang <1136742008@qq.com> --- .github/workflows/stale-checker.yml | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 .github/workflows/stale-checker.yml diff --git a/.github/workflows/stale-checker.yml b/.github/workflows/stale-checker.yml deleted file mode 100644 index 478fc504f7a..00000000000 --- a/.github/workflows/stale-checker.yml +++ /dev/null @@ -1,19 +0,0 @@ -name: 'Stale Checker' -on: - schedule: - - cron: '0 0 * * *' - -jobs: - stale: - runs-on: ubuntu-latest - steps: - - uses: actions/stale@v4 - with: - days-before-stale: 30 - stale-issue-message: 'This issue is stale because it has been open 30 days with no activity.' - stale-issue-label: 'status/stale' - days-before-issue-close: -1 - stale-pr-message: 'This PR is stale because it has been open 30 days with no activity. Remove the status/stale label or comment or this PR will be closed in 7 days.' - stale-pr-label: 'status/stale' - days-before-pr-close: 7 - close-pr-message: 'This PR was closed because it has been stalled for 7 days with no activity.'