the invisible mvcc versions are not purged by gc

[shaoxiqian]

Bug Report

What version of TiKV are you using?

v5.4.0

What operating system and CPU are you using?

Steps to reproduce

This is a customer's workload which is only "read"

What did you expect?

TiKV will clear the invisible mvcc version automatically when there are a lot of rocksdb tombstone keys instead of waiting for compaction file GC

What did happened?

QPS is just 49.9

IO usage on TiKV is high, avg 571MB/s. CPU usage of TiDB/TiKV is low.

We can see the total_keys is almost equal to key_skipped_count. It means during the scan, coprocessor meets a lot of versions.

The iterator calls millions of next but only process tens of keys. It means there are too many invisible mvcc versions in the TiKV. From the flamegraph, the iterator is busy spinning on finding next valid key.

From TiKV metrics, that GC rarely happens in the collected time frame.

There is no modifications during the time frame, so no gc is executed

All above shows there are too many delete versions that have not been compacted into tombstones which affected the performance.

[hicqu]

@shaoxiqian thanks for your report! In the current implementation, MVCC-deletions can only be handled at the bottlemost level. #10545 may be helpful for this case, we will continue on it.

[dbsid]

/severity critical

[dbsid]

I think this is a critical bug for the default gc. It’s devastating that the old mvcc versions are not purged from the user perspective.

[tonyxuqqi]

@hicqu would you please move #10545 forward?

[dbsid]

/severity major

[Lily2025]

/remove-severity critical

[shaoxiqian]

/remove-severity critical

[ti-chi-bot]

@shaoxiqian: These labels are not set on the issue: severity/critical.

In response to this:

/remove-severity critical

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 ti-community-infra/tichi repository.

[tonyxuqqi]

/cc jiayang-zheng

[BusyJay]

I think an easy fix is to change the function

tikv/components/raftstore/src/store/worker/compact.rs

Lines 177 to 192 in dcb5e2c

	fn need_compact(
	num_entires: u64,
	num_versions: u64,
	tombstones_num_threshold: u64,
	tombstones_percent_threshold: u64,
	) -> bool {
	if num_entires <= num_versions {
	return false;
	}
	// When the number of tombstones exceed threshold and ratio, this range need
	// compacting.
	let estimate_num_del = num_entires - num_versions;
	estimate_num_del >= tombstones_num_threshold
	&& estimate_num_del * 100 >= tombstones_percent_threshold * num_entires
	}

If there are too many versions, compaction should be triggered for write cf, otherwise stale versions won't be deleted.

[cfzjywxk]

Maybe it's needed to consider more about the compaction triggering strategy, before that we could abstract a simpler case to help investigate this issue.
/cc @you06

[tonyxuqqi]

/assign jiayang-zheng

[ti-chi-bot]

@tonyxuqqi: GitHub didn't allow me to assign the following users: jiayang-zheng.

Note that only tikv members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign jiayang-zheng

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/test-infra repository.

[cfzjywxk]

@tonyxuqqi
@you06 is already working on this and doing related tests.

[afeinberg]

another PR to address this is merged: #15995

[tonyxuqqi]

Since we already have the enhanced check_compact as well as the periodic full compaction. I think the issue can be closed for now.