siddontang: - Github Plus

delete branch

siddontang delete branch gh-readonly-queue/master/pr-1526-911ba088dfa57a517d98603f1cb69d80554de6d5

Created at 3 days ago

opened issue

need to remove any pingcap/tidb dependency in this repo

I found that client-go still uses TiDB mockstore and some codes for testing, like https://github.com/tikv/client-go/blob/master/integration_tests/util_test.go#L50

From my side, TiKV is the underlying component of TiDB, so it is very strange to let the TiKV client depend on TiDB's code, even in the test, and I also don't think we must rely on the TiDB's logic to test the TiKV client.

Another risk is that if we heavily use TiDB's mockstore, and client-go's origin mockstore may then lack maintenance. I tried to use client-go's own store instead of TiDB's mockstore below

@@ -83,7 +83,7 @@ type testLockSuite struct {
 }

 func (s *testLockSuite) SetupTest() {
-       s.store = tikv.StoreProbe{KVStore: NewTestUniStore(s.T())}
+       s.store = tikv.StoreProbe{KVStore: NewTestStore(s.T())}
 }

But failed with the following message:

   --- FAIL: TestLock/TestBatchResolveLocks (0.00s)
        lock_test.go:563:
            	Error Trace:	/Users/tangliu/eng/client-go/integration_tests/lock_test.go:563
            	Error:      	Expected nil, but got: not exist
            	Test:       	TestLock/TestBatchResolveLocks
        lock_test.go:564:
            	Error Trace:	/Users/tangliu/eng/client-go/integration_tests/lock_test.go:564
            	Error:      	Should be true
            	Test:       	TestLock/TestBatchResolveLocks
        lock_test.go:566:
            	Error Trace:	/Users/tangliu/eng/client-go/integration_tests/lock_test.go:566
            	Error:      	Expected nil, but got: not exist
            	Test:       	TestLock/TestBatchResolveLocks
        lock_test.go:567:
            	Error Trace:	/Users/tangliu/eng/client-go/integration_tests/lock_test.go:567
            	Error:      	Should be true
            	Test:       	TestLock/TestBatchResolveLocks

I didn't investigate deeply, but my guess is that the mockstore in client-go doesn't support lock TTL very well.

I know removing TiDB's dependency is a very hard work, but we need to consider it now. Let's follow some good engineering practices from now on.

Another thing is now we already have a mockstore in client-go, can we simplify the mockstore code in TiDB too (maybe let TiDB mostly use client-go's code). I still worry that later we may still focus on changing the code in TiDB instead of client-go, especially when we develop Transaction features.

/cc @zhangjinpeng1987 @kevin-xianliu

Created at 5 days ago

opened issue

tikv/client-go

why calculate keyspace ID every time in the request with V2 VPI

I found that now with V2 API, we will calculate the keyspace every time

func (c *codecV2) GetKeyspaceID() KeyspaceID {
	prefix := append([]byte{}, c.prefix...)
	prefix[0] = 0
	return KeyspaceID(binary.BigEndian.Uint32(prefix))
}

https://github.com/tikv/client-go/blob/master/internal/apicodec/codec_v2.go#L99

Why do we do this? seem that we can cache the value of KeyspaceID in the codecV2 structure directly.

@sunxiaoguang

Created at 1 week ago

push

siddontang/client-go