*: Make tiflash compatible with the cse branch #9765

JaySon-Huang · 2025-01-06T11:19:10Z

What problem does this PR solve?

Issue Number: close #9767

Problem Summary:

What is changed and how it works?

The following adoption of cse only enable when SERVERLESS_PROXY = 1 is defined

Length of short value in lock CF encode as varInt instead of u8
Blocklist feature
- keyspace blocklist
- region_id blocklist
- store_id blocklist under disagg arch
mtls
A HTTP API for Acquiring the all the region ids created in this TiFlash node with given keyspace id
File-base large txn
Special threshold on parallel prehandle

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

None

Signed-off-by: JaySon-Huang <[email protected]>

yongman

LGTM

ti-chi-bot · 2025-01-08T12:31:45Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yongman
Once this PR has been reviewed and has the lgtm label, please ask for approval from jayson-huang, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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-01-08T12:31:46Z

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

In response to this:

LGTM

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.

dbms/src/Storages/KVStore/MultiRaft/RegionPersister.cpp

dbms/src/Server/Server.cpp

dbms/src/Interpreters/loadMetadata.cpp

CalvinNeo · 2025-01-14T03:50:51Z

dbms/src/Server/MetricsPrometheus.cpp

+        Poco::Net::Context::VerificationMode::VERIFY_STRICT
+#else
+        // mtls: metrics server allows anonymous pullers @iosmanthus
+        Poco::Net::Context::VerificationMode::VERIFY_RELAXED


Where does this line come from? Can not find on either tiflash master or cse

it has been merge months ago https://github.com/tidbcloud/tiflash-cse/pull/216

dbms/src/Server/Server.cpp

CalvinNeo · 2025-01-14T03:52:40Z

dbms/src/Storages/KVStore/FFI/ProxyFFI.cpp

-                "Failed to get lock by key {}, region_id={}",
-                tikv_key.toDebugString(),
-                region_id);
+            LOG_DEBUG(Logger::get(), "Failed to get lock by key {}, region_id={}", tikv_key.toDebugString(), region_id);


Why change this to DEBUG?

it has been changed to debug level in the cse branch because there could be lots of logging after file base large txn

CalvinNeo · 2025-01-14T03:56:35Z

dbms/src/TiDB/Schema/TiDBSchemaSyncer.cpp

        if (version <= cur_version)
        {
            return false;
        }
+#else
+        if (version == cur_version)


// if the `version` is less than `cur_version`, it means that the schema version in TiKV has been rolled back by restore. // We should sync the schema again.

Why won't this happen in TiFlash master?

CalvinNeo · 2025-01-14T04:00:37Z

/cc @guo-shaoge Could you please also review the blocklist part?

Co-authored-by: Calvin Neo <[email protected]>

JaySon-Huang · 2025-01-07T06:27:38Z

dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp

+{
+#if SERVERLESS_PROXY == 0
+    return getMaxPrehandleSubtaskSize();
+#else
+    auto max_subtask_size = getMaxPrehandleSubtaskSize();
+    // In serverless mode, IO takes more part in decoding stage, so we can increase parallel limit.
+    // In real test, the prehandling speed decreases if we use higher concurrency.
+    return std::min(4, total_concurrency);
+#endif
+}


https://github.com/tidbcloud/tiflash-cse/pull/289

JaySon-Huang · 2025-01-14T08:17:26Z

dbms/src/Storages/KVStore/FFI/ProxyFFI.cpp

-                "Failed to get lock by key {}, region_id={}",
-                tikv_key.toDebugString(),
-                region_id);
+            LOG_DEBUG(Logger::get(), "Failed to get lock by key {}, region_id={}", tikv_key.toDebugString(), region_id);


it has been changed to debug level in the cse branch because there could be lots of logging after file base large txn

JaySon-Huang · 2025-01-14T08:21:38Z

dbms/src/Server/MetricsPrometheus.cpp

+        Poco::Net::Context::VerificationMode::VERIFY_STRICT
+#else
+        // mtls: metrics server allows anonymous pullers @iosmanthus
+        Poco::Net::Context::VerificationMode::VERIFY_RELAXED


it has been merge months ago https://github.com/tidbcloud/tiflash-cse/pull/216

ti-chi-bot · 2025-01-14T08:59:29Z

@JaySon-Huang: 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-unit-test	`3964824`	link	true	`/test pull-unit-test`
pull-integration-test	`3964824`	link	true	`/test pull-integration-test`

Full PR test history. Your PR dashboard.

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.

JaySon-Huang added 9 commits January 7, 2025 12:35

Align the diff of short value in lock cf under cse

5632a14

Signed-off-by: JaySon-Huang <[email protected]>

Add ut

feda366

Signed-off-by: JaySon-Huang <[email protected]>

