PageStorage: Optimize the critical region when removing a BlobFile from disk (#8651)

[ti-chi-bot]

This is an automated cherry-pick of #8651

What problem does this PR solve?

Issue Number: close #8650

Problem Summary:

When all page data in a BlobFile gets removed in the GC thread, removePosFromStats will remove the BlobFile from disk. However, it holds a lock on mtx_blob_files when removing the file from disk. If BlobStore::write happens to write a page data to a new BlobFile, BlobStore::getBlobFile will be blocked by the GC thread.

According to local tests, removing a BlobFile takes tens of milliseconds. Thus introducing performance jitter.

tiflash/dbms/src/Storages/Page/V3/BlobStore.cpp

Lines 803 to 814 in 63eb295

	{
	// Remove the blob file from disk and memory
	std::lock_guard files_gurad(mtx_blob_files);
	if (auto iter = blob_files.find(blob_id); iter != blob_files.end())
	{
	auto blob_file = iter->second;
	blob_file->remove();
	blob_files.erase(iter);
	}
	// If the blob_id does not exist, the blob_file is never
	// opened for read/write. It is safe to ignore it.
	}

tiflash/dbms/src/Storages/Page/V3/BlobStore.cpp

Lines 632 to 636 in 63eb295

	SCOPE_EXIT({
	GET_METRIC(tiflash_storage_page_write_duration_seconds, type_blob_write).Observe(watch.elapsedSeconds());
	});
	auto blob_file = getBlobFile(blob_id);
	blob_file->write(buffer, offset_in_file, all_page_data_size, write_limiter);

tiflash/dbms/src/Storages/Page/V3/BlobStore.cpp

Lines 1494 to 1503 in 63eb295

	template <typename Trait>
	BlobFilePtr BlobStore<Trait>::getBlobFile(BlobFileId blob_id)
	{
	std::lock_guard files_gurad(mtx_blob_files);
	if (auto iter = blob_files.find(blob_id); iter != blob_files.end())
	return iter->second;
	auto file = std::make_shared<BlobFile>(getBlobFileParentPath(blob_id), blob_id, file_provider, delegator);
	blob_files.emplace(blob_id, file);
	return file;
	}

What is changed and how it works?

Keep a pointer within the lock on mtx_blob_files, and remove the blob file from disk without the lock

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

Reduce the effect of background GC tasks on foreground read/write tasks

[JaySon-Huang]

/run-all-tests

[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-01-03 10:24:32.189767445 +0000 UTC m=+2252563.226994371: ☑️ agreed by JinheLin.
• 2024-01-03 10:24:50.93802993 +0000 UTC m=+2252581.975256842: ☑️ agreed by Lloyd-Pottiger.