Skip to content

Commit

Permalink
Tiered compaction: improvements to the windows (#7787)
Browse files Browse the repository at this point in the history
Tiered compaction employs two sliding windows over the keyspace:
`KeyspaceWindow` for the image layer generation and `Window` for the
delta layer generation. Do some fixes to both windows:

* The distinction between the two windows is not very clear. Do the
absolute minimum to mention where they are used in the rustdoc
description of the struct. Maybe we should rename them (say
`WindowForImage` and `WindowForDelta`) or merge them into one window
implementation.
* Require the keys to strictly increase. The `accum_key_values` already
combines the key, so there is no logic needed in `Window::feed` for the
same key repeating. This is a follow-up to address the request in
#7671 (review)
* In `choose_next_delta`, we claimed in the comment to use 1.25 as the
factor but it was 1.66 instead. Fix this discrepancy by using `*5/4` as
the two operations.
  • Loading branch information
arpad-m authored May 16, 2024
1 parent 4c5afb7 commit 4b8809b
Showing 1 changed file with 13 additions and 11 deletions.
24 changes: 13 additions & 11 deletions pageserver/compaction/src/compact_tiered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,8 @@ where
}
}

// Sliding window through keyspace and values
// This is used by over_with_images to decide on good split points
/// Sliding window through keyspace and values for image layer
/// This is used by [`LevelCompactionState::cover_with_images`] to decide on good split points
struct KeyspaceWindow<K> {
head: KeyspaceWindowHead<K>,

Expand Down Expand Up @@ -804,9 +804,9 @@ struct WindowElement<K> {
accum_size: u64,
}

// Sliding window through keyspace and values
//
// This is used to decide what layer to write next, from the beginning of the window.
/// Sliding window through keyspace and values for delta layer tiling
///
/// This is used to decide which delta layer to write next.
struct Window<K> {
elems: VecDeque<WindowElement<K>>,

Expand All @@ -830,11 +830,13 @@ where
fn feed(&mut self, key: K, size: u64) {
let last_size;
if let Some(last) = self.elems.back_mut() {
assert!(last.last_key <= key);
if key == last.last_key {
last.accum_size += size;
return;
}
// We require the keys to be strictly increasing for the window.
// Keys should already have been deduplicated by `accum_key_values`
assert!(
last.last_key < key,
"last_key(={}) >= key(={key})",
last.last_key
);
last_size = last.accum_size;
} else {
last_size = 0;
Expand Down Expand Up @@ -922,7 +924,7 @@ where
// If we're willing to stretch it up to 1.25 target size, could we
// gobble up the rest of the work? This avoids creating very small
// "tail" layers at the end of the keyspace
if !has_more && self.remain_size() < target_size * 5 / 3 {
if !has_more && self.remain_size() < target_size * 5 / 4 {
self.commit_upto(self.elems.len());
} else {
let delta_split_at = self.find_size_split(target_size);
Expand Down

1 comment on commit 4b8809b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3153 tests run: 3014 passed, 0 failed, 139 skipped (full report)


Code coverage* (full report)

  • functions: 31.4% (6349 of 20190 functions)
  • lines: 47.4% (47939 of 101054 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
4b8809b at 2024-05-16T21:43:03.062Z :recycle:

Please sign in to comment.