KVStore: reduce log (#10813)

[ti-chi-bot]

This is an automated cherry-pick of #10813

What problem does this PR solve?

Issue Number: ref #10814

Problem Summary:

What is changed and how it works?

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

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

None

Summary by CodeRabbit

• Refactor
  • Internal persistence logic refactored for safer message handling and more consistent flush/persist behavior.
  • Debug/log messages enriched with additional context for flush and force-persist operations, improving observability.
  • Safer string handling reduces risk of formatting issues during persistence.

[ti-chi-bot]

@JaySon-Huang This PR has conflicts, I have hold it.
Please resolve them or ask others to resolve them, then comment /unhold to remove the hold label.

[ti-chi-bot]

@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 39316c6a-e618-4060-bccf-16440579ee04

📥 Commits

Reviewing files that changed from the base of the PR and between 7fc9959 and 99b9a65.

📒 Files selected for processing (1)

• dbms/src/Storages/KVStore/MultiRaft/Persistence.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• dbms/src/Storages/KVStore/MultiRaft/Persistence.cpp

---

📝 Walkthrough

Walkthrough

Method signatures for KVStore persistence were changed to use std::string_view; flush decision points now build structured diagnostic messages and pass them through forceFlushRegionDataImpl into persistRegion only when non-empty.

Changes

Persistence Messaging Modernization

Layer / File(s)	Summary
Header: Include and method signatures dbms/src/Storages/KVStore/KVStore.h	Adds <string_view> and updates signatures: forceFlushRegionDataImpl gains std::string_view persist_extra_msg; persistRegion's extra_msg becomes std::string_view.
persistRegion implementation update dbms/src/Storages/KVStore/MultiRaft/Persistence.cpp	persistRegion accepts extra_msg as std::string_view and appends it into the caller string only when non-empty.
forceFlushRegionDataImpl signature & forwarding dbms/src/Storages/KVStore/MultiRaft/Persistence.cpp	forceFlushRegionDataImpl now accepts persist_extra_msg and forwards it to persistRegion instead of using an empty-string argument.
Message enrichment at flush call sites dbms/src/Storages/KVStore/MultiRaft/Persistence.cpp	tryFlushRegionData and canFlushRegionDataImpl build structured messages (force_persist_msg / flush_msg with term/index/truncated/gap/ids) and pass them into forceFlushRegionDataImpl.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through headers, found a view,
Replaced old chars with slices true,
Flushes whisper term and index bright,
Persist listens only when words alight,
A tidy trail of messages through the night.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description largely follows the template structure but lacks substantive content; the commit-message block is empty, and the problem summary and 'What is changed' sections are not filled in, making it unclear what this PR actually accomplishes.	Fill in the Problem Summary and 'What is changed and how it works' sections with concrete details about the API refactoring, including rationale and impact. Include the commit message explaining the changes.
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'KVStore: reduce log (#10813)' is vague and does not clearly communicate the actual technical change—updating method signatures to use std::string_view for better logging context.	Replace with a more descriptive title that explains the actual change, such as 'KVStore: use std::string_view for extra_msg parameters in persist methods' or 'KVStore: refactor persistence logging to use string_view'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@dbms/src/Storages/KVStore/MultiRaft/Persistence.cpp`:
- Around line 222-249: Resolve the leftover merge markers by keeping the
incoming flush_msg construction and the LOG_DEBUG that uses it, remove the
conflict markers and the old LOG_INFO block, and ensure flush_msg is declared
before the call to forceFlushRegionDataImpl so
forceFlushRegionDataImpl(curr_region, ..., flush_msg) compiles; also fix the
unknown accessor curr_region_ptr->getRegionTableSize() either by replacing it
with the correct existing accessor on the region object (e.g., the actual method
that returns the in-memory table size) or by adding/declaring
getRegionTableSize() in the region class, and keep usage of curr_region and
curr_region_ptr consistent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0246688f-e497-46db-8de6-7a4d16c7d8ad

📥 Commits

Reviewing files that changed from the base of the PR and between 2560b3d and 7fc9959.

📒 Files selected for processing (2)

• dbms/src/Storages/KVStore/KVStore.h
• dbms/src/Storages/KVStore/MultiRaft/Persistence.cpp

[JaySon-Huang]

/hold
wait for pingcap/tidb-engine-ext#457 and update proxy commit

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-05-29 09:15:11.79680773 +0000 UTC m=+10652.171617446: ☑️ agreed by JinheLin.

[ti-chi-bot]

@kolafish: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JinheLin, kolafish

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 [JinheLin]

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