Add blocklist feature

254e2fb

Signed-off-by: JaySon-Huang <[email protected]>

fix blacklist-file args to proxy

7c46643

Signed-off-by: JaySon-Huang <[email protected]>

Add http api for cse

2db71b8

Signed-off-by: JaySon-Huang <[email protected]>

mtls: metrics server allows anonymous pullers

4195065

Signed-off-by: JaySon-Huang <[email protected]>

Enlarge the max parall prehandle size

4fff97b

Signed-off-by: JaySon-Huang <[email protected]>

txn-file: support txn file feature with cse

78104f1

Signed-off-by: JaySon-Huang <[email protected]>

Fix parallel prehandle limit

92b36b1

Signed-off-by: JaySon-Huang <[email protected]>

JaySon-Huang force-pushed the align_with_cse branch from 9acc7d5 to 2dbf453 Compare January 7, 2025 04:57

JaySon-Huang added 2 commits January 7, 2025 15:40

Fix lint

e019cfa

Signed-off-by: JaySon-Huang <[email protected]>

Fix compile error

236d14f

Signed-off-by: JaySon-Huang <[email protected]>

JaySon-Huang force-pushed the align_with_cse branch from 2dbf453 to 236d14f Compare January 7, 2025 07:40

Remove unstable settings

9d18047

Signed-off-by: JaySon-Huang <[email protected]>

JaySon-Huang changed the title ~~[WIP] *: Make tiflash compatible with the cse branch~~ *: Make tiflash compatible with the cse branch Jan 8, 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 Jan 8, 2025

JaySon-Huang requested review from CalvinNeo and yongman January 8, 2025 09:55

yongman approved these changes Jan 8, 2025

View reviewed changes

CalvinNeo reviewed Jan 10, 2025

View reviewed changes

dbms/src/Storages/KVStore/MultiRaft/RegionPersister.cpp Outdated Show resolved Hide resolved

CalvinNeo reviewed Jan 10, 2025

View reviewed changes

dbms/src/Server/Server.cpp Outdated Show resolved Hide resolved

CalvinNeo reviewed Jan 10, 2025

View reviewed changes

dbms/src/Server/Server.cpp Outdated Show resolved Hide resolved

CalvinNeo reviewed Jan 14, 2025

View reviewed changes

dbms/src/Interpreters/loadMetadata.cpp Outdated Show resolved Hide resolved

CalvinNeo reviewed Jan 14, 2025

View reviewed changes

dbms/src/Interpreters/loadMetadata.cpp Outdated Show resolved Hide resolved

CalvinNeo reviewed Jan 14, 2025

View reviewed changes

dbms/src/Server/Server.cpp Outdated Show resolved Hide resolved

CalvinNeo reviewed Jan 14, 2025

View reviewed changes

Apply suggestions from code review

3964824

Co-authored-by: Calvin Neo <[email protected]>

JaySon-Huang commented Jan 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: Make tiflash compatible with the cse branch #9765

*: Make tiflash compatible with the cse branch #9765

JaySon-Huang commented Jan 6, 2025 •

edited

Loading

yongman left a comment

ti-chi-bot bot commented Jan 8, 2025

ti-chi-bot bot commented Jan 8, 2025

CalvinNeo Jan 14, 2025

JaySon-Huang Jan 14, 2025

CalvinNeo Jan 14, 2025

JaySon-Huang Jan 14, 2025

CalvinNeo Jan 14, 2025 •

edited

Loading

CalvinNeo commented Jan 14, 2025

JaySon-Huang Jan 7, 2025

JaySon-Huang Jan 14, 2025

JaySon-Huang Jan 14, 2025

ti-chi-bot bot commented Jan 14, 2025

*: Make tiflash compatible with the cse branch #9765

Are you sure you want to change the base?

*: Make tiflash compatible with the cse branch #9765

Conversation

JaySon-Huang commented Jan 6, 2025 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

yongman left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 8, 2025

ti-chi-bot bot commented Jan 8, 2025

CalvinNeo Jan 14, 2025

Choose a reason for hiding this comment

JaySon-Huang Jan 14, 2025

Choose a reason for hiding this comment

CalvinNeo Jan 14, 2025

Choose a reason for hiding this comment

JaySon-Huang Jan 14, 2025

Choose a reason for hiding this comment

CalvinNeo Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

CalvinNeo commented Jan 14, 2025

JaySon-Huang Jan 7, 2025

Choose a reason for hiding this comment

JaySon-Huang Jan 14, 2025

Choose a reason for hiding this comment

JaySon-Huang Jan 14, 2025

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 14, 2025

JaySon-Huang commented Jan 6, 2025 •

edited

Loading

CalvinNeo Jan 14, 2025 •

edited

Loading