flash: handle TiCI estimate count RPC by wshwsh12 · Pull Request #10826 · pingcap/tiflash

wshwsh12 · 2026-04-27T02:34:42Z

What problem does this PR solve?

Issue Number: None

Problem Summary:

TiDB needs TiFlash to expose TiCI-side estimate count so the optimizer can use sampled TiCI fulltext cardinality instead of relying only on local planner estimates.

What is changed and how it works?

flash: handle GetEstimateTiCICount RPC for TiCI fulltext stats

Add FlashService handling for GetEstimateTiCICount.
Build TiCI estimate input from the request expression and shard/range list.
Return TiCI estimate count to TiDB and ignore missing shards during estimate.
Update kvproto to the branch containing the estimate count RPC definitions.

Check List

Tests

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

Manual test:

TiUP playground tag: tici-estimate-skew-20260426-2240.
Query: EXPLAIN SELECT COUNT(*) FROM estimate_skew WHERE MATCH(content) AGAINST ('heavyhit' IN BOOLEAN MODE);.
Result: TiDB used TiCI estimate 4321; exact match count was 4000; total table row count was 9400.
Logs confirmed TiFlash GetEstimateTiCICount done, est_count=4321 and TiCI EstimateCountResult { estimated_total_count: 4321, ... }.

Side effects

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

Documentation

Release note

None

coderabbitai · 2026-04-27T02:34:49Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (3)

release-8.5
release-7.5
release-8.1

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 29ae0122-e3d3-4150-b88d-6c76ed6ea289

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ 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

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

Signed-off-by: wshwsh12 <793703860@qq.com>

Copilot

Pull request overview

This PR adds a new TiFlash gRPC endpoint to support TiDB optimizer cardinality estimation for TiCI fulltext queries by forwarding an “estimate count” request to the TiCI search library and returning the result.

Changes:

Add FlashService::GetEstimateTiCICount gRPC handler and supporting request parsing / timezone handling.
Expose TiCIReadTaskPool::tipbToTiCIExpr for reuse when building the TiCI estimate query from tipb expressions.
Refactor getKeyRanges into a reusable helper in TantivyInputStream.h for building TiCI range inputs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

File	Description
dbms/src/Storages/Tantivy/TiCIReadTaskPool.h	Exposes expression conversion helper for building TiCI queries outside the task pool.
dbms/src/Storages/Tantivy/TantivyInputStream.h	Moves key-range conversion into a shared helper used by both scan and estimate paths.
dbms/src/Flash/FlashService.h	Declares the new `GetEstimateTiCICount` RPC handler.
dbms/src/Flash/FlashService.cpp	Implements the new RPC: parses query/ranges/timezone, calls TiCI `estimate_count`, and returns the estimate.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: wshwsh12 <793703860@qq.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ChangRui-Ryan · 2026-04-29T09:48:06Z

lgtm

Signed-off-by: wshwsh12 <793703860@qq.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: wshwsh12 <793703860@qq.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: wshwsh12 <793703860@qq.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T12:34:02Z

+        LOG_WARNING(log, "GetEstimateTiCICount failed with unknown exception");
+        response->set_other_error("other exception");


catch (...) currently logs only a generic message and sets other_error to a generic string, which makes unexpected failures hard to diagnose in production. Consider calling tryLogCurrentException(log, ...) (or otherwise logging the current exception detail) and returning an other_error that at least indicates an unknown exception occurred (so the server log can be correlated).

Suggested change

LOG_WARNING(log, "GetEstimateTiCICount failed with unknown exception");

response->set_other_error("other exception");

try

{

throw;

}

catch (const std::exception & e)

{

LOG_WARNING(log, "GetEstimateTiCICount failed with unknown std exception: {}", e.what());

}

catch (...)

{

LOG_WARNING(log, "GetEstimateTiCICount failed with unknown non-std exception");

}

response->set_other_error("unknown exception");

Copilot · 2026-04-29T12:34:03Z

+namespace DB::TS
+{
+
+// Convert literal value from timezone specified in cop request to UTC in-place.


The comment says the timezone conversion happens "in-place", but convertPackedU64WithTimezone takes from_time by value and returns the converted value instead of mutating an input parameter. Please adjust the comment (or the signature) so it accurately reflects the behavior.

Suggested change

// Convert literal value from timezone specified in cop request to UTC in-place.

// Convert a literal value from the timezone specified in the cop request to UTC and return the converted value.

Signed-off-by: wshwsh12 <793703860@qq.com>

windtalker

lgtm

ti-chi-bot · 2026-04-30T07:15:00Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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,windtalker]

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 · 2026-04-30T07:15:03Z

[LGTM Timeline notifier]

Timeline:

2026-04-30 07:09:24.718207631 +0000 UTC m=+2840969.923567678: ☑️ agreed by windtalker.
2026-04-30 07:15:02.398809724 +0000 UTC m=+2841307.604169781: ☑️ agreed by JinheLin.

