unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
api-versioningerror-handlingtesting-strategy

store: fix double-encoded API V2 keys in StoreBatchCop lock resolution

Project: pingcap/tidb | Language: Go | Concepts: api-versioning, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/pingcap/tidb/pull/70680

The problem

When using API V2, StoreBatchCop lock resolution could fail because lock keys (specifically the PrimaryLock field) were being double-encoded with the API V2 prefix. This caused TiDB to send an incorrectly encoded key to the CheckTxnStatus request, resulting in an error (Error 1105) and failed lock resolution.

Why it's non-trivial

A naive fix might just decode or encode keys at the point of use, but this can introduce subtle bugs if the encoding/decoding is not consistently handled throughout the stack. The key format must be correct at every layer: decoding too early or encoding too late can break compatibility, especially when mixing legacy and API V2 keyspaces. Correctness requires that the key is encoded exactly once before being sent to TiKV, and that all cache and lock resolution logic works with the expected key format.

The approach

1. Update dependency to bring in the fix from client-go

The core fix is actually in the client-go library, so the PR updates the dependency to a version that includes the proper key decoding logic:

-	github.com/tikv/client-go/v2 v2.0.8-0.20260831055105-6793680de8f2
+	github.com/tikv/client-go/v2 v2.0.8-0.20260831103552-e4905600583b

This ensures that the logic for decoding API V2 keys is present.

2. Regenerate Bazel metadata to match the new dependency

The Bazel build system's dependency lock file is updated to reference the new client-go version and checksum:

-        sum = "h1:QzYTd/5rcLgMX1o46/GkhxAsii7YIaYqs5aLZxjftwY=",
-        version = "v2.0.8-0.20260831055105-6793680de8f2",
+        sum = "h1:GPImWDAr0eb6FqS89eNcO3AdEbGq5ygQhIEaftOl0IY=",
+        version = "v2.0.8-0.20260831103552-e4905600583b",

This keeps Bazel and Go module builds in sync.

3. Add a regression test to verify correct key encoding

A new test is added to ensure that the lock resolution logic only encodes the key once for API V2, preventing regressions:

func testHandleBatchCopResponseResolvesAPIV2ChildLock(t *testing.T) {
    ...
    rawClient := &apiV2StoreBatchLockClient{
        Client: mockClient,
        copResponse: &tikvrpc.Response{Resp: &coprocessor.Response{
            BatchResponses: []*coprocessor.StoreBatchTaskResponse{
                {
                    TaskId: child.taskID,
                    Locked: &kvrpcpb.LockInfo{
                        Key:         codec.EncodeKey([]byte("key")),
                        PrimaryLock: codec.EncodeKey(primaryKey),
                        LockVersion: 1,
                        // Keep lock cleanup synchronous with the test.
                        LockType: kvrpcpb.Op_PessimisticLock,
                    },
                },
            },
        }},
    }
    ...
    // Lock resolution must encode the decoded primary exactly once.
    require.Equal(t, codec.EncodeKey(primaryKey), rawClient.checkTxnStatusPrimaryKey)
}

This test sets up a mock client and simulates a lock with an API V2-encoded primary key. It then checks that the outgoing CheckTxnStatus request uses the correctly encoded key (not double-encoded).

4. Add documentation to clarify the decoding point

A clarifying comment is added to the coprocessor logic to explain where and why key decoding happens:

+	// client-go decodes API v2 bucket keys before this cache update; otherwise the
+	// cache would compare encoded boundaries with decoded task keys. Malformed keys
+	// fail response decoding before reaching this point.

From the review thread

No notable review discussion on this PR.

Takeaways

  • Always test key encoding/decoding at API boundaries: When supporting multiple key formats or API versions, ensure that encoding/decoding happens at well-defined boundaries, and add regression tests to catch double-encoding bugs.
  • Keep dependencies in sync across build systems: When fixing bugs in upstream libraries, update all relevant dependency files (e.g., Go modules and Bazel) to avoid build inconsistencies.
  • Document subtle data handling logic: Add comments explaining why and where data is transformed, especially when the logic is non-obvious or could lead to subtle bugs if changed.