KVStore: Fix fail to restore tikv value larger than 16MiB by JaySon-Huang · Pull Request #10055 · pingcap/tiflash

JaySon-Huang · 2025-03-28T10:02:23Z

What problem does this PR solve?

Issue Number: close #10052

Problem Summary:

# tidb.toml
[performance]
txn-entry-size-limit = 125829120
# tiup edit
#    performance.txn-entry-size-limit: 125829120

# tikv.toml
[raftstore]
raft-entry-max-size = '120MiB'
# tiup edit
#     raftstore.raft-entry-max-size: '120MiB'

set global max_allowed_packet = 1073741824;
CREATE TABLE `t` (
  `id` bigint(21) unsigned NOT NULL,
  `text` longtext DEFAULT NULL
);

TiDB could allow a raft-log with 120MiB at max, while TiFlash Region::deserialize use readBinary2<std::string> only allow 16MiB at max. So if there are some raftlog larger than 16MiB and flushed to RegionData, then TiFlash could fail to restart when restoring Region data from disk.
If the raftlog is committed, the rows are decoded into Block and write down to IStorage. Reading/writing rows larger than 16MiB in the IStorage layer does not cause any error.

What is changed and how it works?

KVStore: Fix restoring large tikv value
Add function `readTiKVStringBinary` for `TiKVValue::deserialize` and `TiKVKey::deserialize` to allow restoring tikv key/values at most 120MiB

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Fix an issue that TiFlash may fail to restart when user write rows larger than 16MiB

dbms/src/Storages/KVStore/Utils/SerializationHelper.h

Signed-off-by: JaySon-Huang <tshent@qq.com>

ti-chi-bot · 2025-03-31T09:42:32Z

[LGTM Timeline notifier]

Timeline:

2025-03-31 09:36:39.230697813 +0000 UTC m=+1471492.914933908: ☑️ agreed by JinheLin.
2025-03-31 09:42:31.666629692 +0000 UTC m=+1471845.350865788: ☑️ agreed by Lloyd-Pottiger.

CalvinNeo

lgtm

ti-chi-bot · 2025-03-31T09:44:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CalvinNeo, JinheLin, Lloyd-Pottiger

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [CalvinNeo,JinheLin,Lloyd-Pottiger]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-04-01T05:34:35Z

In response to a cherrypick label: new pull request created to branch release-8.5: #10067.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2025-04-01T05:34:41Z

In response to a cherrypick label: new pull request created to branch release-8.1: #10068.
But this PR has conflicts, please resolve them!

ti-chi-bot · 2025-04-01T05:34:46Z

In response to a cherrypick label: new pull request created to branch release-7.5: #10069.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

…10067) close #10052 KVStore: Fix restoring large tikv value Add function `readTiKVStringBinary` for `TiKVValue::deserialize` and `TiKVKey::deserialize` to allow restoring tikv key/values at most 120MiB Signed-off-by: JaySon-Huang <tshent@qq.com> Co-authored-by: JaySon-Huang <tshent@qq.com> Co-authored-by: JaySon <tshent@qq.com>

…10069) close #10052 KVStore: Fix restoring large tikv value Add function `readTiKVStringBinary` for `TiKVValue::deserialize` and `TiKVKey::deserialize` to allow restoring tikv key/values at most 120MiB Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: JaySon-Huang <tshent@qq.com> Co-authored-by: JaySon <tshent@qq.com> Co-authored-by: JaySon-Huang <tshent@qq.com>

…10068) close #10052 KVStore: Fix restoring large tikv value Add function `readTiKVStringBinary` for `TiKVValue::deserialize` and `TiKVKey::deserialize` to allow restoring tikv key/values at most 120MiB Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: JaySon-Huang <tshent@qq.com> Co-authored-by: JaySon <tshent@qq.com> Co-authored-by: JaySon-Huang <tshent@qq.com>

) close pingcap#10052 KVStore: Fix restoring large tikv value Add function `readTiKVStringBinary` for `TiKVValue::deserialize` and `TiKVKey::deserialize` to allow restoring tikv key/values at most 120MiB Signed-off-by: JaySon-Huang <tshent@qq.com>

