Fix user comparator receiving internal key (#4575)

[ajkr]

Summary:
There was a bug that the user comparator would receive the internal key instead of the user key. The bug was due to RangeMightExistAfterSortedRun expecting user key but receiving internal key when called in GenerateBottommostFiles. The patch augment an existing unit test to reproduce the bug and fixes it.
Pull Request resolved: facebook#4575

Differential Revision: D10500434

Pulled By: maysamyabandeh

fbshipit-source-id: 858346d2fd102cce9e20516d77338c112bdfe366

---

This change is 

[petermattis]

What is the effect of this bug? Seems like it could cause a comparator to crash if it expected a key to match a particular format. It also might incorrectly populate bottommost_files_, but I'm not familiar with what that variable is used for.

[ajkr]

The incorrect population of bottommost_files_ is potentially problematic. The compaction picker uses it when deciding which files to mark for compaction. In particular we want to recompact a bottommost SST when it has key versions that can be eliminated, maybe because they were written to that file for a snapshot that no longer exists. This feature prevents old key versions from getting stuck in the bottom level forever, which caused high space-amp.

I have some unproven fear that an infinite loop is possible if bottommost_files_ is wrong and we put non-bottommost files through this type of compaction. For a non-bottommost file, we're not guaranteed to drop unnecessary key versions (e.g., tombstones covering nothing may be preserved), so the output file may look the same as the input file. Then the conditions for triggering compaction on that new file would be met, so it'd continue forever.