fix: Avoid leveled compaction crash when recovering from manifest

[YangchenYe323]

Fixes #62

• Add a test case to capture the error
  Below is what the test failure looks like on old version of the code:

--- STDERR:              mini-lsm tests::week2_day5::test_multiple_compacted_ssts_leveled ---
thread 'tests::week2_day5::test_multiple_compacted_ssts_leveled' panicked at mini-lsm/src/compact/leveled.rs:223:18:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

• Fix the error by moving the sorting logic outside of apply_compaction_result into trigger_compaction

[skyzh]

Thanks for the pull request and the fix! However, I really want the apply_compaction_result function keeps everything sorted, so that if we use that function, we always get a consistent state. I would rather opt in a design that apply_compaction_result takes a parameter in_recovery that controls whether to sort or not.

[skyzh]

Or we can include the start/end key in the manifest, which is what most storage engines are doing.

[YangchenYe323]

@skyzh Which option do you prefer? I'm happy to implement either. Looks to me that adding a is_recovery flag is an easy change, whereas there are extra works in order to go the second route, e.g., you need to pass something like an Option<HashMap<usize, (KeyByte, KeyByte)>> to apply_compaction_result for it to use at recovery time. Does it align with your thinking? Personally I think adding a simple flag is fine. You don't need the leveled SSTs to be sorted at all times during recovery after all.

[skyzh]

Sounds good, let's have a recovery flag :) Take your time if you want to work on that, and feel free to ping me when you need review or help.

[YangchenYe323]

@skyzh Would you like to take another look? Thanks!

[skyzh]

Sorry it took a while and I'll probably take a look this weekend. This involves API changes and I might need to revisit the tutorial to ensure everything is consistent :)

[skyzh]

Thanks for the fix!