refactor: get rid of ApproxAccurate

This commit is contained in:
Joonas Koivunen
2023-03-28 11:26:45 +03:00
committed by Joonas Koivunen
parent 0943dd30eb
commit 17b5c8d1c4
4 changed files with 31 additions and 126 deletions

View File

@@ -1,100 +0,0 @@
/// Three-state `max` accumulator.
///
/// If it accumulates over 0 or many `Some(T)` values, it is `Accurate` maximum of those values.
/// If a single `None` value is merged, it becomes `Approximate` variant.
///
/// Remove when `Layer::file_size` is no longer an `Option`.
#[derive(Default, Debug, Clone, Copy)]
pub enum ApproxAccurate<T> {
Approximate(T),
Accurate(T),
#[default]
Empty,
}
impl<T: Ord + Copy + Default> ApproxAccurate<T> {
/// `max(a, b)` where the approximate is inflicted receiving a `None`, or infected onwards.
#[must_use]
pub fn max(self, next: Option<T>) -> ApproxAccurate<T> {
use ApproxAccurate::*;
match (self, next) {
(Accurate(a) | Approximate(a), None) => Approximate(a),
(Empty, None) => Approximate(T::default()),
(Accurate(a), Some(b)) => Accurate(a.max(b)),
(Approximate(a), Some(b)) => Approximate(a.max(b)),
(Empty, Some(b)) => Accurate(b),
}
}
pub fn is_approximate(&self) -> bool {
matches!(self, ApproxAccurate::Approximate(_))
}
pub fn accurate(self) -> Option<T> {
use ApproxAccurate::*;
match self {
Accurate(a) => Some(a),
Empty => Some(T::default()),
Approximate(_) => None,
}
}
pub fn unwrap_accurate_or(self, default: T) -> T {
use ApproxAccurate::*;
match self {
Accurate(a) => a,
Approximate(_) => default,
// Empty is still accurate, just special case for above `max`
Empty => T::default(),
}
}
}
#[cfg(test)]
mod tests {
use super::ApproxAccurate;
#[test]
fn accumulate_only_some() {
let acc = (0..=5)
.into_iter()
.map(Some)
.fold(ApproxAccurate::default(), |acc, next| acc.max(next));
assert_eq!(acc.accurate(), Some(5));
assert!(!acc.is_approximate());
assert_eq!(acc.unwrap_accurate_or(42), 5);
}
#[test]
fn accumulate_some_and_none() {
let acc = [Some(0), None, Some(2)]
.into_iter()
.fold(ApproxAccurate::default(), |acc, next| acc.max(next));
assert_eq!(acc.accurate(), None);
assert!(acc.is_approximate());
assert_eq!(acc.unwrap_accurate_or(42), 42);
}
#[test]
fn accumulate_none_and_some() {
let acc = [None, Some(1), None]
.into_iter()
.fold(ApproxAccurate::default(), |acc, next| acc.max(next));
assert_eq!(acc.accurate(), None);
assert!(acc.is_approximate());
assert_eq!(acc.unwrap_accurate_or(42), 42);
}
#[test]
fn accumulate_none() {
let acc = ApproxAccurate::<i32>::default();
// it is accurate empty
assert_eq!(acc.accurate(), Some(0));
assert!(!acc.is_approximate());
assert_eq!(acc.unwrap_accurate_or(42), 0);
}
}

View File

@@ -33,7 +33,6 @@ pub mod pid_file;
// Misc
pub mod accum;
pub mod approx_accurate;
pub mod shutdown;
// Utility for binding TcpListeners with proper socket options.

View File

