From 6a906c68c97faf6ed7bfb5f6c4847aea9c50e491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arpad=20M=C3=BCller?= Date: Fri, 4 Aug 2023 11:35:45 +0200 Subject: [PATCH] Make {DeltaLayer,ImageLayer}::{load,load_inner} async (#4883) ## Problem The functions `DeltaLayer::load_inner` and `ImageLayer::load_inner` are calling `read_blk` internally, which we would like to turn into an async fn. ## Summary of changes We switch from `once_cell`'s `OnceCell` implementation to the one in `tokio` in order to be able to call an async `get_or_try_init` function. Builds on top of #4839, part of #4743 --- .../src/tenant/storage_layer/delta_layer.rs | 21 ++++++++++++------- .../src/tenant/storage_layer/image_layer.rs | 17 ++++++++++----- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index 70249eb60e77..fd002ef39866 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -41,7 +41,6 @@ use crate::virtual_file::VirtualFile; use crate::{walrecord, TEMP_FILE_SUFFIX}; use crate::{DELTA_FILE_MAGIC, STORAGE_FORMAT_VERSION}; use anyhow::{bail, ensure, Context, Result}; -use once_cell::sync::OnceCell; use pageserver_api::models::{HistoricLayerInfo, LayerAccessKind}; use rand::{distributions::Alphanumeric, Rng}; use serde::{Deserialize, Serialize}; @@ -52,6 +51,7 @@ use std::ops::Range; use std::os::unix::fs::FileExt; use std::path::{Path, PathBuf}; use std::sync::Arc; +use tokio::sync::OnceCell; use tracing::*; use utils::{ @@ -242,7 +242,7 @@ impl Layer for DeltaLayer { return Ok(()); } - let inner = self.load(LayerAccessKind::Dump, ctx)?; + let inner = self.load(LayerAccessKind::Dump, ctx).await?; println!( "index_start_blk: {}, root {}", @@ -317,7 +317,9 @@ impl Layer for DeltaLayer { { // Open the file and lock the metadata in memory - let inner = self.load(LayerAccessKind::GetValueReconstructData, ctx)?; + let inner = self + .load(LayerAccessKind::GetValueReconstructData, ctx) + .await?; // Scan the page versions backwards, starting from `lsn`. let file = &inner.file; @@ -497,7 +499,7 @@ impl DeltaLayer { /// Open the underlying file and read the metadata into memory, if it's /// not loaded already. /// - fn load( + async fn load( &self, access_kind: LayerAccessKind, ctx: &RequestContext, @@ -507,10 +509,11 @@ impl DeltaLayer { // Quick exit if already loaded self.inner .get_or_try_init(|| self.load_inner()) + .await .with_context(|| format!("Failed to load delta layer {}", self.path().display())) } - fn load_inner(&self) -> Result> { + async fn load_inner(&self) -> Result> { let path = self.path(); let file = VirtualFile::open(&path) @@ -571,7 +574,7 @@ impl DeltaLayer { file_size, ), access_stats, - inner: once_cell::sync::OnceCell::new(), + inner: OnceCell::new(), } } @@ -598,7 +601,7 @@ impl DeltaLayer { metadata.len(), ), access_stats: LayerAccessStats::empty_will_record_residence_event_later(), - inner: once_cell::sync::OnceCell::new(), + inner: OnceCell::new(), }) } @@ -621,6 +624,7 @@ impl DeltaLayer { pub async fn load_val_refs(&self, ctx: &RequestContext) -> Result> { let inner = self .load(LayerAccessKind::KeyIter, ctx) + .await .context("load delta layer")?; DeltaLayerInner::load_val_refs(inner) .await @@ -631,6 +635,7 @@ impl DeltaLayer { pub async fn load_keys(&self, ctx: &RequestContext) -> Result> { let inner = self .load(LayerAccessKind::KeyIter, ctx) + .await .context("load delta layer keys")?; DeltaLayerInner::load_keys(inner) .await @@ -784,7 +789,7 @@ impl DeltaLayerWriterInner { metadata.len(), ), access_stats: LayerAccessStats::empty_will_record_residence_event_later(), - inner: once_cell::sync::OnceCell::new(), + inner: OnceCell::new(), }; // fsync the file diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index c92d27a6b870..6c19d0a871d2 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -38,7 +38,6 @@ use crate::{IMAGE_FILE_MAGIC, STORAGE_FORMAT_VERSION, TEMP_FILE_SUFFIX}; use anyhow::{bail, ensure, Context, Result}; use bytes::Bytes; use hex; -use once_cell::sync::OnceCell; use pageserver_api::models::{HistoricLayerInfo, LayerAccessKind}; use rand::{distributions::Alphanumeric, Rng}; use serde::{Deserialize, Serialize}; @@ -48,6 +47,7 @@ use std::io::{Seek, SeekFrom}; use std::ops::Range; use std::os::unix::prelude::FileExt; use std::path::{Path, PathBuf}; +use tokio::sync::OnceCell; use tracing::*; use utils::{ @@ -168,7 +168,7 @@ impl Layer for ImageLayer { return Ok(()); } - let inner = self.load(LayerAccessKind::Dump, ctx)?; + let inner = self.load(LayerAccessKind::Dump, ctx).await?; let file = &inner.file; let tree_reader = DiskBtreeReader::<_, KEY_SIZE>::new(inner.index_start_blk, inner.index_root_blk, file); @@ -197,7 +197,9 @@ impl Layer for ImageLayer { assert!(lsn_range.start >= self.lsn); assert!(lsn_range.end >= self.lsn); - let inner = self.load(LayerAccessKind::GetValueReconstructData, ctx)?; + let inner = self + .load(LayerAccessKind::GetValueReconstructData, ctx) + .await?; let file = &inner.file; let tree_reader = DiskBtreeReader::new(inner.index_start_blk, inner.index_root_blk, file); @@ -314,7 +316,11 @@ impl ImageLayer { /// Open the underlying file and read the metadata into memory, if it's /// not loaded already. /// - fn load(&self, access_kind: LayerAccessKind, ctx: &RequestContext) -> Result<&ImageLayerInner> { + async fn load( + &self, + access_kind: LayerAccessKind, + ctx: &RequestContext, + ) -> Result<&ImageLayerInner> { self.access_stats .record_access(access_kind, ctx.task_kind()); loop { @@ -323,11 +329,12 @@ impl ImageLayer { } self.inner .get_or_try_init(|| self.load_inner()) + .await .with_context(|| format!("Failed to load image layer {}", self.path().display()))?; } } - fn load_inner(&self) -> Result { + async fn load_inner(&self) -> Result { let path = self.path(); // Open the file if it's not open already.