ddl: Fix adding vector index on empty table may blocked forever

[JaySon-Huang]

What problem does this PR solve?

Issue Number: ref #9032

Problem Summary:

When creating vector index on empty table, the DDL may be blocked forever.

This is because when an empty table is synced to TiFlash, the StorageDeltaMerge is created without initializing the DeltaMergeStore instance. This is to avoid generating too many fragmented, small files that exhaust the filesystem inode.
However, if the DeltaMergeStore instance is not created, then system.dt_local_index will not get the local index stats. Which make tidb consider the vector index is still under building and the ddl does not end.

tiflash/dbms/src/Storages/System/StorageSystemDTLocalIndexes.cpp

Lines 88 to 94 in 1a781c7

	auto dm_storage = std::dynamic_pointer_cast<StorageDeltaMerge>(storage);
	const auto & table_info = dm_storage->getTableInfo();
	auto table_id = table_info.id;
	auto store = dm_storage->getStoreIfInited();
	if (!store)
	continue;

What is changed and how it works?

• If StorageDeltaMerge is created but DeltaMergeStore is not inited, then generate the local index info by the TiDB::TableInfo

Small refactor:

• Remove a verbose logging when system table is read by tidb-server

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

[JaySon-Huang]

Running queries on the system table will trigger this logging, which is verbose but not provide any information. Remove it.

[Lloyd-Pottiger]

Why not just insert a row even when store is not init?

[breezewish]

Not sure whether this idea is ok, we may also regard progress=1 and missing table in tiflash_indexes as index build is finished, because progress=1 already means TiFlash is aware of this table, and then no entry in tiflash_indexes means exactly this case?

[JaySon-Huang]

Not sure whether this idea is ok, we may also regard progress=1 and missing table in tiflash_indexes as index build is finished, because progress=1 already means TiFlash is aware of this table, and then no entry in tiflash_indexes means exactly this case?

@breezewish No, we can not tell the difference between these two cases

• A table without any vector index but the tiflash replica is already built
• A table with the tiflash replica is already built, tidb added a vector index definition to the table, but tiflash is not aware that the vector index has been added

[JaySon-Huang]

@Lloyd-Pottiger DeltaMergeStore::getLocalIndexStats requires a DeltaMergeStore instance to get the stats of local indexes. Without that result, we are hard to tell wihch index_id need to be added to dt_local_index

tiflash/dbms/src/Storages/System/StorageSystemDTLocalIndexes.cpp

Lines 91 to 98 in 1a781c7

	auto store = dm_storage->getStoreIfInited();
	if (!store)
	continue;
	if (dm_storage->isTombstone())
	continue;
	auto index_stats = store->getLocalIndexStats();

[Lloyd-Pottiger]

@Lloyd-Pottiger DeltaMergeStore::getLocalIndexStats requires a DeltaMergeStore instance to get the stats of local indexes. Without that result, we are hard to tell wihch index_id need to be added to dt_local_index

tiflash/dbms/src/Storages/System/StorageSystemDTLocalIndexes.cpp

Lines 91 to 98 in 1a781c7

	auto store = dm_storage->getStoreIfInited();
	if (!store)
	continue;
	if (dm_storage->isTombstone())
	continue;
	auto index_stats = store->getLocalIndexStats();

The index info can get from table info, and the rows indexed is 0.

[wuhuizuo]

/retest

[JaySon-Huang]

@Lloyd-Pottiger @JinheLin PTAL

[Lloyd-Pottiger]

lgtm

[Lloyd-Pottiger]

Need to update description.

[JaySon-Huang]

Need to update description.

Updated

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [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]

[LGTM Timeline notifier]

Timeline:

• 2024-09-27 12:06:40.027206939 +0000 UTC m=+10955.447419944: ☑️ agreed by Lloyd-Pottiger.
• 2024-09-29 03:42:26.686555973 +0000 UTC m=+153502.106768987: ☑️ agreed by JinheLin.