[Tiered Storage] Replace penultimate naming with proximal #13460
Conversation
|
@jaykorean has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jaykorean has updated the pull request. You must reimport the pull request before landing. |
|
@jaykorean has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jaykorean has updated the pull request. You must reimport the pull request before landing. |
| input_file.level = 0; | ||
| input_file.index = i; | ||
| smallest_key_priority_q.push(std::move(input_file)); | ||
| smallest_key_priority_q.emplace(c->input(0, i), 0, i); |
There was a problem hiding this comment.
Meta internal linter was complaining about the use-after-move. Please let me know if I should do this in a separate PR.
jaykorean
force-pushed
the
rename_penultimate_to_proximal
branch
from
13a2afc to
93328ff
Compare
|
@jaykorean has updated the pull request. You must reimport the pull request before landing. |
|
@jaykorean has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
jaykorean
force-pushed
the
rename_penultimate_to_proximal
branch
from
93328ff to
00250e1
Compare
|
@jaykorean has updated the pull request. You must reimport the pull request before landing. |
|
@jaykorean has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jaykorean has updated the pull request. You must reimport the pull request before landing. |
|
@jaykorean has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| input_file.level = 0; | ||
| input_file.index = i; | ||
| smallest_key_priority_q.push(std::move(input_file)); | ||
| smallest_key_priority_q.emplace(c->input(0, i), 0, i); |
| @@ -215,8 +215,8 @@ TEST_F(TieredCompactionTest, SequenceBasedTieredStorageUniversal) { | |||
| } | |||
| ASSERT_OK(dbfull()->TEST_WaitForCompact()); | |||
|
|
|||
| // the penultimate level file temperature is not cold, all data are output to | |||
| // the penultimate level. | |||
| // the proximal level file temperature is not cold, all data are output to | |||
There was a problem hiding this comment.
In my mind, "proximal" makes sense relative to the primary/normal output level of a compaction: it's the level before it. It doesn't really make sense in absolute terms. The penultimate level is the proximal level to the last level. I think "penultimate" still makes sense for cases in this file that are referring specifically to the next-to-last level, instead of to compaction things outside this file.
Not a big deal
|
@jaykorean merged this pull request in ca7367a. |
Summary
With generalized age-based tiering (work-in-progress), the "warm tier" data will no longer necessarily be placed in the second-to-last level (also known as the "penultimate level").
Also, the cold tier may no longer necessarily be at the last level, so we need to rename options like
preclude_last_level_secondstopreclude_cold_tier_seconds, but renaming options is trickier because it can be a breaking change for consuming applications. We will do this later as a follow up.Minor fix included: Fixed one
use-after-movein CompactionPickerTest Plan
CI