Fixes #9565

[gitbw95]

Summary:
Compaction::IsTrivialMove checks whether allow_trivial_move is set, and if so it returns the value of is_trivial_move_. The allow_trivial_move option is there for universal compaction. So when this is set and leveled compaction is enabled, then useful code that follows this block never gets a chance to run.

A check that compaction_style == kCompactionStyleUniversal should be added to avoid doing the wrong thing for leveled.

Test Plan:
To reproduce this:
First edit db/compaction/compaction.cc with

diff --git a/db/compaction/compaction.cc b/db/compaction/compaction.cc
index 7ae50b91e..52dd489b1 100644
--- a/db/compaction/compaction.cc
+++ b/db/compaction/compaction.cc
@@ -319,6 +319,8 @@ bool Compaction::IsTrivialMove() const {
   // input files are non overlapping
   if ((mutable_cf_options_.compaction_options_universal.allow_trivial_move) &&
       (output_level_ != 0)) {
+    printf("IsTrivialMove:: return %d because universal allow_trivial_move\n", (int) is_trivial_move_);
+    // abort();
     return is_trivial_move_;
   }

And then run

./db_bench --benchmarks=fillseq --allow_concurrent_memtable_write=false --level0_file_num_compaction_trigger=4 --level0_slowdown_writes_trigger=20 --level0_stop_writes_trigger=30 --max_background_jobs=8 --max_write_buffer_number=8 --db=/data/m/rx --wal_dir=/data/m/rx --num=800000000 --num_levels=8 --key_size=20 --value_size=400 --block_size=8192 --cache_size=51539607552 --cache_numshardbits=6 --compression_max_dict_bytes=0 --compression_ratio=0.5 --compression_type=lz4 --bytes_per_sync=8388608 --cache_index_and_filter_blocks=1 --cache_high_pri_pool_ratio=0.5 --benchmark_write_rate_limit=0 --write_buffer_size=16777216 --target_file_size_base=16777216 --max_bytes_for_level_base=67108864 --verify_checksum=1 --delete_obsolete_files_period_micros=62914560 --max_bytes_for_level_multiplier=8 --statistics=0 --stats_per_interval=1 --stats_interval_seconds=20 --histogram=1 --memtablerep=skip_list --bloom_bits=10 --open_files=-1 --subcompactions=1 --compaction_style=0 --min_level_to_compress=3 --level_compaction_dynamic_level_bytes=true --pin_l0_filter_and_index_blocks_in_cache=1 --soft_pending_compaction_bytes_limit=167503724544 --hard_pending_compaction_bytes_limit=335007449088 --min_level_to_compress=0 --use_existing_db=0 --sync=0 --threads=1 --memtablerep=vector --allow_concurrent_memtable_write=false --disable_wal=1 --seed=1641328309 --universal_allow_trivial_move=1 

Example output with the debug code added

IsTrivialMove:: return 0 because universal allow_trivial_move
IsTrivialMove:: return 0 because universal allow_trivial_move

After this PR, the bug is fixed.

[ajkr]

LGTM, thanks. See #9334 for an example of how to organize the info in the GitHub title/description. These fields form the commit message and it's preferable for the commit message to have minimal external references.

edit: One thing my example doesn't demonstrate but yours could use is a line that says "Fixes #<issue number>." so the related issue gets auto-closed upon this PR landing.

[gitbw95]

Thanks Andrew! The comments are addressed.

[facebook-github-bot]

@gitbw95 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[ajkr]

Thanks for addressing the comment. LGTM again!

[anand1976]

Can we add a unit test for this? Maybe something simple like mismatched compression type between start and output level, and compaction_options_universal.allow_trivial_move set to true. Earlier, it would have allowed the move, but should fail now.

[gitbw95]

I will add a new pr with the unit test.