CalvinNeo reviewed Mar 31, 2025

View reviewed changes

dbms/src/Storages/KVStore/Utils/SerializationHelper.h Outdated Show resolved Hide resolved

JaySon-Huang force-pushed the fix_large_tikv_value branch from 836ff21 to 82a283c Compare March 31, 2025 03:39

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 31, 2025

Fix deserialize large tikv value && add unit test

5e96448

Signed-off-by: JaySon-Huang <tshent@qq.com>

JaySon-Huang force-pushed the fix_large_tikv_value branch from 5baf623 to 5e96448 Compare March 31, 2025 05:07

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed do-not-merge/needs-triage-completed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 31, 2025

JaySon-Huang added 2 commits March 31, 2025 13:11

Avoid disk io in ut

c30f7d0

Signed-off-by: JaySon-Huang <tshent@qq.com>

Update comments

4438ec8

Signed-off-by: JaySon-Huang <tshent@qq.com>

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 31, 2025

JaySon-Huang changed the title ~~[WIP]KVStore: Fix restoring large tikv value~~ KVStore: Fix fail to restore tikv value larger than 16MiB Mar 31, 2025

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 31, 2025

Merge branch 'master' into fix_large_tikv_value

d500881

JaySon-Huang requested review from CalvinNeo, JinheLin and Lloyd-Pottiger March 31, 2025 09:34

JinheLin approved these changes Mar 31, 2025

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Mar 31, 2025

Lloyd-Pottiger approved these changes Mar 31, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 31, 2025

CalvinNeo approved these changes Mar 31, 2025

View reviewed changes

ti-chi-bot bot merged commit c4bf6f4 into pingcap:master Mar 31, 2025
5 checks passed

JaySon-Huang deleted the fix_large_tikv_value branch March 31, 2025 10:07

ti-chi-bot bot added needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. labels Apr 1, 2025

ti-chi-bot mentioned this pull request Apr 1, 2025

KVStore: Fix fail to restore tikv value larger than 16MiB (#10055) #10067

Merged

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Apr 1, 2025

This is an automated cherry-pick of pingcap#10055

1118bf9

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Apr 1, 2025

KVStore: Fix fail to restore tikv value larger than 16MiB (#10055) #10068

Merged

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Apr 1, 2025

This is an automated cherry-pick of pingcap#10055

59c5fbd

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Apr 1, 2025

KVStore: Fix fail to restore tikv value larger than 16MiB (#10055) #10069

Merged

12 tasks

ti-chi-bot bot removed needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. labels Apr 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KVStore: Fix fail to restore tikv value larger than 16MiB#10055

KVStore: Fix fail to restore tikv value larger than 16MiB#10055
ti-chi-bot[bot] merged 4 commits intopingcap:masterfrom
JaySon-Huang:fix_large_tikv_value

JaySon-Huang commented Mar 28, 2025 •

edited

Loading

Uh oh!

Uh oh!

ti-chi-bot bot commented Mar 31, 2025

Uh oh!

CalvinNeo left a comment

Uh oh!

ti-chi-bot bot commented Mar 31, 2025

Uh oh!

Uh oh!

ti-chi-bot commented Apr 1, 2025

Uh oh!

ti-chi-bot commented Apr 1, 2025

Uh oh!

ti-chi-bot commented Apr 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

JaySon-Huang commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

Uh oh!

ti-chi-bot bot commented Mar 31, 2025

[LGTM Timeline notifier]

Uh oh!

CalvinNeo left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Mar 31, 2025

Uh oh!

Uh oh!

ti-chi-bot commented Apr 1, 2025

Uh oh!

ti-chi-bot commented Apr 1, 2025

Uh oh!

ti-chi-bot commented Apr 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JaySon-Huang commented Mar 28, 2025 •

edited

Loading