diff --git a/Cargo.lock b/Cargo.lock index ef1b7327c5..d95b351e47 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -650,7 +650,7 @@ dependencies = [ "crossterm_winapi", "libc", "mio", - "parking_lot 0.12.0", + "parking_lot 0.12.1", "signal-hook", "signal-hook-mio", "winapi", @@ -1899,6 +1899,7 @@ dependencies = [ "metrics", "nix", "once_cell", + "parking_lot 0.12.1", "postgres", "postgres-protocol", "postgres-types", @@ -1939,9 +1940,9 @@ dependencies = [ [[package]] name = "parking_lot" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f5ec2493a61ac0506c0f4199f99070cbe83857b0337006a30f3e6719b8ef58" +checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" dependencies = [ "lock_api", "parking_lot_core 0.9.2", @@ -2308,7 +2309,7 @@ dependencies = [ "lazy_static", "md5", "metrics", - "parking_lot 0.12.0", + "parking_lot 0.12.1", "pin-project-lite", "rand", "rcgen", @@ -3356,7 +3357,7 @@ dependencies = [ "fallible-iterator", "futures", "log", - "parking_lot 0.12.0", + "parking_lot 0.12.1", "percent-encoding", "phf", "pin-project-lite", diff --git a/pageserver/Cargo.toml b/pageserver/Cargo.toml index b7d97a67c0..ee3d8b842e 100644 --- a/pageserver/Cargo.toml +++ b/pageserver/Cargo.toml @@ -63,6 +63,8 @@ workspace_hack = { version = "0.1", path = "../workspace_hack" } close_fds = "0.3.2" walkdir = "2.3.2" +parking_lot = "0.12.1" + [dev-dependencies] hex-literal = "0.3" tempfile = "3.2" diff --git a/pageserver/src/page_cache.rs b/pageserver/src/page_cache.rs index 716df0f749..5a27adb20b 100644 --- a/pageserver/src/page_cache.rs +++ b/pageserver/src/page_cache.rs @@ -36,13 +36,12 @@ //! mapping is automatically removed and the slot is marked free. //! +use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; + use std::{ collections::{hash_map::Entry, HashMap}, convert::TryInto, - sync::{ - atomic::{AtomicU8, AtomicUsize, Ordering}, - RwLock, RwLockReadGuard, RwLockWriteGuard, TryLockError, - }, + sync::atomic::{AtomicU8, AtomicUsize, Ordering}, }; use once_cell::sync::OnceCell; @@ -385,7 +384,7 @@ impl PageCache { for slot_idx in 0..self.slots.len() { let slot = &self.slots[slot_idx]; - let mut inner = slot.inner.write().unwrap(); + let mut inner = slot.inner.write(); if let Some(key) = &inner.key { match key { CacheKey::EphemeralPage { file_id, blkno: _ } if *file_id == drop_file_id => { @@ -413,7 +412,7 @@ impl PageCache { for slot_idx in 0..self.slots.len() { let slot = &self.slots[slot_idx]; - let mut inner = slot.inner.write().unwrap(); + let mut inner = slot.inner.write(); if let Some(key) = &inner.key { match key { CacheKey::ImmutableFilePage { file_id, blkno: _ } @@ -454,7 +453,7 @@ impl PageCache { // that it's still what we expected (because we released the mapping // lock already, another thread could have evicted the page) let slot = &self.slots[slot_idx]; - let inner = slot.inner.read().unwrap(); + let inner = slot.inner.read(); if inner.key.as_ref() == Some(cache_key) { slot.inc_usage_count(); return Some(PageReadGuard(inner)); @@ -543,7 +542,7 @@ impl PageCache { // that it's still what we expected (because we don't released the mapping // lock already, another thread could have evicted the page) let slot = &self.slots[slot_idx]; - let inner = slot.inner.write().unwrap(); + let inner = slot.inner.write(); if inner.key.as_ref() == Some(cache_key) { slot.inc_usage_count(); return Some(PageWriteGuard { inner, valid: true }); @@ -611,7 +610,7 @@ impl PageCache { fn search_mapping(&self, cache_key: &mut CacheKey) -> Option { match cache_key { CacheKey::MaterializedPage { hash_key, lsn } => { - let map = self.materialized_page_map.read().unwrap(); + let map = self.materialized_page_map.read(); let versions = map.get(hash_key)?; let version_idx = match versions.binary_search_by_key(lsn, |v| v.lsn) { @@ -624,11 +623,11 @@ impl PageCache { Some(version.slot_idx) } CacheKey::EphemeralPage { file_id, blkno } => { - let map = self.ephemeral_page_map.read().unwrap(); + let map = self.ephemeral_page_map.read(); Some(*map.get(&(*file_id, *blkno))?) } CacheKey::ImmutableFilePage { file_id, blkno } => { - let map = self.immutable_page_map.read().unwrap(); + let map = self.immutable_page_map.read(); Some(*map.get(&(*file_id, *blkno))?) } } @@ -641,7 +640,7 @@ impl PageCache { fn search_mapping_for_write(&self, key: &CacheKey) -> Option { match key { CacheKey::MaterializedPage { hash_key, lsn } => { - let map = self.materialized_page_map.read().unwrap(); + let map = self.materialized_page_map.read(); let versions = map.get(hash_key)?; if let Ok(version_idx) = versions.binary_search_by_key(lsn, |v| v.lsn) { @@ -651,11 +650,11 @@ impl PageCache { } } CacheKey::EphemeralPage { file_id, blkno } => { - let map = self.ephemeral_page_map.read().unwrap(); + let map = self.ephemeral_page_map.read(); Some(*map.get(&(*file_id, *blkno))?) } CacheKey::ImmutableFilePage { file_id, blkno } => { - let map = self.immutable_page_map.read().unwrap(); + let map = self.immutable_page_map.read(); Some(*map.get(&(*file_id, *blkno))?) } } @@ -670,7 +669,7 @@ impl PageCache { hash_key: old_hash_key, lsn: old_lsn, } => { - let mut map = self.materialized_page_map.write().unwrap(); + let mut map = self.materialized_page_map.write(); if let Entry::Occupied(mut old_entry) = map.entry(old_hash_key.clone()) { let versions = old_entry.get_mut(); @@ -685,12 +684,12 @@ impl PageCache { } } CacheKey::EphemeralPage { file_id, blkno } => { - let mut map = self.ephemeral_page_map.write().unwrap(); + let mut map = self.ephemeral_page_map.write(); map.remove(&(*file_id, *blkno)) .expect("could not find old key in mapping"); } CacheKey::ImmutableFilePage { file_id, blkno } => { - let mut map = self.immutable_page_map.write().unwrap(); + let mut map = self.immutable_page_map.write(); map.remove(&(*file_id, *blkno)) .expect("could not find old key in mapping"); } @@ -708,7 +707,7 @@ impl PageCache { hash_key: new_key, lsn: new_lsn, } => { - let mut map = self.materialized_page_map.write().unwrap(); + let mut map = self.materialized_page_map.write(); let versions = map.entry(new_key.clone()).or_default(); match versions.binary_search_by_key(new_lsn, |v| v.lsn) { Ok(version_idx) => Some(versions[version_idx].slot_idx), @@ -725,7 +724,7 @@ impl PageCache { } } CacheKey::EphemeralPage { file_id, blkno } => { - let mut map = self.ephemeral_page_map.write().unwrap(); + let mut map = self.ephemeral_page_map.write(); match map.entry((*file_id, *blkno)) { Entry::Occupied(entry) => Some(*entry.get()), Entry::Vacant(entry) => { @@ -735,7 +734,7 @@ impl PageCache { } } CacheKey::ImmutableFilePage { file_id, blkno } => { - let mut map = self.immutable_page_map.write().unwrap(); + let mut map = self.immutable_page_map.write(); match map.entry((*file_id, *blkno)) { Entry::Occupied(entry) => Some(*entry.get()), Entry::Vacant(entry) => { @@ -765,11 +764,8 @@ impl PageCache { if slot.dec_usage_count() == 0 { let mut inner = match slot.inner.try_write() { - Ok(inner) => inner, - Err(TryLockError::Poisoned(err)) => { - panic!("buffer lock was poisoned: {:?}", err) - } - Err(TryLockError::WouldBlock) => { + Some(inner) => inner, + None => { // If we have looped through the whole buffer pool 10 times // and still haven't found a victim buffer, something's wrong. // Maybe all the buffers were in locked. That could happen in