trash item清除存在问题

[skyoct]

Bug Report

Briefly describe the bug

对于trash item中的part在刚重启时候会迅速清除大部分数据，但是随着不断运行，到后面清除的很慢。这个是部分日志：

2024.06.20 14:49:54.251141 [ 940 ] {} <Trace> ods.app_10001_base_default (92d8ca50-ccee-44d6-b5bc-f51320ad793f)(PartGCThread): [p2] getDataItemsInTrash end with start_key: `cloud-byconity_GCTRASH_92d8ca50-ccee-4
4d6-b5bc-f51320ad793f_PT_20240620_450588707407003663_450588707407003663_0_450588675421241356`, items: 400
2024.06.20 14:49:54.251180 [ 940 ] {} <Trace> ods.app_10001_base_default (92d8ca50-ccee-44d6-b5bc-f51320ad793f)(PartGCThread): [p2] items after filtered: 0
2024.06.20 14:49:54.252584 [ 940 ] {} <Trace> ods.app_10001_base_default (92d8ca50-ccee-44d6-b5bc-f51320ad793f)(PartGCThread): [p2] Removed no data for 10 round(s). Delay schedule for 300000 ms.

根据日志可以看出来每次是可以从fdb拿出最大能拿出的条数，但是这些都被过滤掉了：https://github.com/ByConity/ByConity/blob/master/src/CloudServices/CnchPartGCThread.cpp#L500-L535 由于phase_two_start_key限制只能往后(只有从fdb加载数据条数小于limit条数时候才会phase_two_start_key设置为空，否则这里一直会往后)，会让可以被删除的部分无法被加载出来。

是否可以在执行对不可删除part过滤后如果条数小于limit条数把phase_two_start_key设置为空，让从头开始扫描。

The result you expected

How to Reproduce

Version

0.4.1

[smmsmm1988]

hi @fky2015, would you help check this issue?

[skyoct]

I tried modifying it, and it looks like it works.

[smmsmm1988]

I tried modifying it, and it looks like it works.

what did you modify

[fky2015]

是否可以在执行对不可删除part过滤后如果条数小于limit条数把phase_two_start_key设置为空，让从头开始扫描。

我明白你想做的事情。先说结论，想要临时解决是调大每次 GC 扫描的 batch size 或者 pool size (比如 gc_remove_part_batch_size)；如果想要彻底解决此类问题，你的方法还是会存在问题，需要另寻他法。

下面解释一下为什么 phase_two_start_key 要设计成这种行为： 实际上“不考虑过滤后的数量”是因为希望 GC Scan 是一种 Round-robin 的方法。假设每次 scan 400 个 key，并且采用了你提供的方法；另外假设前 400 个 key 都是不可删除的。那么：每次 scan 将无法回收任何存储，并且直到最开始的 400 个 key 变为可删除，GC 机制会完全失效。因此，采用 Round-robin 的方式是为了保证所有的 key 都有潜在被扫描到的机会。

回到你这个问题，我个人怀疑你的 fdb 中存在一段连续（比如 4k 个）的不可删除数据。这个时候如果 GC 频繁的不命中，就会降低之后 GC 调度的速度。（L465-L469）

降低调度速度的考虑是因为此类 IO 会有额外的性能成本以及远端存储的费用成本，所以增加了一个自适应逻辑。目前看来，由于目前的设计（即 GC 扫描的区域中同时存在可删除和不可删除的内容），没有办法找到一种不存在假阴性的算法。

目前我能想到代价最小的方式就是增加每次扫描的步长（也即 batch size * pool size），或者减小等待时间指数增加的指数，或者考虑全部扫完一轮以后，才触发（L465-L469）的机制（也就是更激进地回收）。另外，当有新的 KV 进入 Trash 的时候，应该通知 GC 线程重新进行更乐观的扫描。

[fky2015]

另外，不知道你是否有一个比较简单的复现方式？

[zeromem]

@fky2015 策略可以尝试优化成：参数 n 表示每轮删除 n 个 entry (可能涉及到多次 kv scan )，然后 startkey 检测到超过一轮，则 sleep 回退。