ti-chi-bot · 2026-04-30T08:54:28Z

@wshwsh12: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-sanitizer-asan	`b0f8f1e`	link	false	`/test pull-sanitizer-asan`
pull-sanitizer-tsan	`b0f8f1e`	link	false	`/test pull-sanitizer-tsan`

Full PR test history. Your PR dashboard.

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

wshwsh12 added 4 commits April 23, 2026 16:34

flash: handle TiCI estimate count rpc

b27be83

flash: ignore missing shards in TiCI estimate

7d6434f

flash: update kvproto for TiCI estimate response

38ccef8

flash: use feature fts based kvproto

923684a

ti-chi-bot Bot added the release-note-none Denotes a PR that doesn't merit a release note. label Apr 27, 2026

ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 27, 2026

wshwsh12 added 3 commits April 29, 2026 14:44

contrib: update kvproto to feature fts

230fc80

Signed-off-by: wshwsh12 <793703860@qq.com>

flash: simplify TiCI estimate shard handling

0c88125

Signed-off-by: wshwsh12 <793703860@qq.com>

flash: reuse TiCI key range conversion

aec1ab7

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 requested review from Copilot and windtalker April 29, 2026 08:41

Copilot started reviewing on behalf of wshwsh12 April 29, 2026 08:42 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread dbms/src/Flash/FlashService.cpp Outdated

Comment thread dbms/src/Flash/FlashService.cpp Outdated

wshwsh12 added 2 commits April 29, 2026 16:52

flash: fix TiCI estimate range lifetime

6c7ab54

Signed-off-by: wshwsh12 <793703860@qq.com>

flash: avoid copying TiCI expression list

08210aa

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 requested a review from Copilot April 29, 2026 09:37

Copilot started reviewing on behalf of wshwsh12 April 29, 2026 09:38 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread dbms/src/Flash/FlashService.cpp Outdated

Comment thread dbms/src/Flash/FlashService.cpp Outdated

flash: refine TiCI estimate error handling

8a99477

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 requested a review from Copilot April 29, 2026 10:01

Copilot started reviewing on behalf of wshwsh12 April 29, 2026 10:02 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread dbms/src/Flash/FlashService.cpp Outdated

Comment thread dbms/src/Storages/Tantivy/TantivyInputStream.h Outdated

Comment thread dbms/src/Storages/Tantivy/TiCIReadTaskPool.h Outdated

flash: extract TiCI request utilities

729ef2d

Signed-off-by: wshwsh12 <793703860@qq.com>

ti-chi-bot Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 29, 2026

wshwsh12 requested a review from Copilot April 29, 2026 11:56

Copilot started reviewing on behalf of wshwsh12 April 29, 2026 11:57 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread dbms/src/Flash/FlashService.cpp Outdated

Comment thread dbms/src/Storages/Tantivy/TiCIRequestUtils.h

Comment thread dbms/src/Storages/Tantivy/TiCIRequestUtils.h Outdated

Comment thread dbms/src/Storages/Tantivy/TiCIRequestUtils.h Outdated

wshwsh12 added 2 commits April 29, 2026 20:04

flash: keep TiCI expr conversion internal

22695eb

Signed-off-by: wshwsh12 <793703860@qq.com>

flash: improve TiCI expr error messages

e405d9c

Signed-off-by: wshwsh12 <793703860@qq.com>

wshwsh12 requested a review from Copilot April 29, 2026 12:26

Copilot started reviewing on behalf of wshwsh12 April 29, 2026 12:27 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

flash: move TiCI estimate count into handler

b0f8f1e

Signed-off-by: wshwsh12 <793703860@qq.com>

windtalker approved these changes Apr 30, 2026

View reviewed changes

ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Apr 30, 2026

JinheLin approved these changes Apr 30, 2026

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 30, 2026

ti-chi-bot Bot merged commit 4d9b25a into pingcap:feature/fts Apr 30, 2026
5 of 7 checks passed

		LOG_WARNING(log, "GetEstimateTiCICount failed with unknown exception");
		response->set_other_error("other exception");

-        LOG_WARNING(log, "GetEstimateTiCICount failed with unknown exception");
-        response->set_other_error("other exception");
+        try
+        {
+            throw;
+        }
+        catch (const std::exception & e)
+        {
+            LOG_WARNING(log, "GetEstimateTiCICount failed with unknown std exception: {}", e.what());
+        }
+        catch (...)
+        {
+            LOG_WARNING(log, "GetEstimateTiCICount failed with unknown non-std exception");
+        }
+        response->set_other_error("unknown exception");

	// Convert literal value from timezone specified in cop request to UTC in-place.
	// Convert a literal value from the timezone specified in the cop request to UTC and return the converted value.

Conversation

wshwsh12 commented Apr 27, 2026

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

coderabbitai Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

ChangRui-Ryan commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

windtalker left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot Bot commented Apr 30, 2026

Uh oh!

ti-chi-bot Bot commented Apr 30, 2026

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot Bot commented Apr 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading