The lock in BlobStore::removePosFromStats introducing performance jitter

[JaySon-Huang]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

2. What did you expect to see? (Required)

3. What did you see instead (Required)

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;
	}

4. What is your TiFlash version? (Required)

master