@@ -39,12 +39,7 @@
// - The `#[allow(dead_code)]` above various structs are to suppress warnings about only the Debug impl
// reading these fields. We use the Debug impl for semi-structured logging, though.
use std::{
collections::HashMap,
ops::ControlFlow,
sync::{Arc, Mutex},
time::Duration,
};
use std::{collections::HashMap, ops::ControlFlow, sync::Arc, time::Duration};
use anyhow::Context;
use nix::dir::Dir;
@@ -54,7 +49,7 @@ use sync_wrapper::SyncWrapper;
use tokio::time::Instant;
use tokio_util::sync::CancellationToken;
use tracing::{debug, error, info, instrument, warn, Instrument};
use utils::{approx_accurate::ApproxAccurate, id::TenantId, serde_percent::Percent};
use utils::{id::TenantId, serde_percent::Percent};
use crate::{
config::PageServerConf,
@@ -511,7 +506,7 @@ async fn extend_lru_candidates(
return ControlFlow::Break(());
}
let mut max_layer_size = ApproxAccurate::default();
let mut max_layer_size: Option<u64> = None;
for tl in tenant.list_timelines() {
if !tl.is_active() {
continue;
@@ -523,7 +518,11 @@ async fn extend_lru_candidates(
.into_iter()
.map(|layer_infos| (tl.clone(), layer_infos)),
);
max_layer_size = max_layer_size.max(info.max_layer_size.accurate());
max_layer_size = match (max_layer_size, info.max_layer_size) {
(Some(x), Some(y)) => Some(x.max(y)),
(Some(only), None) | (None, Some(only)) => Some(only),
(None, None) => None,
};
if cancel.is_cancelled() {
return ControlFlow::Break(());
@@ -538,21 +537,28 @@ async fn extend_lru_candidates(
Mode::RespectTenantMinResidentSize => match tenant.get_min_resident_size_override() {
Some(size) => size,
None => {
match max_layer_size.accurate() {
match max_layer_size {
Some(size) => size,
None => {
let prod_max_layer_file_size = 332_880_000;
// rate-limit warning in case above comment is wrong and we're missing `LayerMetadata` for many layers
static LAST_WARNED: Mutex<Option<Instant>> = Mutex::new(None);
let mut last_warned = LAST_WARNED.lock().unwrap();
if last_warned
.map(|v| v.elapsed() > Duration::from_secs(60))
.unwrap_or(true)
{
warn!(value=prod_max_layer_file_size, "some layers don't have LayerMetadata to calculate max_layer_file_size, using default value");
*last_warned = Some(Instant::now());
if !scratch.is_empty() {
// soft assert
warn!("BUG: no maximum layer size, but still found layers");
scratch.clear();
}
prod_max_layer_file_size
return ControlFlow::Continue(());
// let prod_max_layer_file_size = 332_880_000;
// // rate-limit warning in case above comment is wrong and we're missing `LayerMetadata` for many layers
// static LAST_WARNED: Mutex<Option<Instant>> = Mutex::new(None);
// let mut last_warned = LAST_WARNED.lock().unwrap();
// if last_warned
// .map(|v| v.elapsed() > Duration::from_secs(60))
// .unwrap_or(true)
// {
// warn!(value=prod_max_layer_file_size, "some layers don't have LayerMetadata to calculate max_layer_file_size, using default value");
// *last_warned = Some(Instant::now());
// }
// prod_max_layer_file_size
}
}
}

View File

@@ -56,7 +56,6 @@ use pageserver_api::reltag::RelTag;
use postgres_connection::PgConnectionConfig;
use postgres_ffi::to_pg_timestamp;
use utils::{
approx_accurate::ApproxAccurate,
id::{TenantId, TimelineId},
lsn::{AtomicLsn, Lsn, RecordLsn},
seqwait::SeqWait,
@@ -4040,7 +4039,7 @@ impl Timeline {
pub struct DiskUsageEvictionInfo {
/// Timeline's largest layer (remote or resident)
pub max_layer_size: ApproxAccurate<u64>,
pub max_layer_size: Option<u64>,
/// Timeline's resident layers
pub resident_layers: Vec<LocalLayerInfoForDiskUsageEviction>,
}
@@ -4073,11 +4072,12 @@ impl Timeline {
pub(crate) fn get_local_layers_for_disk_usage_eviction(&self) -> DiskUsageEvictionInfo {
let layers = self.layers.read().unwrap();
let mut max_layer_size = ApproxAccurate::default();
let mut max_layer_size: Option<u64> = None;
let mut resident_layers = Vec::new();
for l in layers.iter_historic_layers() {
max_layer_size = max_layer_size.max(Some(l.file_size()));
let file_size = l.file_size();
max_layer_size = max_layer_size.map_or(Some(file_size), |m| Some(m.max(file_size)));
if l.is_remote_layer() {
continue;