Use RwLocks in image and delta layers for more concurrency.

With a Mutex, only one thread could read from the layer at a time. I did
some ad hoc profiling with pgbench and saw that a fair amout of time was
spent blocked on these Mutexes.

pageserver/src/layered_repository/delta_layer.rs

@@ -58,7 +58,7 @@ use std::io::{BufWriter, Write};
 use std::ops::Bound::Included;
 use std::os::unix::fs::FileExt;
 use std::path::{Path, PathBuf};
-use std::sync::{Mutex, MutexGuard};
+use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard, TryLockError};
 
 use bookfile::{Book, BookWriter, BoundedReader, ChapterWriter};
 
@@ -142,7 +142,7 @@ pub struct DeltaLayer {
 
     dropped: bool,
 
-    inner: Mutex<DeltaLayerInner>,
+    inner: RwLock<DeltaLayerInner>,
 }
 
 pub struct DeltaLayerInner {
@@ -316,7 +316,11 @@ impl Layer for DeltaLayer {
     /// it will need to be loaded back.
     ///
     fn unload(&self) -> Result<()> {
-        let mut inner = self.inner.lock().unwrap();
+        let mut inner = match self.inner.try_write() {
+            Ok(inner) => inner,
+            Err(TryLockError::WouldBlock) => return Ok(()),
+            Err(TryLockError::Poisoned(_)) => panic!("DeltaLayer lock was poisoned"),
+        };
         inner.page_version_metas = VecMap::default();
         inner.seg_sizes = VecMap::default();
         inner.loaded = false;
@@ -406,16 +410,37 @@ impl DeltaLayer {
     }
 
     ///
-    /// Load the contents of the file into memory
+    /// Open the underlying file and read the metadata into memory, if it's
+    /// not loaded already.
     ///
-    fn load(&self) -> Result<MutexGuard<DeltaLayerInner>> {
-        // quick exit if already loaded
-        let mut inner = self.inner.lock().unwrap();
+    fn load(&self) -> Result<RwLockReadGuard<DeltaLayerInner>> {
+        loop {
+            // Quick exit if already loaded
+            let inner = self.inner.read().unwrap();
+            if inner.loaded {
+                return Ok(inner);
+            }
 
-        if inner.loaded {
-            return Ok(inner);
+            // Need to open the file and load the metadata. Upgrade our lock to
+            // a write lock. (Or rather, release and re-lock in write mode.)
+            drop(inner);
+            let inner = self.inner.write().unwrap();
+            if !inner.loaded {
+                self.load_inner(inner)?;
+            } else {
+                // Another thread loaded it while we were not holding the lock.
+            }
+
+            // We now have the file open and loaded. There's no function to do
+            // that in the std library RwLock, so we have to release and re-lock
+            // in read mode. (To be precise, the lock guard was moved in the
+            // above call to `load_inner`, so it's already been released). And
+            // while we do that, another thread could unload again, so we have
+            // to re-check and retry if that happens.
         }
+    }
 
+    fn load_inner(&self, mut inner: RwLockWriteGuard<DeltaLayerInner>) -> Result<()> {
         let path = self.path();
 
         // Open the file if it's not open already.
@@ -462,7 +487,7 @@ impl DeltaLayer {
         inner.seg_sizes = seg_sizes;
         inner.loaded = true;
 
-        Ok(inner)
+        Ok(())
     }
 
     /// Create a DeltaLayer struct representing an existing file on disk.
@@ -480,7 +505,7 @@ impl DeltaLayer {
             start_lsn: filename.start_lsn,
             end_lsn: filename.end_lsn,
             dropped: filename.dropped,
-            inner: Mutex::new(DeltaLayerInner {
+            inner: RwLock::new(DeltaLayerInner {
                 loaded: false,
                 book: None,
                 page_version_metas: VecMap::default(),
@@ -507,7 +532,7 @@ impl DeltaLayer {
             start_lsn: summary.start_lsn,
             end_lsn: summary.end_lsn,
             dropped: summary.dropped,
-            inner: Mutex::new(DeltaLayerInner {
+            inner: RwLock::new(DeltaLayerInner {
                 loaded: false,
                 book: None,
                 page_version_metas: VecMap::default(),
@@ -689,7 +714,7 @@ impl DeltaLayerWriter {
             start_lsn: self.start_lsn,
             end_lsn: self.end_lsn,
             dropped: self.dropped,
-            inner: Mutex::new(DeltaLayerInner {
+            inner: RwLock::new(DeltaLayerInner {
                 loaded: false,
                 book: None,
                 page_version_metas: VecMap::default(),

pageserver/src/layered_repository/image_layer.rs

@@ -37,7 +37,7 @@ use std::convert::TryInto;
 use std::fs;
 use std::io::{BufWriter, Write};
 use std::path::{Path, PathBuf};
-use std::sync::{Mutex, MutexGuard};
+use std::sync::{RwLock, RwLockReadGuard};
 
 use bookfile::{Book, BookWriter, ChapterWriter};
 
@@ -93,7 +93,7 @@ pub struct ImageLayer {
     // This entry contains an image of all pages as of this LSN
     pub lsn: Lsn,
 
-    inner: Mutex<ImageLayerInner>,
+    inner: RwLock<ImageLayerInner>,
 }
 
 #[derive(Clone)]
@@ -273,16 +273,38 @@ impl ImageLayer {
     }
 
     ///
-    /// Load the contents of the file into memory
+    /// Open the underlying file and read the metadata into memory, if it's
+    /// not loaded already.
     ///
-    fn load(&self) -> Result<MutexGuard<ImageLayerInner>> {
-        // quick exit if already loaded
-        let mut inner = self.inner.lock().unwrap();
+    fn load(&self) -> Result<RwLockReadGuard<ImageLayerInner>> {
+        loop {
+            // Quick exit if already loaded
+            let inner = self.inner.read().unwrap();
+            if inner.book.is_some() {
+                return Ok(inner);
+            }
 
-        if inner.book.is_some() {
-            return Ok(inner);
+            // Need to open the file and load the metadata. Upgrade our lock to
+            // a write lock. (Or rather, release and re-lock in write mode.)
+            drop(inner);
+            let mut inner = self.inner.write().unwrap();
+            if inner.book.is_none() {
+                self.load_inner(&mut inner)?;
+            } else {
+                // Another thread loaded it while we were not holding the lock.
+            }
+
+            // We now have the file open and loaded. There's no function to do
+            // that in the std library RwLock, so we have to release and re-lock
+            // in read mode. (To be precise, the lock guard was moved in the
+            // above call to `load_inner`, so it's already been released). And
+            // while we do that, another thread could unload again, so we have
+            // to re-check and retry if that happens.
+            drop(inner);
         }
+    }
 
+    fn load_inner(&self, inner: &mut ImageLayerInner) -> Result<()> {
         let path = self.path();
         let file = VirtualFile::open(&path)
             .with_context(|| format!("Failed to open virtual file '{}'", path.display()))?;
@@ -336,7 +358,7 @@ impl ImageLayer {
             image_type,
         };
 
-        Ok(inner)
+        Ok(())
     }
 
     /// Create an ImageLayer struct representing an existing file on disk
@@ -352,7 +374,7 @@ impl ImageLayer {
             tenantid,
             seg: filename.seg,
             lsn: filename.lsn,
-            inner: Mutex::new(ImageLayerInner {
+            inner: RwLock::new(ImageLayerInner {
                 book: None,
                 image_type: ImageType::Blocky { num_blocks: 0 },
             }),
@@ -375,7 +397,7 @@ impl ImageLayer {
             tenantid: summary.tenantid,
             seg: summary.seg,
             lsn: summary.lsn,
-            inner: Mutex::new(ImageLayerInner {
+            inner: RwLock::new(ImageLayerInner {
                 book: None,
                 image_type: ImageType::Blocky { num_blocks: 0 },
             }),
@@ -522,7 +544,7 @@ impl ImageLayerWriter {
             tenantid: self.tenantid,
             seg: self.seg,
             lsn: self.lsn,
-            inner: Mutex::new(ImageLayerInner {
+            inner: RwLock::new(ImageLayerInner {
                 book: None,
                 image_type,
             }),