badcase 可能是发生在实时表当天分区，最近 3 小时内有大量 entry ，这时候 ttl 没过期所以无法删除，并且经过 10 轮还在这个分区，所以会回退

[skyoct]

@fky2015 从头开始确实不能覆盖所有场景：

1. 针对不使用表快照从头开始扫描性价比最高。
2. 针对 使用的有表快照前可能会出现卡死。

出现这个问题原因应该是我们单个表每30秒产生的part的超过400个（700多个），这样可能永远无法删除过期的数据。

[skyoct]

@fky2015 理论上batch_size 和 pool_size设置低一点，插入part并发高一点（每30秒插入数据量大于batch_size*2，我没详细计算）应该就可以复现。

[fky2015]

@zeromem

策略可以尝试优化成：参数 n 表示每轮删除 n 个 entry (可能涉及到多次 kv scan )，然后 startkey 检测到超过一轮，则 sleep 回退。

后面这个我理解；前半句“参数 n 表示每轮删除 n 个 entry”意思是要作为后半句的一个参数？（ startkey 检测超过 n 轮，sleep 回退）。

我个人感觉只要实现后半句的回退机制，就能缓解这个问题。

另一个想法：但是，当 KV 中不可删除的 Key 占比非常高的时候，Scan 的代价还是挺大的，大部分的 Scan 都是无用功。如果我们能额外引入一个按照时间排序的二级索引的话，就能更好的解决这个问题。

[smmsmm1988]

@zeromem

策略可以尝试优化成：参数 n 表示每轮删除 n 个 entry (可能涉及到多次 kv scan )，然后 startkey 检测到超过一轮，则 sleep 回退。

后面这个我理解；前半句“参数 n 表示每轮删除 n 个 entry”意思是要作为后半句的一个参数？（ startkey 检测超过 n 轮，sleep 回退）。

我个人感觉只要实现后半句的回退机制，就能缓解这个问题。

另一个想法：但是，当 KV 中不可删除的 Key 占比非常高的时候，Scan 的代价还是挺大的，大部分的 Scan 都是无用功。如果我们能额外引入一个按照时间排序的二级索引的话，就能更好的解决这个问题。

@fky2015 sleep回退机制是否要放宽松，不是10轮删除不到数据就回退，而是必须要从头到尾scan一遍所有key（可以采用当前的分批方式）后再考虑回退？

[fky2015]

@smmsmm1988 是，我上面的回复也是这个意思。

或者考虑全部扫完一轮以后，才触发（L465-L469）的机制（也就是更激进地回收）。另外，当有新的 KV 进入 Trash 的时候，应该通知 GC 线程重新进行更乐观的扫描。

[zeromem]

前半句“参数 n 表示每轮删除 n 个 entry”意思是要作为后半句的一个参数？

现在的策略 比如参数 n=400，我们每次从 kv 捞出来 400 个 entry，然后判断时候能删除，然后执行 gc。这一轮可能轮空（issue 里面反馈的 badcase）。我想表达的是，我们可以尝试改成：400 意味着当前这一轮会从 kv 里不断获取 entry，尽量保证能成功删掉 400 个 entry。 这个过程可能会发生：及时扫完一轮 key 也没达到 400 个。这时候就是后半句说的，key 走完一轮后，需要回退等待。

[nudles]

@fky2015 will there be a PR for this issue? may I know the conclusion? thanks!

[fky2015]

@nudles

@fky2015 will there be a PR for this issue? may I know the conclusion? thanks!

Certainly! After my vacation, which ends on July 1st, I will submit a PR.

In simple terms, the conclusion is that we should be more careful with the GC speed regression. It will keep scanning until there is no data left for the entire table.

[fky2015]

https://github.com/ByConity/ByConity/commit/4b9f1238ff60a70d5c8798e14d0c2d4df776b8d8 would address this issue to a large extent.

https://github.com/ByConity/ByConity/pull/1739 will further address this issue.

[nudles]

1739 cannot be merged into 0.4.x branch because some other commits are not there.

We plan to cherry-pick other commits into master and then release v1.0. Please wait for #1738