Skip to content

Commit

Permalink
fix(pageserver): ensure sparse keyspace is ordered (#8285)
Browse files Browse the repository at this point in the history
## Problem

Sparse keyspaces were constructed with ranges out of order: this didn't break things obviously, but meant that users of KeySpace functions that assume ordering would assert out.

Closes #8277

## Summary of changes

make sure the sparse keyspace has ordered keyspace parts
  • Loading branch information
skyzh committed Jul 8, 2024
1 parent 27fe7f8 commit 154ba5e
Showing 1 changed file with 45 additions and 7 deletions.
52 changes: 45 additions & 7 deletions pageserver/src/pgdatadir_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,9 @@ impl Timeline {
result.add_key(AUX_FILES_KEY);
}

// Add extra keyspaces in the test cases. Some test cases write keys into the storage without
// creating directory keys. These test cases will add such keyspaces into `extra_test_dense_keyspace`
// and the keys will not be garbage-colllected.
#[cfg(test)]
{
let guard = self.extra_test_dense_keyspace.load();
Expand All @@ -927,13 +930,48 @@ impl Timeline {
}
}

Ok((
result.to_keyspace(),
/* AUX sparse key space */
SparseKeySpace(KeySpace {
ranges: vec![repl_origin_key_range(), Key::metadata_aux_key_range()],
}),
))
let dense_keyspace = result.to_keyspace();
let sparse_keyspace = SparseKeySpace(KeySpace {
ranges: vec![Key::metadata_aux_key_range(), repl_origin_key_range()],
});

if cfg!(debug_assertions) {
// Verify if the sparse keyspaces are ordered and non-overlapping.

// We do not use KeySpaceAccum for sparse_keyspace because we want to ensure each
// category of sparse keys are split into their own image/delta files. If there
// are overlapping keyspaces, they will be automatically merged by keyspace accum,
// and we want the developer to keep the keyspaces separated.

let ranges = &sparse_keyspace.0.ranges;

// TODO: use a single overlaps_with across the codebase
fn overlaps_with<T: Ord>(a: &Range<T>, b: &Range<T>) -> bool {
!(a.end <= b.start || b.end <= a.start)
}
for i in 0..ranges.len() {
for j in 0..i {
if overlaps_with(&ranges[i], &ranges[j]) {
panic!(
"overlapping sparse keyspace: {}..{} and {}..{}",
ranges[i].start, ranges[i].end, ranges[j].start, ranges[j].end
);
}
}
}
for i in 1..ranges.len() {
assert!(
ranges[i - 1].end <= ranges[i].start,
"unordered sparse keyspace: {}..{} and {}..{}",
ranges[i - 1].start,
ranges[i - 1].end,
ranges[i].start,
ranges[i].end
);
}
}

Ok((dense_keyspace, sparse_keyspace))
}

/// Get cached size of relation if it not updated after specified LSN
Expand Down

1 comment on commit 154ba5e

@github-actions
Copy link

Choose a reason for hiding this comment

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

3129 tests run: 3000 passed, 2 failed, 127 skipped (full report)


Failures on Postgres 16

  • test_lsn_lease_size[False]: debug

Failures on Postgres 14

  • test_lsn_lease_size[False]: debug
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_lsn_lease_size[debug-pg14-False] or test_lsn_lease_size[debug-pg16-False]"

Test coverage report is not available

The comment gets automatically updated with the latest test results
154ba5e at 2024-07-08T14:35:29.021Z :recycle:

Please sign in to comment.