refactor(compact): make get_compact_task easy to read

[Little-Wallace]

What's changed and what's your intention?

close #2121
This is also part of leveled-compaction

Checklist

• I have written necessary docs and comments
• I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

[soundOfDestiny]

will come back immediately

[soundOfDestiny]

Does we support parallel L0 to L1 here?

[soundOfDestiny]

@Little-Wallace tells me that these code are expected to support parallel.
but if we have [7,8) [7,8) [0,1) [5,6) [9,10) in epoch order in L0 and first [7,8) is now being compacted, not only the second [7,8], but other SSTs cannot be selected to compact as well.

[codecov]

Codecov Report

Merging #2122 (eb9c925) into main (ade4fea) will increase coverage by 0.03%.
The diff coverage is 91.82%.

@@            Coverage Diff             @@
##             main    #2122      +/-   ##
==========================================
+ Coverage   70.90%   70.94%   +0.03%     
==========================================
  Files         652      654       +2     
  Lines       82790    82858      +68     
==========================================
+ Hits        58704    58785      +81     
+ Misses      24086    24073      -13     

Flag	Coverage Δ
rust	70.94% <91.82%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/meta/src/hummock/mod.rs	31.13% <0.00%> (+2.18%)	⬆️
src/meta/src/hummock/vacuum.rs	84.23% <ø> (ø)
src/storage/hummock_sdk/src/compact.rs	100.00% <ø> (ø)
src/meta/src/hummock/hummock_manager.rs	91.56% <70.58%> (-0.07%)	⬇️
src/meta/src/hummock/compaction/mod.rs	83.53% <83.53%> (ø)
src/meta/src/hummock/level_handler.rs	73.80% <91.37%> (-26.20%)	⬇️
...a/src/hummock/compaction/tier_compaction_picker.rs	97.84% <97.84%> (ø)
...rc/meta/src/hummock/compaction/overlap_strategy.rs	100.00% <100.00%> (ø)
src/meta/src/hummock/metrics_utils.rs	96.24% <100.00%> (-0.47%)	⬇️
src/meta/src/hummock/test_utils.rs	95.41% <100.00%> (+0.03%)	⬆️
... and 10 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[soundOfDestiny]

@Little-Wallace tells me that these code are expected to support parallel.
but if we have [7,8) [7,8) [0,1) [5,6) [9,10) in epoch order in L0 and first [7,8) is now being compacted, not only the second [7,8], but other SSTs cannot be selected to compact as well.

and it cannot be simply postponed to next PR, because if we simply enable this situation in code of this PR, it will cause overlapping SSTs generated in L1. Therefore I guess code here perhaps needs to be refactored (for example, key range shall be retained to maintain this situation) to implement points above.

[Little-Wallace]

Before we support vertical group, the file compact from shared buffer is very likely to overlap with other files.
After we support vertical group, we will split files with different prefix to different group, such sene is not need.

[soundOfDestiny]

but I worry that we base our LSM on an assumption that 'each L0 SST is in same key range'

[soundOfDestiny]

Actually, if we make the assumption that 'each L0 SST is in same key range', code can be much more cleaner because all we need is just compaction. We do not need to care key ranges of SST any more.
and if not, codes here haven't supported very well.

[Little-Wallace]

If they are not in the same key range, we can also compact them in parallel.
The only thing that is not allowed is that there are three sst files: sst1, sst2, sst3. When the sst1 is doing compaction job, we can not start a compact job include sst3 but exclude sst2.

[soundOfDestiny]

If they are not in the same key range, we can also compact them in parallel.
The only thing that is not allowed is that there are three sst files: sst1, sst2, sst3. When the sst1 is doing compaction job, we can not start a compact job include sst3 but exclude sst2.

but if sst2 is from L0 intra compaction, this situation is very likely to happen then

[soundOfDestiny]

If current code is not OOP enough, I can refactor them. However, I personally don't want to drop some existing feature without reasonable consideration because I guess those features will not impact code tidy.

[soundOfDestiny]

In addition, if we are used to depending on L0 intra compaction, it will make each L0 SST have a very large key range, which will involve very many L1 SSTs while being compacted to L1. From my perspective, I think we should reduce L0 intra compaction to avoid this situation.

[soundOfDestiny]

Certainly, if there are too many L0 SSTs and L1 is in congestion, we have to do L0 intra compaction. However, if L1 is not in congestion and amount of SSTs in L0 is acceptable, I prefer compacting them to L1 respectively to doing L0 intra compaction first.
Note that compacting them to L1 respectively would not cause extra write amplification because with the condition that SSTs in L1 are non-overlapped, we can compact L0 SSTs with corresponding 1~2 L1 SSTs each.

[soundOfDestiny]

Suppose that there are 3 SSTs in L0 in this order:
[0, 1) in compaction, [0, 10) not in compaction, [6, 7) not in compaction
current code will collect [6, 7) as next compaction, which is incorrect

[zwang28]

LGTM

[github-actions]

license-eye has totally checked 759 files.

Valid	Invalid	Ignored	Fixed
756	1	2	0

Click to see the invalid file list

• src/meta/src/hummock/compaction/overlap_strategy.rs