fixup: update external loop state before handling keyspaces

`ValuesReconstructState::consume_done_keys` may only be called once
after a layer visit. The code in the previous commit called it for
each keyspace, resulting in keys not being marked done in the split
keyspace by floor LSN scenario.

.github/workflows/_push-to-acr.yml

@@ -26,9 +26,15 @@ on:
         description: Azure tenant ID
         required: true
         type: string
+      skip_if:
+        description: Skip the job if this expression is true
+        required: true
+        type: boolean
 
 jobs:
   push-to-acr:
+    if: ${{ !inputs.skip_if }}
+
     runs-on: ubuntu-22.04
     permissions:
       contents: read  # This is required for actions/checkout

.github/workflows/build_and_test.yml

@@ -849,7 +849,6 @@ jobs:
           done
 
   push-to-acr-dev:
-    if: github.ref_name == 'main'
     needs: [ tag, promote-images ]
     uses: ./.github/workflows/_push-to-acr.yml
     with:
@@ -859,9 +858,9 @@ jobs:
       registry_name: ${{ vars.AZURE_DEV_REGISTRY_NAME }}
       subscription_id: ${{ vars.AZURE_DEV_SUBSCRIPTION_ID }}
       tenant_id: ${{ vars.AZURE_TENANT_ID }}
+      skip_if: ${{ github.ref_name != 'main' }}
 
   push-to-acr-prod:
-    if: github.ref_name == 'release'|| github.ref_name == 'release-proxy'
     needs: [ tag, promote-images ]
     uses: ./.github/workflows/_push-to-acr.yml
     with:
@@ -871,6 +870,7 @@ jobs:
       registry_name: ${{ vars.AZURE_PROD_REGISTRY_NAME }}
       subscription_id: ${{ vars.AZURE_PROD_SUBSCRIPTION_ID }}
       tenant_id: ${{ vars.AZURE_TENANT_ID }}
+      skip_if: ${{ !startsWith(github.ref_name, 'release') }}
 
   trigger-custom-extensions-build-and-wait:
     needs: [ check-permissions, tag ]
@@ -948,8 +948,7 @@ jobs:
 
   deploy:
     needs: [ check-permissions, promote-images, tag, build-and-test-locally, trigger-custom-extensions-build-and-wait, push-to-acr-dev, push-to-acr-prod ]
-    # `!failure() && !cancelled()` is required because the workflow depends on the job that can be skipped: `push-to-acr-dev` and `push-to-acr-prod`
-    if: (github.ref_name == 'main' || github.ref_name == 'release' || github.ref_name == 'release-proxy') && !failure() && !cancelled()
+    if: github.ref_name == 'main' || github.ref_name == 'release' || github.ref_name == 'release-proxy'
 
     runs-on: [ self-hosted, small ]
     container: 369495373322.dkr.ecr.eu-central-1.amazonaws.com/ansible:latest
@@ -1047,8 +1046,7 @@ jobs:
   # The job runs on `release` branch and copies compatibility data and Neon artifact from the last *release PR* to the latest directory
   promote-compatibility-data:
     needs: [ deploy ]
-    # `!failure() && !cancelled()` is required because the workflow transitively depends on the job that can be skipped: `push-to-acr-dev` and `push-to-acr-prod`
-    if: github.ref_name == 'release' && !failure() && !cancelled()
+    if: github.ref_name == 'release'
 
     runs-on: ubuntu-22.04
     steps:

control_plane/src/background_process.rs

@@ -151,7 +151,7 @@ where
                     print!(".");
                     io::stdout().flush().unwrap();
                 }
-                tokio::time::sleep(RETRY_INTERVAL).await;
+                thread::sleep(RETRY_INTERVAL);
             }
             Err(e) => {
                 println!("error starting process {process_name:?}: {e:#}");

control_plane/src/bin/neon_local.rs

@@ -34,14 +34,12 @@ use safekeeper_api::{
     DEFAULT_HTTP_LISTEN_PORT as DEFAULT_SAFEKEEPER_HTTP_PORT,
     DEFAULT_PG_LISTEN_PORT as DEFAULT_SAFEKEEPER_PG_PORT,
 };
-use std::borrow::Cow;
 use std::collections::{BTreeSet, HashMap};
 use std::path::PathBuf;
 use std::process::exit;
 use std::str::FromStr;
 use std::time::Duration;
 use storage_broker::DEFAULT_LISTEN_ADDR as DEFAULT_BROKER_ADDR;
-use tokio::task::JoinSet;
 use url::Host;
 use utils::{
     auth::{Claims, Scope},
@@ -89,35 +87,35 @@ fn main() -> Result<()> {
 
     // Check for 'neon init' command first.
     let subcommand_result = if sub_name == "init" {
-        handle_init(sub_args).map(|env| Some(Cow::Owned(env)))
+        handle_init(sub_args).map(Some)
     } else {
         // all other commands need an existing config
-
-        let env = LocalEnv::load_config(&local_env::base_path()).context("Error loading config")?;
+        let mut env =
+            LocalEnv::load_config(&local_env::base_path()).context("Error loading config")?;
         let original_env = env.clone();
-        let env = Box::leak(Box::new(env));
+
         let rt = tokio::runtime::Builder::new_current_thread()
             .enable_all()
             .build()
             .unwrap();
 
         let subcommand_result = match sub_name {
-            "tenant" => rt.block_on(handle_tenant(sub_args, env)),
-            "timeline" => rt.block_on(handle_timeline(sub_args, env)),
-            "start" => rt.block_on(handle_start_all(env, get_start_timeout(sub_args))),
-            "stop" => rt.block_on(handle_stop_all(sub_args, env)),
-            "pageserver" => rt.block_on(handle_pageserver(sub_args, env)),
-            "storage_controller" => rt.block_on(handle_storage_controller(sub_args, env)),
-            "storage_broker" => rt.block_on(handle_storage_broker(sub_args, env)),
-            "safekeeper" => rt.block_on(handle_safekeeper(sub_args, env)),
-            "endpoint" => rt.block_on(handle_endpoint(sub_args, env)),
-            "mappings" => handle_mappings(sub_args, env),
+            "tenant" => rt.block_on(handle_tenant(sub_args, &mut env)),
+            "timeline" => rt.block_on(handle_timeline(sub_args, &mut env)),
+            "start" => rt.block_on(handle_start_all(&env, get_start_timeout(sub_args))),
+            "stop" => rt.block_on(handle_stop_all(sub_args, &env)),
+            "pageserver" => rt.block_on(handle_pageserver(sub_args, &env)),
+            "storage_controller" => rt.block_on(handle_storage_controller(sub_args, &env)),
+            "storage_broker" => rt.block_on(handle_storage_broker(sub_args, &env)),
+            "safekeeper" => rt.block_on(handle_safekeeper(sub_args, &env)),
+            "endpoint" => rt.block_on(handle_endpoint(sub_args, &env)),
+            "mappings" => handle_mappings(sub_args, &mut env),
             "pg" => bail!("'pg' subcommand has been renamed to 'endpoint'"),
             _ => bail!("unexpected subcommand {sub_name}"),
         };
 
-        if &original_env != env {
-            subcommand_result.map(|()| Some(Cow::Borrowed(env)))
+        if original_env != env {
+            subcommand_result.map(|()| Some(env))
         } else {
             subcommand_result.map(|()| None)
         }
@@ -1275,95 +1273,48 @@ async fn handle_storage_broker(sub_match: &ArgMatches, env: &local_env::LocalEnv
 }
 
 async fn handle_start_all(
-    env: &'static local_env::LocalEnv,
+    env: &local_env::LocalEnv,
     retry_timeout: &Duration,
 ) -> anyhow::Result<()> {
-    let Err(errors) = handle_start_all_impl(env, *retry_timeout).await else {
-        neon_start_status_check(env, retry_timeout)
-            .await
-            .context("status check after successful startup of all services")?;
-        return Ok(());
-    };
-
-    eprintln!("startup failed because one or more services could not be started");
-
-    for e in errors {
-        eprintln!("{e}");
-        let debug_repr = format!("{e:?}");
-        for line in debug_repr.lines() {
-            eprintln!("  {line}");
-        }
-    }
-
-    try_stop_all(env, true).await;
-
-    exit(2);
-}
-
-/// Returns Ok() if and only if all services could be started successfully.
-/// Otherwise, returns the list of errors that occurred during startup.
-async fn handle_start_all_impl(
-    env: &'static local_env::LocalEnv,
-    retry_timeout: Duration,
-) -> Result<(), Vec<anyhow::Error>> {
     // Endpoints are not started automatically
 
-    let mut js = JoinSet::new();
+    broker::start_broker_process(env, retry_timeout).await?;
 
-    // force infalliblity through closure
-    #[allow(clippy::redundant_closure_call)]
-    (|| {
-        js.spawn(async move {
-            let retry_timeout = retry_timeout;
-            broker::start_broker_process(env, &retry_timeout).await
-        });
-
-        // Only start the storage controller if the pageserver is configured to need it
-        if env.control_plane_api.is_some() {
-            js.spawn(async move {
-                let storage_controller = StorageController::from_env(env);
-                storage_controller
-                    .start(NeonStorageControllerStartArgs::with_default_instance_id(
-                        retry_timeout.into(),
-                    ))
-                    .await
-                    .map_err(|e| e.context("start storage_controller"))
-            });
-        }
-
-        for ps_conf in &env.pageservers {
-            js.spawn(async move {
-                let pageserver = PageServerNode::from_env(env, ps_conf);
-                pageserver
-                    .start(&retry_timeout)
-                    .await
-                    .map_err(|e| e.context(format!("start pageserver {}", ps_conf.id)))
-            });
-        }
-
-        for node in env.safekeepers.iter() {
-            js.spawn(async move {
-                let safekeeper = SafekeeperNode::from_env(env, node);
-                safekeeper
-                    .start(vec![], &retry_timeout)
-                    .await
-                    .map_err(|e| e.context(format!("start safekeeper {}", safekeeper.id)))
-            });
-        }
-    })();
-
-    let mut errors = Vec::new();
-    while let Some(result) = js.join_next().await {
-        let result = result.expect("we don't panic or cancel the tasks");
-        if let Err(e) = result {
-            errors.push(e);
+    // Only start the storage controller if the pageserver is configured to need it
+    if env.control_plane_api.is_some() {
+        let storage_controller = StorageController::from_env(env);
+        if let Err(e) = storage_controller
+            .start(NeonStorageControllerStartArgs::with_default_instance_id(
+                (*retry_timeout).into(),
+            ))
+            .await
+        {
+            eprintln!("storage_controller start failed: {:#}", e);
+            try_stop_all(env, true).await;
+            exit(1);
         }
     }
 
-    if !errors.is_empty() {
-        return Err(errors);
+    for ps_conf in &env.pageservers {
+        let pageserver = PageServerNode::from_env(env, ps_conf);
+        if let Err(e) = pageserver.start(retry_timeout).await {
+            eprintln!("pageserver {} start failed: {:#}", ps_conf.id, e);
+            try_stop_all(env, true).await;
+            exit(1);
+        }
     }
 
+    for node in env.safekeepers.iter() {
+        let safekeeper = SafekeeperNode::from_env(env, node);
+        if let Err(e) = safekeeper.start(vec![], retry_timeout).await {
+            eprintln!("safekeeper {} start failed: {:#}", safekeeper.id, e);
+            try_stop_all(env, false).await;
+            exit(1);
+        }
+    }
+
+    neon_start_status_check(env, retry_timeout).await?;
+
     Ok(())
 }
 

control_plane/src/endpoint.rs

@@ -702,7 +702,7 @@ impl Endpoint {
                     }
                 }
             }
-            tokio::time::sleep(ATTEMPT_INTERVAL).await;
+            std::thread::sleep(ATTEMPT_INTERVAL);
         }
 
         // disarm the scopeguard, let the child outlive this function (and neon_local invoction)

control_plane/src/pageserver.rs

@@ -17,7 +17,9 @@ use std::time::Duration;
 
 use anyhow::{bail, Context};
 use camino::Utf8PathBuf;
-use pageserver_api::models::{self, AuxFilePolicy, TenantInfo, TimelineInfo};
+use pageserver_api::models::{
+    self, AuxFilePolicy, LocationConfig, TenantHistorySize, TenantInfo, TimelineInfo,
+};
 use pageserver_api::shard::TenantShardId;
 use pageserver_client::mgmt_api;
 use postgres_backend::AuthType;
@@ -322,6 +324,22 @@ impl PageServerNode {
         background_process::stop_process(immediate, "pageserver", &self.pid_file())
     }
 
+    pub async fn page_server_psql_client(
+        &self,
+    ) -> anyhow::Result<(
+        tokio_postgres::Client,
+        tokio_postgres::Connection<tokio_postgres::Socket, tokio_postgres::tls::NoTlsStream>,
+    )> {
+        let mut config = self.pg_connection_config.clone();
+        if self.conf.pg_auth_type == AuthType::NeonJWT {
+            let token = self
+                .env
+                .generate_auth_token(&Claims::new(None, Scope::PageServerApi))?;
+            config = config.set_password(Some(token));
+        }
+        Ok(config.connect_no_tls().await?)
+    }
+
     pub async fn check_status(&self) -> mgmt_api::Result<()> {
         self.http_client.status().await
     }
@@ -522,6 +540,19 @@ impl PageServerNode {
         Ok(())
     }
 
+    pub async fn location_config(
+        &self,
+        tenant_shard_id: TenantShardId,
+        config: LocationConfig,
+        flush_ms: Option<Duration>,
+        lazy: bool,
+    ) -> anyhow::Result<()> {
+        Ok(self
+            .http_client
+            .location_config(tenant_shard_id, config, flush_ms, lazy)
+            .await?)
+    }
+
     pub async fn timeline_list(
         &self,
         tenant_shard_id: &TenantShardId,
@@ -605,4 +636,14 @@ impl PageServerNode {
 
         Ok(())
     }
+
+    pub async fn tenant_synthetic_size(
+        &self,
+        tenant_shard_id: TenantShardId,
+    ) -> anyhow::Result<TenantHistorySize> {
+        Ok(self
+            .http_client
+            .tenant_synthetic_size(tenant_shard_id)
+            .await?)
+    }
 }

control_plane/src/postgresql_conf.rs

@@ -4,10 +4,13 @@
 /// NOTE: This doesn't implement the full, correct postgresql.conf syntax. Just
 /// enough to extract a few settings we need in Neon, assuming you don't do
 /// funny stuff like include-directives or funny escaping.
+use anyhow::{bail, Context, Result};
 use once_cell::sync::Lazy;
 use regex::Regex;
 use std::collections::HashMap;
 use std::fmt;
+use std::io::BufRead;
+use std::str::FromStr;
 
 /// In-memory representation of a postgresql.conf file
 #[derive(Default, Debug)]
@@ -16,16 +19,84 @@ pub struct PostgresConf {
     hash: HashMap<String, String>,
 }
 
+static CONF_LINE_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"^((?:\w|\.)+)\s*=\s*(\S+)$").unwrap());
+
 impl PostgresConf {
     pub fn new() -> PostgresConf {
         PostgresConf::default()
     }
 
+    /// Read file into memory
+    pub fn read(read: impl std::io::Read) -> Result<PostgresConf> {
+        let mut result = Self::new();
+
+        for line in std::io::BufReader::new(read).lines() {
+            let line = line?;
+
+            // Store each line in a vector, in original format
+            result.lines.push(line.clone());
+
+            // Also parse each line and insert key=value lines into a hash map.
+            //
+            // FIXME: This doesn't match exactly the flex/bison grammar in PostgreSQL.
+            // But it's close enough for our usage.
+            let line = line.trim();
+            if line.starts_with('#') {
+                // comment, ignore
+                continue;
+            } else if let Some(caps) = CONF_LINE_RE.captures(line) {
+                let name = caps.get(1).unwrap().as_str();
+                let raw_val = caps.get(2).unwrap().as_str();
+
+                if let Ok(val) = deescape_str(raw_val) {
+                    // Note: if there's already an entry in the hash map for
+                    // this key, this will replace it. That's the behavior what
+                    // we want; when PostgreSQL reads the file, each line
+                    // overrides any previous value for the same setting.
+                    result.hash.insert(name.to_string(), val.to_string());
+                }
+            }
+        }
+        Ok(result)
+    }
+
     /// Return the current value of 'option'
     pub fn get(&self, option: &str) -> Option<&str> {
         self.hash.get(option).map(|x| x.as_ref())
     }
 
+    /// Return the current value of a field, parsed to the right datatype.
+    ///
+    /// This calls the FromStr::parse() function on the value of the field. If
+    /// the field does not exist, or parsing fails, returns an error.
+    ///
+    pub fn parse_field<T>(&self, field_name: &str, context: &str) -> Result<T>
+    where
+        T: FromStr,
+        <T as FromStr>::Err: std::error::Error + Send + Sync + 'static,
+    {
+        self.get(field_name)
+            .with_context(|| format!("could not find '{}' option {}", field_name, context))?
+            .parse::<T>()
+            .with_context(|| format!("could not parse '{}' option {}", field_name, context))
+    }
+
+    pub fn parse_field_optional<T>(&self, field_name: &str, context: &str) -> Result<Option<T>>
+    where
+        T: FromStr,
+        <T as FromStr>::Err: std::error::Error + Send + Sync + 'static,
+    {
+        if let Some(val) = self.get(field_name) {
+            let result = val
+                .parse::<T>()
+                .with_context(|| format!("could not parse '{}' option {}", field_name, context))?;
+
+            Ok(Some(result))
+        } else {
+            Ok(None)
+        }
+    }
+
     ///
     /// Note: if you call this multiple times for the same option, the config
     /// file will a line for each call. It would be nice to have a function
@@ -83,8 +154,48 @@ fn escape_str(s: &str) -> String {
     }
 }
 
+/// De-escape a possibly-quoted value.
+///
+/// See `DeescapeQuotedString` function in PostgreSQL sources for how PostgreSQL
+/// does this.
+fn deescape_str(s: &str) -> Result<String> {
+    // If the string has a quote at the beginning and end, strip them out.
+    if s.len() >= 2 && s.starts_with('\'') && s.ends_with('\'') {
+        let mut result = String::new();
+
+        let mut iter = s[1..(s.len() - 1)].chars().peekable();
+        while let Some(c) = iter.next() {
+            let newc = if c == '\\' {
+                match iter.next() {
+                    Some('b') => '\x08',
+                    Some('f') => '\x0c',
+                    Some('n') => '\n',
+                    Some('r') => '\r',
+                    Some('t') => '\t',
+                    Some('0'..='7') => {
+                        // TODO
+                        bail!("octal escapes not supported");
+                    }
+                    Some(n) => n,
+                    None => break,
+                }
+            } else if c == '\'' && iter.peek() == Some(&'\'') {
+                // doubled quote becomes just one quote
+                iter.next().unwrap()
+            } else {
+                c
+            };
+
+            result.push(newc);
+        }
+        Ok(result)
+    } else {
+        Ok(s.to_string())
+    }
+}
+
 #[test]
-fn test_postgresql_conf_escapes() -> anyhow::Result<()> {
+fn test_postgresql_conf_escapes() -> Result<()> {
     assert_eq!(escape_str("foo bar"), "'foo bar'");
     // these don't need to be quoted
     assert_eq!(escape_str("foo"), "foo");
@@ -103,5 +214,13 @@ fn test_postgresql_conf_escapes() -> anyhow::Result<()> {
     assert_eq!(escape_str("fo\\o"), "'fo\\\\o'");
     assert_eq!(escape_str("10 cats"), "'10 cats'");
 
+    // Test de-escaping
+    assert_eq!(deescape_str(&escape_str("foo"))?, "foo");
+    assert_eq!(deescape_str(&escape_str("fo'o\nba\\r"))?, "fo'o\nba\\r");
+    assert_eq!(deescape_str("'\\b\\f\\n\\r\\t'")?, "\x08\x0c\n\r\t");
+
+    // octal-escapes are currently not supported
+    assert!(deescape_str("'foo\\7\\07\\007'").is_err());
+
     Ok(())
 }

libs/consumption_metrics/src/lib.rs

@@ -5,7 +5,7 @@ use chrono::{DateTime, Utc};
 use rand::Rng;
 use serde::{Deserialize, Serialize};
 
-#[derive(Serialize, Deserialize, Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd)]
+#[derive(Serialize, serde::Deserialize, Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd)]
 #[serde(tag = "type")]
 pub enum EventType {
     #[serde(rename = "absolute")]
@@ -107,7 +107,7 @@ pub const CHUNK_SIZE: usize = 1000;
 
 // Just a wrapper around a slice of events
 // to serialize it as `{"events" : [ ] }
-#[derive(serde::Serialize, Deserialize)]
+#[derive(serde::Serialize, serde::Deserialize)]
 pub struct EventChunk<'a, T: Clone> {
     pub events: std::borrow::Cow<'a, [T]>,
 }

libs/pageserver_api/src/config.rs

@@ -173,6 +173,40 @@ impl Default for EvictionOrder {
     }
 }
 
+#[derive(
+    Eq,
+    PartialEq,
+    Debug,
+    Copy,
+    Clone,
+    strum_macros::EnumString,
+    strum_macros::Display,
+    serde_with::DeserializeFromStr,
+    serde_with::SerializeDisplay,
+)]
+#[strum(serialize_all = "kebab-case")]
+pub enum GetVectoredImpl {
+    Sequential,
+    Vectored,
+}
+
+#[derive(
+    Eq,
+    PartialEq,
+    Debug,
+    Copy,
+    Clone,
+    strum_macros::EnumString,
+    strum_macros::Display,
+    serde_with::DeserializeFromStr,
+    serde_with::SerializeDisplay,
+)]
+#[strum(serialize_all = "kebab-case")]
+pub enum GetImpl {
+    Legacy,
+    Vectored,
+}
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 #[serde(transparent)]
 pub struct MaxVectoredReadBytes(pub NonZeroUsize);
@@ -304,6 +338,8 @@ pub mod defaults {
     pub const DEFAULT_IMAGE_COMPRESSION: ImageCompressionAlgorithm =
         ImageCompressionAlgorithm::Zstd { level: Some(1) };
 
+    pub const DEFAULT_VALIDATE_VECTORED_GET: bool = false;
+
     pub const DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB: usize = 0;
 
     pub const DEFAULT_IO_BUFFER_ALIGNMENT: usize = 512;
@@ -340,10 +376,7 @@ impl Default for ConfigToml {
 
             concurrent_tenant_warmup: (NonZeroUsize::new(DEFAULT_CONCURRENT_TENANT_WARMUP)
                 .expect("Invalid default constant")),
-            concurrent_tenant_size_logical_size_queries: NonZeroUsize::new(
-                DEFAULT_CONCURRENT_TENANT_SIZE_LOGICAL_SIZE_QUERIES,
-            )
-            .unwrap(),
+            concurrent_tenant_size_logical_size_queries: NonZeroUsize::new(1).unwrap(),
             metric_collection_interval: (humantime::parse_duration(
                 DEFAULT_METRIC_COLLECTION_INTERVAL,
             )
@@ -434,6 +467,8 @@ pub mod tenant_conf_defaults {
     // By default ingest enough WAL for two new L0 layers before checking if new image
     // image layers should be created.
     pub const DEFAULT_IMAGE_LAYER_CREATION_CHECK_THRESHOLD: u8 = 2;
+
+    pub const DEFAULT_INGEST_BATCH_SIZE: u64 = 100;
 }
 
 impl Default for TenantConfigToml {

libs/pageserver_api/src/models.rs

@@ -495,7 +495,7 @@ pub struct CompactionAlgorithmSettings {
     pub kind: CompactionAlgorithm,
 }
 
-#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
+#[derive(Debug, PartialEq, Eq, Clone, serde::Deserialize, serde::Serialize)]
 #[serde(tag = "mode", rename_all = "kebab-case", deny_unknown_fields)]
 pub enum L0FlushConfig {
     #[serde(rename_all = "snake_case")]

libs/postgres_backend/src/lib.rs

@@ -280,6 +280,16 @@ pub struct PostgresBackend<IO> {
 
 pub type PostgresBackendTCP = PostgresBackend<tokio::net::TcpStream>;
 
+pub fn query_from_cstring(query_string: Bytes) -> Vec<u8> {
+    let mut query_string = query_string.to_vec();
+    if let Some(ch) = query_string.last() {
+        if *ch == 0 {
+            query_string.pop();
+        }
+    }
+    query_string
+}
+
 /// Cast a byte slice to a string slice, dropping null terminator if there's one.
 fn cstr_to_str(bytes: &[u8]) -> anyhow::Result<&str> {
     let without_null = bytes.strip_suffix(&[0]).unwrap_or(bytes);

libs/postgres_ffi/src/pg_constants.rs

@@ -9,8 +9,8 @@
 //! comments on them.
 //!
 
-use crate::PageHeaderData;
 use crate::BLCKSZ;
+use crate::{PageHeaderData, XLogRecord};
 
 //
 // From pg_tablespace_d.h
@@ -194,6 +194,8 @@ pub const XLR_RMGR_INFO_MASK: u8 = 0xF0;
 pub const XLOG_TBLSPC_CREATE: u8 = 0x00;
 pub const XLOG_TBLSPC_DROP: u8 = 0x10;
 
+pub const SIZEOF_XLOGRECORD: u32 = size_of::<XLogRecord>() as u32;
+
 //
 // from xlogrecord.h
 //
@@ -217,6 +219,8 @@ pub const BKPIMAGE_HAS_HOLE: u8 = 0x01; /* page image has "hole" */
 /* From transam.h */
 pub const FIRST_NORMAL_TRANSACTION_ID: u32 = 3;
 pub const INVALID_TRANSACTION_ID: u32 = 0;
+pub const FIRST_BOOTSTRAP_OBJECT_ID: u32 = 12000;
+pub const FIRST_NORMAL_OBJECT_ID: u32 = 16384;
 
 /* pg_control.h */
 pub const XLOG_CHECKPOINT_SHUTDOWN: u8 = 0x00;

libs/postgres_ffi/src/xlog_utils.rs

@@ -26,7 +26,6 @@ use bytes::{Buf, Bytes};
 use log::*;
 
 use serde::Serialize;
-use std::ffi::OsStr;
 use std::fs::File;
 use std::io::prelude::*;
 use std::io::ErrorKind;
@@ -79,34 +78,19 @@ pub fn XLogFileName(tli: TimeLineID, logSegNo: XLogSegNo, wal_segsz_bytes: usize
     )
 }
 
-pub fn XLogFromFileName(
-    fname: &OsStr,
-    wal_seg_size: usize,
-) -> anyhow::Result<(XLogSegNo, TimeLineID)> {
-    if let Some(fname_str) = fname.to_str() {
-        let tli = u32::from_str_radix(&fname_str[0..8], 16)?;
-        let log = u32::from_str_radix(&fname_str[8..16], 16)? as XLogSegNo;
-        let seg = u32::from_str_radix(&fname_str[16..24], 16)? as XLogSegNo;
-        Ok((log * XLogSegmentsPerXLogId(wal_seg_size) + seg, tli))
-    } else {
-        anyhow::bail!("non-ut8 filename: {:?}", fname);
-    }
+pub fn XLogFromFileName(fname: &str, wal_seg_size: usize) -> (XLogSegNo, TimeLineID) {
+    let tli = u32::from_str_radix(&fname[0..8], 16).unwrap();
+    let log = u32::from_str_radix(&fname[8..16], 16).unwrap() as XLogSegNo;
+    let seg = u32::from_str_radix(&fname[16..24], 16).unwrap() as XLogSegNo;
+    (log * XLogSegmentsPerXLogId(wal_seg_size) + seg, tli)
 }
 
-pub fn IsXLogFileName(fname: &OsStr) -> bool {
-    if let Some(fname) = fname.to_str() {
-        fname.len() == XLOG_FNAME_LEN && fname.chars().all(|c| c.is_ascii_hexdigit())
-    } else {
-        false
-    }
+pub fn IsXLogFileName(fname: &str) -> bool {
+    return fname.len() == XLOG_FNAME_LEN && fname.chars().all(|c| c.is_ascii_hexdigit());
 }
 
-pub fn IsPartialXLogFileName(fname: &OsStr) -> bool {
-    if let Some(fname) = fname.to_str() {
-        fname.ends_with(".partial") && IsXLogFileName(OsStr::new(&fname[0..fname.len() - 8]))
-    } else {
-        false
-    }
+pub fn IsPartialXLogFileName(fname: &str) -> bool {
+    fname.ends_with(".partial") && IsXLogFileName(&fname[0..fname.len() - 8])
 }
 
 /// If LSN points to the beginning of the page, then shift it to first record,

libs/postgres_ffi/wal_craft/src/lib.rs

@@ -7,7 +7,6 @@ use postgres_ffi::{WAL_SEGMENT_SIZE, XLOG_BLCKSZ};
 use postgres_ffi::{
     XLOG_SIZE_OF_XLOG_LONG_PHD, XLOG_SIZE_OF_XLOG_RECORD, XLOG_SIZE_OF_XLOG_SHORT_PHD,
 };
-use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::time::{Duration, Instant};
@@ -27,6 +26,7 @@ macro_rules! xlog_utils_test {
 
 postgres_ffi::for_all_postgres_versions! { xlog_utils_test }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Conf {
     pub pg_version: u32,
     pub pg_distrib_dir: PathBuf,
@@ -136,8 +136,8 @@ impl Conf {
 
     pub fn pg_waldump(
         &self,
-        first_segment_name: &OsStr,
-        last_segment_name: &OsStr,
+        first_segment_name: &str,
+        last_segment_name: &str,
     ) -> anyhow::Result<std::process::Output> {
         let first_segment_file = self.datadir.join(first_segment_name);
         let last_segment_file = self.datadir.join(last_segment_name);

libs/postgres_ffi/wal_craft/src/xlog_utils_test.rs

@@ -4,7 +4,6 @@ use super::*;
 use crate::{error, info};
 use regex::Regex;
 use std::cmp::min;
-use std::ffi::OsStr;
 use std::fs::{self, File};
 use std::io::Write;
 use std::{env, str::FromStr};
@@ -55,7 +54,7 @@ fn test_end_of_wal<C: crate::Crafter>(test_name: &str) {
         .wal_dir()
         .read_dir()
         .unwrap()
-        .map(|f| f.unwrap().file_name())
+        .map(|f| f.unwrap().file_name().into_string().unwrap())
         .filter(|fname| IsXLogFileName(fname))
         .max()
         .unwrap();
@@ -71,11 +70,11 @@ fn test_end_of_wal<C: crate::Crafter>(test_name: &str) {
             start_lsn
         );
         for file in fs::read_dir(cfg.wal_dir()).unwrap().flatten() {
-            let fname = file.file_name();
+            let fname = file.file_name().into_string().unwrap();
             if !IsXLogFileName(&fname) {
                 continue;
             }
-            let (segno, _) = XLogFromFileName(&fname, WAL_SEGMENT_SIZE).unwrap();
+            let (segno, _) = XLogFromFileName(&fname, WAL_SEGMENT_SIZE);
             let seg_start_lsn = XLogSegNoOffsetToRecPtr(segno, 0, WAL_SEGMENT_SIZE);
             if seg_start_lsn > u64::from(*start_lsn) {
                 continue;
@@ -94,10 +93,10 @@ fn test_end_of_wal<C: crate::Crafter>(test_name: &str) {
     }
 }
 
-fn find_pg_waldump_end_of_wal(cfg: &crate::Conf, last_segment: &OsStr) -> Lsn {
+fn find_pg_waldump_end_of_wal(cfg: &crate::Conf, last_segment: &str) -> Lsn {
     // Get the actual end of WAL by pg_waldump
     let waldump_output = cfg
-        .pg_waldump(OsStr::new("000000010000000000000001"), last_segment)
+        .pg_waldump("000000010000000000000001", last_segment)
         .unwrap()
         .stderr;
     let waldump_output = std::str::from_utf8(&waldump_output).unwrap();
@@ -118,7 +117,7 @@ fn find_pg_waldump_end_of_wal(cfg: &crate::Conf, last_segment: &OsStr) -> Lsn {
 
 fn check_end_of_wal(
     cfg: &crate::Conf,
-    last_segment: &OsStr,
+    last_segment: &str,
     start_lsn: Lsn,
     expected_end_of_wal: Lsn,
 ) {
@@ -133,8 +132,7 @@ fn check_end_of_wal(
     // Rename file to partial to actually find last valid lsn, then rename it back.
     fs::rename(
         cfg.wal_dir().join(last_segment),
-        cfg.wal_dir()
-            .join(format!("{}.partial", last_segment.to_str().unwrap())),
+        cfg.wal_dir().join(format!("{}.partial", last_segment)),
     )
     .unwrap();
     let wal_end = find_end_of_wal(&cfg.wal_dir(), WAL_SEGMENT_SIZE, start_lsn).unwrap();
@@ -144,8 +142,7 @@ fn check_end_of_wal(
     );
     assert_eq!(wal_end, expected_end_of_wal);
     fs::rename(
-        cfg.wal_dir()
-            .join(format!("{}.partial", last_segment.to_str().unwrap())),
+        cfg.wal_dir().join(format!("{}.partial", last_segment)),
         cfg.wal_dir().join(last_segment),
     )
     .unwrap();

libs/remote_storage/src/lib.rs

@@ -127,6 +127,10 @@ impl RemotePath {
         &self.0
     }
 
+    pub fn extension(&self) -> Option<&str> {
+        self.0.extension()
+    }
+
     pub fn strip_prefix(&self, p: &RemotePath) -> Result<&Utf8Path, std::path::StripPrefixError> {
         self.0.strip_prefix(&p.0)
     }

libs/utils/src/vec_map.rs

@@ -120,6 +120,32 @@ impl<K: Ord, V> VecMap<K, V> {
         Ok((None, delta_size))
     }
 
+    /// Split the map into two.
+    ///
+    /// The left map contains everything before `cutoff` (exclusive).
+    /// Right map contains `cutoff` and everything after (inclusive).
+    pub fn split_at(&self, cutoff: &K) -> (Self, Self)
+    where
+        K: Clone,
+        V: Clone,
+    {
+        let split_idx = self
+            .data
+            .binary_search_by_key(&cutoff, extract_key)
+            .unwrap_or_else(std::convert::identity);
+
+        (
+            VecMap {
+                data: self.data[..split_idx].to_vec(),
+                ordering: self.ordering,
+            },
+            VecMap {
+                data: self.data[split_idx..].to_vec(),
+                ordering: self.ordering,
+            },
+        )
+    }
+
     /// Move items from `other` to the end of `self`, leaving `other` empty.
     /// If the `other` ordering is different from `self` ordering
     /// `ExtendOrderingError` error will be returned.

pageserver/src/config.rs

@@ -13,6 +13,7 @@ use pageserver_api::{
 use remote_storage::{RemotePath, RemoteStorageConfig};
 use std::env;
 use storage_broker::Uri;
+use utils::crashsafe::path_with_suffix_extension;
 use utils::logging::SecretString;
 
 use once_cell::sync::OnceCell;
@@ -32,7 +33,7 @@ use crate::tenant::storage_layer::inmemory_layer::IndexEntry;
 use crate::tenant::{TENANTS_SEGMENT_NAME, TIMELINES_SEGMENT_NAME};
 use crate::virtual_file;
 use crate::virtual_file::io_engine;
-use crate::{TENANT_HEATMAP_BASENAME, TENANT_LOCATION_CONFIG_NAME};
+use crate::{TENANT_HEATMAP_BASENAME, TENANT_LOCATION_CONFIG_NAME, TIMELINE_DELETE_MARK_SUFFIX};
 
 /// Global state of pageserver.
 ///
@@ -256,6 +257,17 @@ impl PageServerConf {
             .join(timeline_id.to_string())
     }
 
+    pub(crate) fn timeline_delete_mark_file_path(
+        &self,
+        tenant_shard_id: TenantShardId,
+        timeline_id: TimelineId,
+    ) -> Utf8PathBuf {
+        path_with_suffix_extension(
+            self.timeline_path(&tenant_shard_id, &timeline_id),
+            TIMELINE_DELETE_MARK_SUFFIX,
+        )
+    }
+
     /// Turns storage remote path of a file into its local path.
     pub fn local_path(&self, remote_path: &RemotePath) -> Utf8PathBuf {
         remote_path.with_base(&self.workdir)
@@ -479,6 +491,11 @@ pub struct ConfigurableSemaphore {
 }
 
 impl ConfigurableSemaphore {
+    pub const DEFAULT_INITIAL: NonZeroUsize = match NonZeroUsize::new(1) {
+        Some(x) => x,
+        None => panic!("const unwrap is not yet stable"),
+    };
+
     /// Initializse using a non-zero amount of permits.
     ///
     /// Require a non-zero initial permits, because using permits == 0 is a crude way to disable a
@@ -499,6 +516,12 @@ impl ConfigurableSemaphore {
     }
 }
 
+impl Default for ConfigurableSemaphore {
+    fn default() -> Self {
+        Self::new(Self::DEFAULT_INITIAL)
+    }
+}
+
 impl PartialEq for ConfigurableSemaphore {
     fn eq(&self, other: &Self) -> bool {
         // the number of permits can be increased at runtime, so we cannot really fulfill the

pageserver/src/tenant.rs

@@ -140,7 +140,6 @@ pub mod metadata;
 pub mod remote_timeline_client;
 pub mod storage_layer;
 
-pub mod checks;
 pub mod config;
 pub mod mgr;
 pub mod secondary;
@@ -1574,9 +1573,6 @@ impl Tenant {
         image_layer_desc: Vec<(Lsn, Vec<(pageserver_api::key::Key, bytes::Bytes)>)>,
         end_lsn: Lsn,
     ) -> anyhow::Result<Arc<Timeline>> {
-        use checks::check_valid_layermap;
-        use itertools::Itertools;
-
         let tline = self
             .create_test_timeline(new_timeline_id, initdb_lsn, pg_version, ctx)
             .await?;
@@ -1591,18 +1587,6 @@ impl Tenant {
                 .force_create_image_layer(lsn, images, Some(initdb_lsn), ctx)
                 .await?;
         }
-        let layer_names = tline
-            .layers
-            .read()
-            .await
-            .layer_map()
-            .unwrap()
-            .iter_historic_layers()
-            .map(|layer| layer.layer_name())
-            .collect_vec();
-        if let Some(err) = check_valid_layermap(&layer_names) {
-            bail!("invalid layermap: {err}");
-        }
         Ok(tline)
     }
 
@@ -3213,9 +3197,6 @@ impl Tenant {
         image_layer_desc: Vec<(Lsn, Vec<(pageserver_api::key::Key, bytes::Bytes)>)>,
         end_lsn: Lsn,
     ) -> anyhow::Result<Arc<Timeline>> {
-        use checks::check_valid_layermap;
-        use itertools::Itertools;
-
         let tline = self
             .branch_timeline_test(src_timeline, dst_id, ancestor_lsn, ctx)
             .await?;
@@ -3236,18 +3217,6 @@ impl Tenant {
                 .force_create_image_layer(lsn, images, Some(ancestor_lsn), ctx)
                 .await?;
         }
-        let layer_names = tline
-            .layers
-            .read()
-            .await
-            .layer_map()
-            .unwrap()
-            .iter_historic_layers()
-            .map(|layer| layer.layer_name())
-            .collect_vec();
-        if let Some(err) = check_valid_layermap(&layer_names) {
-            bail!("invalid layermap: {err}");
-        }
         Ok(tline)
     }
 

pageserver/src/tenant/checks.rs

@@ -1,55 +0,0 @@
-use std::collections::BTreeSet;
-
-use itertools::Itertools;
-
-use super::storage_layer::LayerName;
-
-/// Checks whether a layer map is valid (i.e., is a valid result of the current compaction algorithm if nothing goes wrong).
-/// The function checks if we can split the LSN range of a delta layer only at the LSNs of the delta layers. For example,
-///
-/// ```plain
-/// |       |                 |       |
-/// |   1   |    |   2   |    |   3   |
-/// |       |    |       |    |       |
-/// ```
-///
-/// This is not a valid layer map because the LSN range of layer 1 intersects with the LSN range of layer 2. 1 and 2 should have
-/// the same LSN range.
-///
-/// The exception is that when layer 2 only contains a single key, it could be split over the LSN range. For example,
-///
-/// ```plain
-/// |       |    |   2   |    |       |
-/// |   1   |    |-------|    |   3   |
-/// |       |    |   4   |    |       |
-///
-/// If layer 2 and 4 contain the same single key, this is also a valid layer map.
-pub fn check_valid_layermap(metadata: &[LayerName]) -> Option<String> {
-    let mut lsn_split_point = BTreeSet::new(); // TODO: use a better data structure (range tree / range set?)
-    let mut all_delta_layers = Vec::new();
-    for name in metadata {
-        if let LayerName::Delta(layer) = name {
-            if layer.key_range.start.next() != layer.key_range.end {
-                all_delta_layers.push(layer.clone());
-            }
-        }
-    }
-    for layer in &all_delta_layers {
-        let lsn_range = &layer.lsn_range;
-        lsn_split_point.insert(lsn_range.start);
-        lsn_split_point.insert(lsn_range.end);
-    }
-    for layer in &all_delta_layers {
-        let lsn_range = layer.lsn_range.clone();
-        let intersects = lsn_split_point.range(lsn_range).collect_vec();
-        if intersects.len() > 1 {
-            let err = format!(
-                "layer violates the layer map LSN split assumption: layer {} intersects with LSN [{}]",
-                layer,
-                intersects.into_iter().map(|lsn| lsn.to_string()).join(", ")
-            );
-            return Some(err);
-        }
-    }
-    None
-}

pageserver/src/tenant/gc_block.rs

@@ -1,29 +1,11 @@
-use std::{collections::HashMap, time::Duration};
+use std::collections::HashMap;
 
-use super::remote_timeline_client::index::GcBlockingReason;
-use tokio::time::Instant;
 use utils::id::TimelineId;
 
-type TimelinesBlocked = HashMap<TimelineId, enumset::EnumSet<GcBlockingReason>>;
+use super::remote_timeline_client::index::GcBlockingReason;
 
-#[derive(Default)]
-struct Storage {
-    timelines_blocked: TimelinesBlocked,
-    /// The deadline before which we are blocked from GC so that
-    /// leases have a chance to be renewed.
-    lsn_lease_deadline: Option<Instant>,
-}
+type Storage = HashMap<TimelineId, enumset::EnumSet<GcBlockingReason>>;
 
-impl Storage {
-    fn is_blocked_by_lsn_lease_deadline(&self) -> bool {
-        self.lsn_lease_deadline
-            .map(|d| Instant::now() < d)
-            .unwrap_or(false)
-    }
-}
-
-/// GcBlock provides persistent (per-timeline) gc blocking and facilitates transient time based gc
-/// blocking.
 #[derive(Default)]
 pub(crate) struct GcBlock {
     /// The timelines which have current reasons to block gc.
@@ -31,12 +13,6 @@ pub(crate) struct GcBlock {
     /// LOCK ORDER: this is held locked while scheduling the next index_part update. This is done
     /// to keep the this field up to date with RemoteTimelineClient `upload_queue.dirty`.
     reasons: std::sync::Mutex<Storage>,
-
-    /// GC background task or manually run `Tenant::gc_iteration` holds a lock on this.
-    ///
-    /// Do not add any more features taking and forbidding taking this lock. It should be
-    /// `tokio::sync::Notify`, but that is rarely used. On the other side, [`GcBlock::insert`]
-    /// synchronizes with gc attempts by locking and unlocking this mutex.
     blocking: tokio::sync::Mutex<()>,
 }
 
@@ -66,20 +42,6 @@ impl GcBlock {
         }
     }
 
-    /// Sets a deadline before which we cannot proceed to GC due to lsn lease.
-    ///
-    /// We do this as the leases mapping are not persisted to disk. By delaying GC by lease
-    /// length, we guarantee that all the leases we granted before will have a chance to renew
-    /// when we run GC for the first time after restart / transition from AttachedMulti to AttachedSingle.
-    pub(super) fn set_lsn_lease_deadline(&self, lsn_lease_length: Duration) {
-        let deadline = Instant::now() + lsn_lease_length;
-        let mut g = self.reasons.lock().unwrap();
-        g.lsn_lease_deadline = Some(deadline);
-    }
-
-    /// Describe the current gc blocking reasons.
-    ///
-    /// TODO: make this json serializable.
     pub(crate) fn summary(&self) -> Option<BlockingReasons> {
         let g = self.reasons.lock().unwrap();
 
@@ -102,7 +64,7 @@ impl GcBlock {
     ) -> anyhow::Result<bool> {
         let (added, uploaded) = {
             let mut g = self.reasons.lock().unwrap();
-            let set = g.timelines_blocked.entry(timeline.timeline_id).or_default();
+            let set = g.entry(timeline.timeline_id).or_default();
             let added = set.insert(reason);
 
             // LOCK ORDER: intentionally hold the lock, see self.reasons.
@@ -133,7 +95,7 @@ impl GcBlock {
 
         let (remaining_blocks, uploaded) = {
             let mut g = self.reasons.lock().unwrap();
-            match g.timelines_blocked.entry(timeline.timeline_id) {
+            match g.entry(timeline.timeline_id) {
                 Entry::Occupied(mut oe) => {
                     let set = oe.get_mut();
                     set.remove(reason);
@@ -147,7 +109,7 @@ impl GcBlock {
                 }
             }
 
-            let remaining_blocks = g.timelines_blocked.len();
+            let remaining_blocks = g.len();
 
             // LOCK ORDER: intentionally hold the lock while scheduling; see self.reasons
             let uploaded = timeline
@@ -172,11 +134,11 @@ impl GcBlock {
     pub(crate) fn before_delete(&self, timeline: &super::Timeline) {
         let unblocked = {
             let mut g = self.reasons.lock().unwrap();
-            if g.timelines_blocked.is_empty() {
+            if g.is_empty() {
                 return;
             }
 
-            g.timelines_blocked.remove(&timeline.timeline_id);
+            g.remove(&timeline.timeline_id);
 
             BlockingReasons::clean_and_summarize(g).is_none()
         };
@@ -187,11 +149,10 @@ impl GcBlock {
     }
 
     /// Initialize with the non-deleted timelines of this tenant.
-    pub(crate) fn set_scanned(&self, scanned: TimelinesBlocked) {
+    pub(crate) fn set_scanned(&self, scanned: Storage) {
         let mut g = self.reasons.lock().unwrap();
-        assert!(g.timelines_blocked.is_empty());
-        g.timelines_blocked
-            .extend(scanned.into_iter().filter(|(_, v)| !v.is_empty()));
+        assert!(g.is_empty());
+        g.extend(scanned.into_iter().filter(|(_, v)| !v.is_empty()));
 
         if let Some(reasons) = BlockingReasons::clean_and_summarize(g) {
             tracing::info!(summary=?reasons, "initialized with gc blocked");
@@ -205,7 +166,6 @@ pub(super) struct Guard<'a> {
 
 #[derive(Debug)]
 pub(crate) struct BlockingReasons {
-    tenant_blocked_by_lsn_lease_deadline: bool,
     timelines: usize,
     reasons: enumset::EnumSet<GcBlockingReason>,
 }
@@ -214,8 +174,8 @@ impl std::fmt::Display for BlockingReasons {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(
             f,
-            "tenant_blocked_by_lsn_lease_deadline: {}, {} timelines block for {:?}",
-            self.tenant_blocked_by_lsn_lease_deadline, self.timelines, self.reasons
+            "{} timelines block for {:?}",
+            self.timelines, self.reasons
         )
     }
 }
@@ -223,15 +183,13 @@ impl std::fmt::Display for BlockingReasons {
 impl BlockingReasons {
     fn clean_and_summarize(mut g: std::sync::MutexGuard<'_, Storage>) -> Option<Self> {
         let mut reasons = enumset::EnumSet::empty();
-        g.timelines_blocked.retain(|_key, value| {
+        g.retain(|_key, value| {
             reasons = reasons.union(*value);
             !value.is_empty()
         });
-        let blocked_by_lsn_lease_deadline = g.is_blocked_by_lsn_lease_deadline();
-        if !g.timelines_blocked.is_empty() || blocked_by_lsn_lease_deadline {
+        if !g.is_empty() {
             Some(BlockingReasons {
-                tenant_blocked_by_lsn_lease_deadline: blocked_by_lsn_lease_deadline,
-                timelines: g.timelines_blocked.len(),
+                timelines: g.len(),
                 reasons,
             })
         } else {
@@ -240,17 +198,14 @@ impl BlockingReasons {
     }
 
     fn summarize(g: &std::sync::MutexGuard<'_, Storage>) -> Option<Self> {
-        let blocked_by_lsn_lease_deadline = g.is_blocked_by_lsn_lease_deadline();
-        if g.timelines_blocked.is_empty() && !blocked_by_lsn_lease_deadline {
+        if g.is_empty() {
             None
         } else {
             let reasons = g
-                .timelines_blocked
                 .values()
                 .fold(enumset::EnumSet::empty(), |acc, next| acc.union(*next));
             Some(BlockingReasons {
-                tenant_blocked_by_lsn_lease_deadline: blocked_by_lsn_lease_deadline,
-                timelines: g.timelines_blocked.len(),
+                timelines: g.len(),
                 reasons,
             })
         }

pageserver/src/tenant/mgr.rs

@@ -949,12 +949,6 @@ impl TenantManager {
                 (LocationMode::Attached(attach_conf), Some(TenantSlot::Attached(tenant))) => {
                     match attach_conf.generation.cmp(&tenant.generation) {
                         Ordering::Equal => {
-                            if attach_conf.attach_mode == AttachmentMode::Single {
-                                tenant
-                                    .gc_block
-                                    .set_lsn_lease_deadline(tenant.get_lsn_lease_length());
-                            }
-
                             // A transition from Attached to Attached in the same generation, we may
                             // take our fast path and just provide the updated configuration
                             // to the tenant.

pageserver/src/tenant/storage_layer.rs

@@ -276,16 +276,6 @@ pub(crate) enum LayerId {
     InMemoryLayerId(InMemoryLayerFileId),
 }
 
-/// Uniquely identify a layer visit by the layer
-/// and LSN floor (or start LSN) of the reads.
-/// The layer itself is not enough since we may
-/// have different LSN lower bounds for delta layer reads.
-#[derive(Debug, PartialEq, Eq, Clone, Hash)]
-struct LayerToVisitId {
-    layer_id: LayerId,
-    lsn_floor: Lsn,
-}
-
 /// Layer wrapper for the read path. Note that it is valid
 /// to use these layers even after external operations have
 /// been performed on them (compaction, freeze, etc.).
@@ -297,9 +287,9 @@ pub(crate) enum ReadableLayer {
 
 /// A partial description of a read to be done.
 #[derive(Debug, Clone)]
-struct LayerVisit {
+struct VisitLocation {
     /// An id used to resolve the readable layer within the fringe
-    layer_to_visit_id: LayerToVisitId,
+    layer_id: LayerId,
     /// Lsn range for the read, used for selecting the next read
     lsn_range: Range<Lsn>,
 }
@@ -313,46 +303,442 @@ struct LayerVisit {
 /// a two layer indexing scheme.
 #[derive(Debug)]
 pub(crate) struct LayerFringe {
-    planned_visits_by_lsn: BinaryHeap<LayerVisit>,
-    visit_reads: HashMap<LayerToVisitId, LayerVisitReads>,
+    visits_by_lsn_index: BinaryHeap<VisitLocation>,
+    layer_visits: HashMap<LayerId, LayerVisitBuilder>,
 }
 
 #[derive(Debug)]
-struct LayerVisitReads {
-    layer: ReadableLayer,
-    target_keyspace: KeySpaceRandomAccum,
+pub(crate) enum LayerVisitBuilder {
+    InMemoryLayer(InMemoryLayerVisitBuilder),
+    PersistentLayer(PersistentLayerVisitBuilder),
+}
+
+#[derive(Debug)]
+pub(crate) enum LayerVisit {
+    InMemoryLayer(InMemoryLayerVisit),
+    PersistentLayer(PersistentLayerVisit),
+}
+
+#[derive(Debug)]
+pub(crate) enum PersistentLayerVisitBuilder {
+    DeltaLayer(DeltaLayerVisitBuilder),
+    ImageLayer(ImageLayerVisitBuilder),
+}
+
+#[derive(Debug)]
+pub(crate) enum PersistentLayerVisit {
+    DeltaLayer(DeltaLayerVisit),
+    ImageLayer(ImageLayerVisit),
+}
+
+#[derive(Debug)]
+pub(crate) struct InMemoryLayerVisitBuilder {
+    /// Key space accumulator which will define which keys we are
+    /// interested in for this layer visit.
+    keyspace_accum: KeySpaceRandomAccum,
+    /// Ignore any keys with an LSN greater or equal
+    /// than the specified one.
+    lsn_ceil: Lsn,
+    /// Only consider keys at LSN greater or equal than the specified one
+    lsn_floor: Lsn,
+    // The in-memory layer to visit
+    layer: Arc<InMemoryLayer>,
+}
+
+#[derive(Debug)]
+pub(crate) struct InMemoryLayerVisit {
+    /// Key space of keys considered by the visit
+    keyspace: KeySpace,
+    /// Ignore any keys with an LSN greater or equal
+    /// than the specified one.
+    lsn_ceil: Lsn,
+    /// Only consider keys at LSN greater or equal than the specified one
+    lsn_floor: Lsn,
+}
+
+#[derive(Debug)]
+pub(crate) struct DeltaLayerVisitBuilder {
+    /// List of key spaces accumulators which will define what deltas are read.
+    /// Each accumulator has an associated LSN which specifies
+    /// the LSN floor for it (i.e. do not read below this LSN).
+    keyspace_accums: HashMap<Lsn, KeySpaceRandomAccum>,
+    /// Ignore any keys with an LSN greater or equal
+    /// than the specified one.
+    lsn_ceil: Lsn,
+    /// Handle for the layer to visit (guaranteed to be a delta layer)
+    layer: Layer,
+}
+
+#[derive(Debug)]
+pub(crate) struct DeltaLayerVisit {
+    /// List of key spaces considered during the visit.
+    /// Each keyspace has an associated LSN which specifies
+    /// the LSN floor for it (i.e. do not read below this LSN).
+    keyspaces: Vec<(Lsn, KeySpace)>,
+    /// Ignore any keys with an LSN greater or equal
+    /// than the specified one.
+    lsn_ceil: Lsn,
+}
+
+#[derive(Debug)]
+pub(crate) struct ImageLayerVisitBuilder {
+    /// Key space which defines which keys we are
+    /// interested in for this layer visit.
+    keyspace_accum: KeySpaceRandomAccum,
+    /// Handle for the layer to visit (guaranteed to be an image layer)
+    layer: Layer,
+    /// Only consider keys at LSN greater or equal than the specified one
+    lsn_floor: Lsn,
+}
+
+#[derive(Debug)]
+pub(crate) struct ImageLayerVisit {
+    /// Key space which defines which keys we are
+    /// interested in for this layer visit.
+    keyspace: KeySpace,
+    /// Only consider keys at LSN greater or equal than the specified one
+    lsn_floor: Lsn,
+}
+
+impl LayerVisitBuilder {
+    pub(crate) fn new(layer: ReadableLayer, keyspace: KeySpace, lsn_range: Range<Lsn>) -> Self {
+        match layer {
+            ReadableLayer::InMemoryLayer(in_mem) => {
+                Self::InMemoryLayer(InMemoryLayerVisitBuilder::new(in_mem, keyspace, lsn_range))
+            }
+            ReadableLayer::PersistentLayer(persistent) => Self::PersistentLayer(
+                PersistentLayerVisitBuilder::new(persistent, keyspace, lsn_range),
+            ),
+        }
+    }
+}
+
+impl InMemoryLayerVisitBuilder {
+    fn new(layer: Arc<InMemoryLayer>, keyspace: KeySpace, lsn_range: Range<Lsn>) -> Self {
+        assert_eq!(lsn_range.start, layer.get_lsn_range().start);
+
+        let mut keyspace_accum = KeySpaceRandomAccum::new();
+        keyspace_accum.add_keyspace(keyspace);
+
+        Self {
+            keyspace_accum,
+            lsn_floor: lsn_range.start,
+            lsn_ceil: lsn_range.end,
+            layer,
+        }
+    }
+}
+
+impl PersistentLayerVisitBuilder {
+    fn new(layer: Layer, keyspace: KeySpace, lsn_range: Range<Lsn>) -> Self {
+        let is_delta = layer.layer_desc().is_delta;
+        if is_delta {
+            Self::DeltaLayer(DeltaLayerVisitBuilder::new(layer, keyspace, lsn_range))
+        } else {
+            Self::ImageLayer(ImageLayerVisitBuilder::new(layer, keyspace, lsn_range))
+        }
+    }
+}
+
+impl DeltaLayerVisitBuilder {
+    fn new(layer: Layer, keyspace: KeySpace, lsn_range: Range<Lsn>) -> Self {
+        assert!(layer.layer_desc().is_delta);
+
+        let mut keyspace_accum = KeySpaceRandomAccum::new();
+        keyspace_accum.add_keyspace(keyspace);
+
+        Self {
+            keyspace_accums: HashMap::from([(lsn_range.start, keyspace_accum)]),
+            lsn_ceil: lsn_range.end,
+            layer,
+        }
+    }
+}
+
+impl ImageLayerVisitBuilder {
+    fn new(layer: Layer, keyspace: KeySpace, lsn_range: Range<Lsn>) -> Self {
+        assert!(!layer.layer_desc().is_delta);
+        assert_eq!(lsn_range.start, layer.layer_desc().lsn_range.start);
+
+        let mut keyspace_accum = KeySpaceRandomAccum::new();
+        keyspace_accum.add_keyspace(keyspace);
+
+        Self {
+            keyspace_accum,
+            lsn_floor: lsn_range.start,
+            layer,
+        }
+    }
+}
+
+pub(crate) trait LayerVisitBuilderUpdate {
+    type L;
+    type LV;
+
+    /// Extend an already planned layer visit to also include the keys
+    /// in the provided keyspace and LSN range.
+    fn update(&mut self, keyspace: KeySpace, lsn_range: Range<Lsn>);
+
+    /// Build the visit!
+    fn build(self) -> (Self::L, Self::LV);
+}
+
+impl LayerVisitBuilderUpdate for LayerVisitBuilder {
+    type L = ReadableLayer;
+    type LV = LayerVisit;
+
+    fn update(&mut self, keyspace: KeySpace, lsn_range: Range<Lsn>) {
+        match self {
+            LayerVisitBuilder::InMemoryLayer(v) => v.update(keyspace, lsn_range),
+            LayerVisitBuilder::PersistentLayer(v) => v.update(keyspace, lsn_range),
+        }
+    }
+
+    fn build(self) -> (Self::L, Self::LV) {
+        match self {
+            LayerVisitBuilder::InMemoryLayer(in_mem) => (
+                ReadableLayer::InMemoryLayer(in_mem.layer),
+                LayerVisit::InMemoryLayer(InMemoryLayerVisit {
+                    keyspace: in_mem.keyspace_accum.to_keyspace(),
+                    lsn_ceil: in_mem.lsn_ceil,
+                    lsn_floor: in_mem.lsn_floor,
+                }),
+            ),
+            LayerVisitBuilder::PersistentLayer(pers) => {
+                let (layer, visit) = pers.build();
+                (
+                    ReadableLayer::PersistentLayer(layer),
+                    LayerVisit::PersistentLayer(visit),
+                )
+            }
+        }
+    }
+}
+
+impl LayerVisitBuilderUpdate for PersistentLayerVisitBuilder {
+    type L = Layer;
+    type LV = PersistentLayerVisit;
+
+    fn update(&mut self, keyspace: KeySpace, lsn_range: Range<Lsn>) {
+        match self {
+            PersistentLayerVisitBuilder::DeltaLayer(v) => v.update(keyspace, lsn_range),
+            PersistentLayerVisitBuilder::ImageLayer(v) => v.update(keyspace, lsn_range),
+        }
+    }
+
+    fn build(self) -> (Self::L, Self::LV) {
+        match self {
+            PersistentLayerVisitBuilder::DeltaLayer(delta) => {
+                let (layer, visit) = delta.build();
+                (layer, PersistentLayerVisit::DeltaLayer(visit))
+            }
+            PersistentLayerVisitBuilder::ImageLayer(img) => {
+                let (layer, visit) = img.build();
+                (layer, PersistentLayerVisit::ImageLayer(visit))
+            }
+        }
+    }
+}
+
+impl LayerVisitBuilderUpdate for InMemoryLayerVisitBuilder {
+    type L = Arc<InMemoryLayer>;
+    type LV = InMemoryLayerVisit;
+
+    fn update(&mut self, keyspace: KeySpace, lsn_range: Range<Lsn>) {
+        // Note: I cannot think of any cases when this update should happen,
+        // since in memory layers span the entire key range.
+        assert_eq!(lsn_range.end, self.lsn_ceil);
+        assert_eq!(lsn_range.start, self.lsn_floor);
+        self.keyspace_accum.add_keyspace(keyspace);
+    }
+
+    fn build(self) -> (Self::L, Self::LV) {
+        (
+            self.layer,
+            InMemoryLayerVisit {
+                keyspace: self.keyspace_accum.to_keyspace(),
+                lsn_floor: self.lsn_floor,
+                lsn_ceil: self.lsn_ceil,
+            },
+        )
+    }
+}
+
+impl LayerVisitBuilderUpdate for DeltaLayerVisitBuilder {
+    type L = Layer;
+    type LV = DeltaLayerVisit;
+
+    fn update(&mut self, keyspace: KeySpace, lsn_range: Range<Lsn>) {
+        assert_eq!(lsn_range.end, self.lsn_ceil);
+        self.keyspace_accums
+            .entry(lsn_range.start)
+            .or_default()
+            .add_keyspace(keyspace);
+    }
+
+    fn build(self) -> (Self::L, Self::LV) {
+        use itertools::Itertools;
+
+        let keyspaces = self
+            .keyspace_accums
+            .into_iter()
+            .filter_map(|(lsn_floor, accum)| {
+                let keyspace = accum.to_keyspace();
+                if keyspace.is_empty() {
+                    None
+                } else {
+                    Some((lsn_floor, keyspace))
+                }
+            })
+            .sorted_by_key(|(_lsn_floor, keyspace)| keyspace.start().unwrap())
+            .collect::<Vec<(Lsn, KeySpace)>>();
+
+        if cfg!(debug_assertions) {
+            // Check that the keyspaces we are going to read from
+            // a layer are non-overlapping.
+            //
+            // The keyspaces provided to vectored read initially are non-overlapping.
+            // We may split keyspaces at each step and keyspaces resulting from a split
+            // are non-overlapping as well. One can prove that the property holds by
+            // induction.
+            let mut prev_end: Option<Key> = None;
+            for (_lsn_floor, crnt) in keyspaces.iter() {
+                if let Some(prev_end) = prev_end {
+                    debug_assert!(prev_end <= crnt.start().unwrap())
+                }
+
+                prev_end = Some(crnt.end().unwrap());
+            }
+        }
+
+        (
+            self.layer,
+            DeltaLayerVisit {
+                keyspaces,
+                lsn_ceil: self.lsn_ceil,
+            },
+        )
+    }
+}
+
+impl LayerVisitBuilderUpdate for ImageLayerVisitBuilder {
+    type L = Layer;
+    type LV = ImageLayerVisit;
+
+    fn update(&mut self, keyspace: KeySpace, lsn_range: Range<Lsn>) {
+        assert_eq!(lsn_range.start, self.lsn_floor);
+        self.keyspace_accum.add_keyspace(keyspace);
+    }
+
+    fn build(self) -> (Self::L, Self::LV) {
+        (
+            self.layer,
+            ImageLayerVisit {
+                keyspace: self.keyspace_accum.to_keyspace(),
+                lsn_floor: self.lsn_floor,
+            },
+        )
+    }
+}
+
+pub(crate) trait LayerVisitDetails {
+    /// Returns the key spaces planned for the visit
+    /// and their associated floor LSNs.
+    fn keyspaces(&self) -> Vec<(Lsn, KeySpace)>;
+}
+
+impl LayerVisitDetails for LayerVisit {
+    fn keyspaces(&self) -> Vec<(Lsn, KeySpace)> {
+        match self {
+            LayerVisit::PersistentLayer(pers) => pers.keyspaces(),
+            LayerVisit::InMemoryLayer(in_mem) => in_mem.keyspaces(),
+        }
+    }
+}
+
+impl LayerVisitDetails for PersistentLayerVisit {
+    fn keyspaces(&self) -> Vec<(Lsn, KeySpace)> {
+        match self {
+            PersistentLayerVisit::DeltaLayer(delta) => delta.keyspaces(),
+            PersistentLayerVisit::ImageLayer(image) => image.keyspaces(),
+        }
+    }
+}
+
+impl LayerVisitDetails for DeltaLayerVisit {
+    fn keyspaces(&self) -> Vec<(Lsn, KeySpace)> {
+        self.keyspaces.clone()
+    }
+}
+
+impl LayerVisitDetails for ImageLayerVisit {
+    fn keyspaces(&self) -> Vec<(Lsn, KeySpace)> {
+        vec![(self.lsn_floor, self.keyspace.clone())]
+    }
+}
+
+impl LayerVisitDetails for InMemoryLayerVisit {
+    fn keyspaces(&self) -> Vec<(Lsn, KeySpace)> {
+        vec![(self.lsn_floor, self.keyspace.clone())]
+    }
+}
+
+impl LayerVisit {
+    fn into_persistent_layer_visit(self) -> PersistentLayerVisit {
+        match self {
+            LayerVisit::PersistentLayer(visit) => visit,
+            LayerVisit::InMemoryLayer(visit) => {
+                panic!("Invalid attempt to cast to PersistentLayerVisit: {visit:?}");
+            }
+        }
+    }
+
+    fn into_in_memory_layer_visit(self) -> InMemoryLayerVisit {
+        match self {
+            LayerVisit::InMemoryLayer(visit) => visit,
+            LayerVisit::PersistentLayer(visit) => {
+                panic!("Invalid attempt to cast to InMemoryLayerVisit: {visit:?}");
+            }
+        }
+    }
+}
+
+impl PersistentLayerVisit {
+    fn into_delta_layer_visit(self) -> DeltaLayerVisit {
+        match self {
+            PersistentLayerVisit::DeltaLayer(visit) => visit,
+            PersistentLayerVisit::ImageLayer(visit) => {
+                panic!("Invalid attempt to cast to DeltaLayerVisit: {visit:?}");
+            }
+        }
+    }
+
+    fn into_image_layer_visit(self) -> ImageLayerVisit {
+        match self {
+            PersistentLayerVisit::ImageLayer(visit) => visit,
+            PersistentLayerVisit::DeltaLayer(visit) => {
+                panic!("Invalid attempt to cast to ImageLayerVisit: {visit:?}");
+            }
+        }
+    }
 }
 
 impl LayerFringe {
     pub(crate) fn new() -> Self {
         LayerFringe {
-            planned_visits_by_lsn: BinaryHeap::new(),
-            visit_reads: HashMap::new(),
+            visits_by_lsn_index: BinaryHeap::new(),
+            layer_visits: HashMap::new(),
         }
     }
 
-    pub(crate) fn next_layer(&mut self) -> Option<(ReadableLayer, KeySpace, Range<Lsn>)> {
-        let read_desc = match self.planned_visits_by_lsn.pop() {
+    pub(crate) fn next_layer(&mut self) -> Option<(ReadableLayer, LayerVisit)> {
+        let read_desc = match self.visits_by_lsn_index.pop() {
             Some(desc) => desc,
             None => return None,
         };
 
-        let removed = self.visit_reads.remove_entry(&read_desc.layer_to_visit_id);
-
-        match removed {
-            Some((
-                _,
-                LayerVisitReads {
-                    layer,
-                    mut target_keyspace,
-                },
-            )) => Some((
-                layer,
-                target_keyspace.consume_keyspace(),
-                read_desc.lsn_range,
-            )),
-            None => unreachable!("fringe internals are always consistent"),
-        }
+        let removed = self.layer_visits.remove(&read_desc.layer_id)?;
+        Some(removed.build())
     }
 
     pub(crate) fn update(
@@ -361,27 +747,19 @@ impl LayerFringe {
         keyspace: KeySpace,
         lsn_range: Range<Lsn>,
     ) {
-        let layer_to_visit_id = LayerToVisitId {
-            layer_id: layer.id(),
-            lsn_floor: lsn_range.start,
-        };
-
-        let entry = self.visit_reads.entry(layer_to_visit_id.clone());
+        let layer_id = layer.id();
+        let entry = self.layer_visits.entry(layer_id.clone());
         match entry {
             Entry::Occupied(mut entry) => {
-                entry.get_mut().target_keyspace.add_keyspace(keyspace);
+                entry.get_mut().update(keyspace, lsn_range);
             }
             Entry::Vacant(entry) => {
-                self.planned_visits_by_lsn.push(LayerVisit {
-                    lsn_range,
-                    layer_to_visit_id: layer_to_visit_id.clone(),
-                });
-                let mut accum = KeySpaceRandomAccum::new();
-                accum.add_keyspace(keyspace);
-                entry.insert(LayerVisitReads {
-                    layer,
-                    target_keyspace: accum,
+                self.visits_by_lsn_index.push(VisitLocation {
+                    lsn_range: lsn_range.clone(),
+                    layer_id: layer_id.clone(),
                 });
+
+                entry.insert(LayerVisitBuilder::new(layer, keyspace, lsn_range));
             }
         }
     }
@@ -393,7 +771,7 @@ impl Default for LayerFringe {
     }
 }
 
-impl Ord for LayerVisit {
+impl Ord for VisitLocation {
     fn cmp(&self, other: &Self) -> Ordering {
         let ord = self.lsn_range.end.cmp(&other.lsn_range.end);
         if ord == std::cmp::Ordering::Equal {
@@ -404,19 +782,19 @@ impl Ord for LayerVisit {
     }
 }
 
-impl PartialOrd for LayerVisit {
+impl PartialOrd for VisitLocation {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         Some(self.cmp(other))
     }
 }
 
-impl PartialEq for LayerVisit {
+impl PartialEq for VisitLocation {
     fn eq(&self, other: &Self) -> bool {
         self.lsn_range == other.lsn_range
     }
 }
 
-impl Eq for LayerVisit {}
+impl Eq for VisitLocation {}
 
 impl ReadableLayer {
     pub(crate) fn id(&self) -> LayerId {
@@ -428,20 +806,27 @@ impl ReadableLayer {
 
     pub(crate) async fn get_values_reconstruct_data(
         &self,
-        keyspace: KeySpace,
-        lsn_range: Range<Lsn>,
+        visit: LayerVisit,
         reconstruct_state: &mut ValuesReconstructState,
         ctx: &RequestContext,
     ) -> Result<(), GetVectoredError> {
         match self {
             ReadableLayer::PersistentLayer(layer) => {
                 layer
-                    .get_values_reconstruct_data(keyspace, lsn_range, reconstruct_state, ctx)
+                    .get_values_reconstruct_data(
+                        visit.into_persistent_layer_visit(),
+                        reconstruct_state,
+                        ctx,
+                    )
                     .await
             }
             ReadableLayer::InMemoryLayer(layer) => {
                 layer
-                    .get_values_reconstruct_data(keyspace, lsn_range.end, reconstruct_state, ctx)
+                    .get_values_reconstruct_data(
+                        visit.into_in_memory_layer_visit(),
+                        reconstruct_state,
+                        ctx,
+                    )
                     .await
             }
         }

pageserver/src/tenant/storage_layer/delta_layer.rs

@@ -75,7 +75,7 @@ use utils::{
     lsn::Lsn,
 };
 
-use super::{AsLayerDesc, LayerName, PersistentLayerDesc, ValuesReconstructState};
+use super::{AsLayerDesc, DeltaLayerVisit, LayerName, PersistentLayerDesc, ValuesReconstructState};
 
 ///
 /// Header stored in the beginning of the file
@@ -841,8 +841,7 @@ impl DeltaLayerInner {
     // can be further optimised to visit the index only once.
     pub(super) async fn get_values_reconstruct_data(
         &self,
-        keyspace: KeySpace,
-        lsn_range: Range<Lsn>,
+        visit: DeltaLayerVisit,
         reconstruct_state: &mut ValuesReconstructState,
         ctx: &RequestContext,
     ) -> Result<(), GetVectoredError> {
@@ -863,8 +862,8 @@ impl DeltaLayerInner {
         let data_end_offset = self.index_start_offset();
 
         let reads = Self::plan_reads(
-            &keyspace,
-            lsn_range.clone(),
+            &visit.keyspaces,
+            visit.lsn_ceil,
             data_end_offset,
             index_reader,
             planner,
@@ -877,14 +876,16 @@ impl DeltaLayerInner {
         self.do_reads_and_update_state(reads, reconstruct_state, ctx)
             .await;
 
-        reconstruct_state.on_lsn_advanced(&keyspace, lsn_range.start);
+        for (lsn_floor, keyspace) in visit.keyspaces {
+            reconstruct_state.on_lsn_advanced(&keyspace, lsn_floor);
+        }
 
         Ok(())
     }
 
     async fn plan_reads<Reader>(
-        keyspace: &KeySpace,
-        lsn_range: Range<Lsn>,
+        keyspaces: &Vec<(Lsn, KeySpace)>,
+        lsn_ceil: Lsn,
         data_end_offset: u64,
         index_reader: DiskBtreeReader<Reader, DELTA_KEY_SIZE>,
         mut planner: VectoredReadPlanner,
@@ -898,48 +899,52 @@ impl DeltaLayerInner {
             .page_content_kind(PageContentKind::DeltaLayerBtreeNode)
             .build();
 
-        for range in keyspace.ranges.iter() {
-            let mut range_end_handled = false;
+        for (lsn_floor, keyspace) in keyspaces {
+            let lsn_range = *lsn_floor..lsn_ceil;
 
-            let start_key = DeltaKey::from_key_lsn(&range.start, lsn_range.start);
-            let index_stream = index_reader.clone().into_stream(&start_key.0, &ctx);
-            let mut index_stream = std::pin::pin!(index_stream);
+            for range in keyspace.ranges.iter() {
+                let mut range_end_handled = false;
 
-            while let Some(index_entry) = index_stream.next().await {
-                let (raw_key, value) = index_entry?;
-                let key = Key::from_slice(&raw_key[..KEY_SIZE]);
-                let lsn = DeltaKey::extract_lsn_from_buf(&raw_key);
-                let blob_ref = BlobRef(value);
+                let start_key = DeltaKey::from_key_lsn(&range.start, lsn_range.start);
+                let index_stream = index_reader.clone().into_stream(&start_key.0, &ctx);
+                let mut index_stream = std::pin::pin!(index_stream);
 
-                // Lsns are not monotonically increasing across keys, so we don't assert on them.
-                assert!(key >= range.start);
+                while let Some(index_entry) = index_stream.next().await {
+                    let (raw_key, value) = index_entry?;
+                    let key = Key::from_slice(&raw_key[..KEY_SIZE]);
+                    let lsn = DeltaKey::extract_lsn_from_buf(&raw_key);
+                    let blob_ref = BlobRef(value);
 
-                let outside_lsn_range = !lsn_range.contains(&lsn);
-                let below_cached_lsn = reconstruct_state.get_cached_lsn(&key) >= Some(lsn);
+                    // Lsns are not monotonically increasing across keys, so we don't assert on them.
+                    assert!(key >= range.start);
 
-                let flag = {
-                    if outside_lsn_range || below_cached_lsn {
-                        BlobFlag::Ignore
-                    } else if blob_ref.will_init() {
-                        BlobFlag::ReplaceAll
+                    let outside_lsn_range = !lsn_range.contains(&lsn);
+                    let below_cached_lsn = reconstruct_state.get_cached_lsn(&key) >= Some(lsn);
+
+                    let flag = {
+                        if outside_lsn_range || below_cached_lsn {
+                            BlobFlag::Ignore
+                        } else if blob_ref.will_init() {
+                            BlobFlag::ReplaceAll
+                        } else {
+                            // Usual path: add blob to the read
+                            BlobFlag::None
+                        }
+                    };
+
+                    if key >= range.end || (key.next() == range.end && lsn >= lsn_range.end) {
+                        planner.handle_range_end(blob_ref.pos());
+                        range_end_handled = true;
+                        break;
                     } else {
-                        // Usual path: add blob to the read
-                        BlobFlag::None
+                        planner.handle(key, lsn, blob_ref.pos(), flag);
                     }
-                };
-
-                if key >= range.end || (key.next() == range.end && lsn >= lsn_range.end) {
-                    planner.handle_range_end(blob_ref.pos());
-                    range_end_handled = true;
-                    break;
-                } else {
-                    planner.handle(key, lsn, blob_ref.pos(), flag);
                 }
-            }
 
-            if !range_end_handled {
-                tracing::debug!("Handling range end fallback at {}", data_end_offset);
-                planner.handle_range_end(data_end_offset);
+                if !range_end_handled {
+                    tracing::debug!("Handling range end fallback at {}", data_end_offset);
+                    planner.handle_range_end(data_end_offset);
+                }
             }
         }
 
@@ -1641,8 +1646,8 @@ pub(crate) mod test {
 
         // Plan and validate
         let vectored_reads = DeltaLayerInner::plan_reads(
-            &keyspace,
-            lsn_range.clone(),
+            &vec![(lsn_range.start, keyspace.clone())],
+            lsn_range.end,
             disk_offset,
             reader,
             planner,
@@ -1895,8 +1900,8 @@ pub(crate) mod test {
             let data_end_offset = inner.index_start_blk as u64 * PAGE_SZ as u64;
 
             let vectored_reads = DeltaLayerInner::plan_reads(
-                &keyspace,
-                entries_meta.lsn_range.clone(),
+                &vec![(entries_meta.lsn_range.start, keyspace)],
+                entries_meta.lsn_range.end,
                 data_end_offset,
                 index_reader,
                 planner,

pageserver/src/tenant/storage_layer/image_layer.rs

@@ -69,7 +69,7 @@ use utils::{
 };
 
 use super::layer_name::ImageLayerName;
-use super::{AsLayerDesc, LayerName, PersistentLayerDesc, ValuesReconstructState};
+use super::{AsLayerDesc, ImageLayerVisit, LayerName, PersistentLayerDesc, ValuesReconstructState};
 
 ///
 /// Header stored in the beginning of the file
@@ -435,12 +435,12 @@ impl ImageLayerInner {
     // the reconstruct state with whatever is found.
     pub(super) async fn get_values_reconstruct_data(
         &self,
-        keyspace: KeySpace,
+        visit: ImageLayerVisit,
         reconstruct_state: &mut ValuesReconstructState,
         ctx: &RequestContext,
     ) -> Result<(), GetVectoredError> {
         let reads = self
-            .plan_reads(keyspace, None, ctx)
+            .plan_reads(visit.keyspace, None, ctx)
             .await
             .map_err(GetVectoredError::Other)?;
 

pageserver/src/tenant/storage_layer/inmemory_layer.rs

@@ -17,7 +17,6 @@ use anyhow::{anyhow, Context, Result};
 use bytes::Bytes;
 use camino::Utf8PathBuf;
 use pageserver_api::key::CompactKey;
-use pageserver_api::keyspace::KeySpace;
 use pageserver_api::models::InMemoryLayerInfo;
 use pageserver_api::shard::TenantShardId;
 use std::collections::{BTreeMap, HashMap};
@@ -36,7 +35,8 @@ use std::sync::atomic::{AtomicU64, AtomicUsize};
 use tokio::sync::RwLock;
 
 use super::{
-    DeltaLayerWriter, PersistentLayerDesc, ValueReconstructSituation, ValuesReconstructState,
+    DeltaLayerWriter, InMemoryLayerVisit, PersistentLayerDesc, ValueReconstructSituation,
+    ValuesReconstructState,
 };
 
 pub(crate) mod vectored_dio_read;
@@ -416,11 +416,14 @@ impl InMemoryLayer {
     // If the key is cached, go no further than the cached Lsn.
     pub(crate) async fn get_values_reconstruct_data(
         &self,
-        keyspace: KeySpace,
-        end_lsn: Lsn,
+        visit: InMemoryLayerVisit,
         reconstruct_state: &mut ValuesReconstructState,
         ctx: &RequestContext,
     ) -> Result<(), GetVectoredError> {
+        let InMemoryLayerVisit {
+            keyspace, lsn_ceil, ..
+        } = visit;
+
         let ctx = RequestContextBuilder::extend(ctx)
             .page_content_kind(PageContentKind::InMemoryLayer)
             .build();
@@ -440,8 +443,8 @@ impl InMemoryLayer {
             {
                 let key = Key::from_compact(*key);
                 let lsn_range = match reconstruct_state.get_cached_lsn(&key) {
-                    Some(cached_lsn) => (cached_lsn + 1)..end_lsn,
-                    None => self.start_lsn..end_lsn,
+                    Some(cached_lsn) => (cached_lsn + 1)..lsn_ceil,
+                    None => self.start_lsn..lsn_ceil,
                 };
 
                 let slice = vec_map.slice_range(lsn_range);

pageserver/src/tenant/storage_layer/layer.rs

@@ -1,9 +1,7 @@
 use anyhow::Context;
 use camino::{Utf8Path, Utf8PathBuf};
-use pageserver_api::keyspace::KeySpace;
 use pageserver_api::models::HistoricLayerInfo;
 use pageserver_api::shard::{ShardIdentity, ShardIndex, TenantShardId};
-use std::ops::Range;
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::{Arc, Weak};
 use std::time::{Duration, SystemTime};
@@ -23,7 +21,7 @@ use super::delta_layer::{self, DeltaEntry};
 use super::image_layer::{self};
 use super::{
     AsLayerDesc, ImageLayerWriter, LayerAccessStats, LayerAccessStatsReset, LayerName,
-    LayerVisibilityHint, PersistentLayerDesc, ValuesReconstructState,
+    LayerVisibilityHint, PersistentLayerDesc, PersistentLayerVisit, ValuesReconstructState,
 };
 
 use utils::generation::Generation;
@@ -303,8 +301,7 @@ impl Layer {
 
     pub(crate) async fn get_values_reconstruct_data(
         &self,
-        keyspace: KeySpace,
-        lsn_range: Range<Lsn>,
+        visit: PersistentLayerVisit,
         reconstruct_data: &mut ValuesReconstructState,
         ctx: &RequestContext,
     ) -> Result<(), GetVectoredError> {
@@ -322,7 +319,7 @@ impl Layer {
         self.record_access(ctx);
 
         layer
-            .get_values_reconstruct_data(keyspace, lsn_range, reconstruct_data, &self.0, ctx)
+            .get_values_reconstruct_data(visit, reconstruct_data, &self.0, ctx)
             .instrument(tracing::debug_span!("get_values_reconstruct_data", layer=%self))
             .await
             .map_err(|err| match err {
@@ -1741,8 +1738,7 @@ impl DownloadedLayer {
 
     async fn get_values_reconstruct_data(
         &self,
-        keyspace: KeySpace,
-        lsn_range: Range<Lsn>,
+        visit: PersistentLayerVisit,
         reconstruct_data: &mut ValuesReconstructState,
         owner: &Arc<LayerInner>,
         ctx: &RequestContext,
@@ -1755,11 +1751,11 @@ impl DownloadedLayer {
             .map_err(GetVectoredError::Other)?
         {
             Delta(d) => {
-                d.get_values_reconstruct_data(keyspace, lsn_range, reconstruct_data, ctx)
+                d.get_values_reconstruct_data(visit.into_delta_layer_visit(), reconstruct_data, ctx)
                     .await
             }
             Image(i) => {
-                i.get_values_reconstruct_data(keyspace, reconstruct_data, ctx)
+                i.get_values_reconstruct_data(visit.into_image_layer_visit(), reconstruct_data, ctx)
                     .await
             }
         }

pageserver/src/tenant/storage_layer/layer/tests.rs

@@ -1,6 +1,6 @@
 use std::time::UNIX_EPOCH;
 
-use pageserver_api::key::CONTROLFILE_KEY;
+use pageserver_api::{key::CONTROLFILE_KEY, keyspace::KeySpace};
 use tokio::task::JoinSet;
 use utils::{
     completion::{self, Completion},
@@ -9,7 +9,10 @@ use utils::{
 
 use super::failpoints::{Failpoint, FailpointKind};
 use super::*;
-use crate::{context::DownloadBehavior, tenant::storage_layer::LayerVisibilityHint};
+use crate::{
+    context::DownloadBehavior,
+    tenant::storage_layer::{ImageLayerVisit, LayerVisibilityHint},
+};
 use crate::{task_mgr::TaskKind, tenant::harness::TenantHarness};
 
 /// Used in tests to advance a future to wanted await point, and not futher.
@@ -56,13 +59,14 @@ async fn smoke_test() {
 
     let img_before = {
         let mut data = ValuesReconstructState::default();
+        let visit = ImageLayerVisit {
+            keyspace: controlfile_keyspace.clone(),
+            lsn_floor: Lsn(0x10),
+        };
+        let visit = PersistentLayerVisit::ImageLayer(visit);
+
         layer
-            .get_values_reconstruct_data(
-                controlfile_keyspace.clone(),
-                Lsn(0x10)..Lsn(0x11),
-                &mut data,
-                &ctx,
-            )
+            .get_values_reconstruct_data(visit, &mut data, &ctx)
             .await
             .unwrap();
         data.keys
@@ -88,13 +92,14 @@ async fn smoke_test() {
     // on accesses when the layer is evicted, it will automatically be downloaded.
     let img_after = {
         let mut data = ValuesReconstructState::default();
+        let visit = ImageLayerVisit {
+            keyspace: controlfile_keyspace.clone(),
+            lsn_floor: Lsn(0x10),
+        };
+        let visit = PersistentLayerVisit::ImageLayer(visit);
+
         layer
-            .get_values_reconstruct_data(
-                controlfile_keyspace.clone(),
-                Lsn(0x10)..Lsn(0x11),
-                &mut data,
-                &ctx,
-            )
+            .get_values_reconstruct_data(visit, &mut data, &ctx)
             .instrument(download_span.clone())
             .await
             .unwrap();

pageserver/src/tenant/tasks.rs

@@ -346,7 +346,6 @@ async fn gc_loop(tenant: Arc<Tenant>, cancel: CancellationToken) {
             RequestContext::todo_child(TaskKind::GarbageCollector, DownloadBehavior::Download);
 
         let mut first = true;
-        tenant.gc_block.set_lsn_lease_deadline(tenant.get_lsn_lease_length());
         loop {
             tokio::select! {
                 _ = cancel.cancelled() => {
@@ -364,6 +363,7 @@ async fn gc_loop(tenant: Arc<Tenant>, cancel: CancellationToken) {
                 first = false;
 
                 let delays = async {
+                    delay_by_lease_length(tenant.get_lsn_lease_length(), &cancel).await?;
                     random_init_delay(period, &cancel).await?;
                     Ok::<_, Cancelled>(())
                 };
@@ -538,12 +538,28 @@ pub(crate) async fn random_init_delay(
         let mut rng = rand::thread_rng();
         rng.gen_range(Duration::ZERO..=period)
     };
+
     match tokio::time::timeout(d, cancel.cancelled()).await {
         Ok(_) => Err(Cancelled),
         Err(_) => Ok(()),
     }
 }
 
+/// Delays GC by defaul lease length at restart.
+///
+/// We do this as the leases mapping are not persisted to disk. By delaying GC by default
+/// length, we gurantees that all the leases we granted before the restart will expire
+/// when we run GC for the first time after the restart.
+pub(crate) async fn delay_by_lease_length(
+    length: Duration,
+    cancel: &CancellationToken,
+) -> Result<(), Cancelled> {
+    match tokio::time::timeout(length, cancel.cancelled()).await {
+        Ok(_) => Err(Cancelled),
+        Err(_) => Ok(()),
+    }
+}
+
 struct Iteration {
     started_at: Instant,
     period: Duration,

pageserver/src/tenant/timeline.rs

@@ -136,7 +136,8 @@ use self::logical_size::LogicalSize;
 use self::walreceiver::{WalReceiver, WalReceiverConf};
 
 use super::{
-    config::TenantConf, storage_layer::inmemory_layer, storage_layer::LayerVisibilityHint,
+    config::TenantConf,
+    storage_layer::{inmemory_layer, LayerVisibilityHint, LayerVisitDetails},
     upload_queue::NotInitialized,
 };
 use super::{debug_assert_current_span_has_tenant_and_timeline_id, AttachedTenantConf};
@@ -3153,12 +3154,12 @@ impl Timeline {
     async fn get_vectored_reconstruct_data_timeline(
         timeline: &Timeline,
         keyspace: KeySpace,
-        mut cont_lsn: Lsn,
+        cont_lsn: Lsn,
         reconstruct_state: &mut ValuesReconstructState,
         cancel: &CancellationToken,
         ctx: &RequestContext,
     ) -> Result<TimelineVisitOutcome, GetVectoredError> {
-        let mut unmapped_keyspace = keyspace.clone();
+        let mut unmapped_keyspaces = vec![(cont_lsn, keyspace.clone())];
         let mut fringe = LayerFringe::new();
 
         let mut completed_keyspace = KeySpace::default();
@@ -3171,84 +3172,86 @@ impl Timeline {
 
             let (keys_done_last_step, keys_with_image_coverage) =
                 reconstruct_state.consume_done_keys();
-            unmapped_keyspace.remove_overlapping_with(&keys_done_last_step);
+
+            // Update state that is external to the loop.
             completed_keyspace.merge(&keys_done_last_step);
-            if let Some(keys_with_image_coverage) = keys_with_image_coverage {
-                unmapped_keyspace
-                    .remove_overlapping_with(&KeySpace::single(keys_with_image_coverage.clone()));
+            if let Some(keys_with_image_coverage) = keys_with_image_coverage.clone() {
                 image_covered_keyspace.add_range(keys_with_image_coverage);
             }
 
-            // Do not descent any further if the last layer we visited
-            // completed all keys in the keyspace it inspected. This is not
-            // required for correctness, but avoids visiting extra layers
-            // which turns out to be a perf bottleneck in some cases.
-            if !unmapped_keyspace.is_empty() {
-                let guard = timeline.layers.read().await;
-                let layers = guard.layer_map()?;
-
-                let in_memory_layer = layers.find_in_memory_layer(|l| {
-                    let start_lsn = l.get_lsn_range().start;
-                    cont_lsn > start_lsn
-                });
-
-                match in_memory_layer {
-                    Some(l) => {
-                        let lsn_range = l.get_lsn_range().start..cont_lsn;
-                        fringe.update(
-                            ReadableLayer::InMemoryLayer(l),
-                            unmapped_keyspace.clone(),
-                            lsn_range,
-                        );
-                    }
-                    None => {
-                        for range in unmapped_keyspace.ranges.iter() {
-                            let results = layers.range_search(range.clone(), cont_lsn);
-
-                            results
-                                .found
-                                .into_iter()
-                                .map(|(SearchResult { layer, lsn_floor }, keyspace_accum)| {
-                                    (
-                                        ReadableLayer::PersistentLayer(guard.get_from_desc(&layer)),
-                                        keyspace_accum.to_keyspace(),
-                                        lsn_floor..cont_lsn,
-                                    )
-                                })
-                                .for_each(|(layer, keyspace, lsn_range)| {
-                                    fringe.update(layer, keyspace, lsn_range)
-                                });
-                        }
-                    }
+            for (cont_lsn, unmapped_keyspace) in unmapped_keyspaces.iter_mut() {
+                // Remove any completed keys from the currently inspected keyspace.
+                unmapped_keyspace.remove_overlapping_with(&keys_done_last_step);
+                if let Some(keys_with_image_coverage) = keys_with_image_coverage.clone() {
+                    unmapped_keyspace
+                        .remove_overlapping_with(&KeySpace::single(keys_with_image_coverage));
                 }
 
-                // It's safe to drop the layer map lock after planning the next round of reads.
-                // The fringe keeps readable handles for the layers which are safe to read even
-                // if layers were compacted or flushed.
-                //
-                // The more interesting consideration is: "Why is the read algorithm still correct
-                // if the layer map changes while it is operating?". Doing a vectored read on a
-                // timeline boils down to pushing an imaginary lsn boundary downwards for each range
-                // covered by the read. The layer map tells us how to move the lsn downwards for a
-                // range at *a particular point in time*. It is fine for the answer to be different
-                // at two different time points.
-                drop(guard);
+                // Do not descent any further if the last layer we visited
+                // completed all keys in the keyspace it inspected. This is not
+                // required for correctness, but avoids visiting extra layers
+                // which turns out to be a perf bottleneck in some cases.
+                if !unmapped_keyspace.is_empty() {
+                    let guard = timeline.layers.read().await;
+                    let layers = guard.layer_map()?;
+
+                    let in_memory_layer = layers.find_in_memory_layer(|l| {
+                        let start_lsn = l.get_lsn_range().start;
+                        *cont_lsn > start_lsn
+                    });
+
+                    match in_memory_layer {
+                        Some(l) => {
+                            let lsn_range = l.get_lsn_range().start..*cont_lsn;
+                            fringe.update(
+                                ReadableLayer::InMemoryLayer(l),
+                                unmapped_keyspace.clone(),
+                                lsn_range,
+                            );
+                        }
+                        None => {
+                            for range in unmapped_keyspace.ranges.iter() {
+                                let results = layers.range_search(range.clone(), *cont_lsn);
+
+                                results
+                                    .found
+                                    .into_iter()
+                                    .map(|(SearchResult { layer, lsn_floor }, keyspace_accum)| {
+                                        (
+                                            ReadableLayer::PersistentLayer(
+                                                guard.get_from_desc(&layer),
+                                            ),
+                                            keyspace_accum.to_keyspace(),
+                                            lsn_floor..*cont_lsn,
+                                        )
+                                    })
+                                    .for_each(|(layer, keyspace, lsn_range)| {
+                                        fringe.update(layer, keyspace, lsn_range)
+                                    });
+                            }
+                        }
+                    }
+
+                    // It's safe to drop the layer map lock after planning the next round of reads.
+                    // The fringe keeps readable handles for the layers which are safe to read even
+                    // if layers were compacted or flushed.
+                    //
+                    // The more interesting consideration is: "Why is the read algorithm still correct
+                    // if the layer map changes while it is operating?". Doing a vectored read on a
+                    // timeline boils down to pushing an imaginary lsn boundary downwards for each range
+                    // covered by the read. The layer map tells us how to move the lsn downwards for a
+                    // range at *a particular point in time*. It is fine for the answer to be different
+                    // at two different time points.
+                    drop(guard);
+                }
             }
 
-            if let Some((layer_to_read, keyspace_to_read, lsn_range)) = fringe.next_layer() {
-                let next_cont_lsn = lsn_range.start;
+            if let Some((layer_to_read, layer_visit)) = fringe.next_layer() {
+                unmapped_keyspaces = layer_visit.keyspaces();
                 layer_to_read
-                    .get_values_reconstruct_data(
-                        keyspace_to_read.clone(),
-                        lsn_range,
-                        reconstruct_state,
-                        ctx,
-                    )
+                    .get_values_reconstruct_data(layer_visit, reconstruct_state, ctx)
                     .await?;
 
-                unmapped_keyspace = keyspace_to_read;
-                cont_lsn = next_cont_lsn;
-
                 reconstruct_state.on_layer_visited(&layer_to_read);
             } else {
                 break;
@@ -4015,7 +4018,6 @@ impl Timeline {
             // partition, so flush it to disk.
             let (desc, path) = image_layer_writer.finish(ctx).await?;
             let image_layer = Layer::finish_creating(self.conf, self, desc, &path)?;
-            info!("created image layer for rel {}", image_layer.local_path());
             Ok(ImageLayerCreationOutcome {
                 image: Some(image_layer),
                 next_start_key: img_range.end,
@@ -4105,10 +4107,6 @@ impl Timeline {
             // partition, so flush it to disk.
             let (desc, path) = image_layer_writer.finish(ctx).await?;
             let image_layer = Layer::finish_creating(self.conf, self, desc, &path)?;
-            info!(
-                "created image layer for metadata {}",
-                image_layer.local_path()
-            );
             Ok(ImageLayerCreationOutcome {
                 image: Some(image_layer),
                 next_start_key: img_range.end,
@@ -5378,8 +5376,7 @@ impl Timeline {
     /// Force create an image layer and place it into the layer map.
     ///
     /// DO NOT use this function directly. Use [`Tenant::branch_timeline_test_with_layers`]
-    /// or [`Tenant::create_test_timeline_with_layers`] to ensure all these layers are
-    /// placed into the layer map in one run AND be validated.
+    /// or [`Tenant::create_test_timeline_with_layers`] to ensure all these layers are placed into the layer map in one run.
     #[cfg(test)]
     pub(super) async fn force_create_image_layer(
         self: &Arc<Timeline>,
@@ -5413,7 +5410,7 @@ impl Timeline {
         }
         let (desc, path) = image_layer_writer.finish(ctx).await?;
         let image_layer = Layer::finish_creating(self.conf, self, desc, &path)?;
-        info!("force created image layer {}", image_layer.local_path());
+
         {
             let mut guard = self.layers.write().await;
             guard.open_mut().unwrap().force_insert_layer(image_layer);
@@ -5425,8 +5422,7 @@ impl Timeline {
     /// Force create a delta layer and place it into the layer map.
     ///
     /// DO NOT use this function directly. Use [`Tenant::branch_timeline_test_with_layers`]
-    /// or [`Tenant::create_test_timeline_with_layers`] to ensure all these layers are
-    /// placed into the layer map in one run AND be validated.
+    /// or [`Tenant::create_test_timeline_with_layers`] to ensure all these layers are placed into the layer map in one run.
     #[cfg(test)]
     pub(super) async fn force_create_delta_layer(
         self: &Arc<Timeline>,
@@ -5452,6 +5448,33 @@ impl Timeline {
         if let Some(check_start_lsn) = check_start_lsn {
             assert!(deltas.lsn_range.start >= check_start_lsn);
         }
+        // check if the delta layer does not violate the LSN invariant, the legacy compaction should always produce a batch of
+        // layers of the same start/end LSN, and so should the force inserted layer
+        {
+            /// Checks if a overlaps with b, assume a/b = [start, end).
+            pub fn overlaps_with<T: Ord>(a: &Range<T>, b: &Range<T>) -> bool {
+                !(a.end <= b.start || b.end <= a.start)
+            }
+
+            if deltas.key_range.start.next() != deltas.key_range.end {
+                let guard = self.layers.read().await;
+                let mut invalid_layers =
+                    guard.layer_map()?.iter_historic_layers().filter(|layer| {
+                        layer.is_delta()
+                        && overlaps_with(&layer.lsn_range, &deltas.lsn_range)
+                        && layer.lsn_range != deltas.lsn_range
+                        // skip single-key layer files
+                        && layer.key_range.start.next() != layer.key_range.end
+                    });
+                if let Some(layer) = invalid_layers.next() {
+                    // If a delta layer overlaps with another delta layer AND their LSN range is not the same, panic
+                    panic!(
+                        "inserted layer violates delta layer LSN invariant: current_lsn_range={}..{}, conflict_lsn_range={}..{}",
+                        deltas.lsn_range.start, deltas.lsn_range.end, layer.lsn_range.start, layer.lsn_range.end
+                    );
+                }
+            }
+        }
         let mut delta_layer_writer = DeltaLayerWriter::new(
             self.conf,
             self.timeline_id,
@@ -5466,7 +5489,7 @@ impl Timeline {
         }
         let (desc, path) = delta_layer_writer.finish(deltas.key_range.end, ctx).await?;
         let delta_layer = Layer::finish_creating(self.conf, self, desc, &path)?;
-        info!("force created delta layer {}", delta_layer.local_path());
+
         {
             let mut guard = self.layers.write().await;
             guard.open_mut().unwrap().force_insert_layer(delta_layer);
@@ -5482,19 +5505,24 @@ impl Timeline {
         lsn: Lsn,
         ctx: &RequestContext,
     ) -> anyhow::Result<Vec<(Key, Bytes)>> {
+        use super::storage_layer::{LayerVisitBuilder, LayerVisitBuilderUpdate};
+
         let mut all_data = Vec::new();
         let guard = self.layers.read().await;
         for layer in guard.layer_map()?.iter_historic_layers() {
             if !layer.is_delta() && layer.image_layer_lsn() == lsn {
                 let layer = guard.get_from_desc(&layer);
                 let mut reconstruct_data = ValuesReconstructState::default();
+
+                let (layer, visit) = LayerVisitBuilder::new(
+                    ReadableLayer::PersistentLayer(layer),
+                    KeySpace::single(Key::MIN..Key::MAX),
+                    lsn..Lsn(lsn.0 + 1),
+                )
+                .build();
+
                 layer
-                    .get_values_reconstruct_data(
-                        KeySpace::single(Key::MIN..Key::MAX),
-                        lsn..Lsn(lsn.0 + 1),
-                        &mut reconstruct_data,
-                        ctx,
-                    )
+                    .get_values_reconstruct_data(visit, &mut reconstruct_data, ctx)
                     .await?;
                 for (k, v) in reconstruct_data.keys {
                     all_data.push((k, v?.img.unwrap().1));

pageserver/src/tenant/timeline/compaction.rs

@@ -29,7 +29,6 @@ use utils::id::TimelineId;
 
 use crate::context::{AccessStatsBehavior, RequestContext, RequestContextBuilder};
 use crate::page_cache;
-use crate::tenant::checks::check_valid_layermap;
 use crate::tenant::remote_timeline_client::WaitCompletionError;
 use crate::tenant::storage_layer::merge_iterator::MergeIterator;
 use crate::tenant::storage_layer::split_writer::{
@@ -1789,12 +1788,20 @@ impl Timeline {
                 stat.visit_image_layer(desc.file_size());
             }
         }
-        let layer_names: Vec<crate::tenant::storage_layer::LayerName> = layer_selection
-            .iter()
-            .map(|layer| layer.layer_desc().layer_name())
-            .collect_vec();
-        if let Some(err) = check_valid_layermap(&layer_names) {
-            bail!("cannot run gc-compaction because {}", err);
+        for layer in &layer_selection {
+            let desc = layer.layer_desc();
+            let key_range = &desc.key_range;
+            if desc.is_delta() && key_range.start.next() != key_range.end {
+                let lsn_range = desc.lsn_range.clone();
+                let intersects = lsn_split_point.range(lsn_range).collect_vec();
+                if intersects.len() > 1 {
+                    bail!(
+                        "cannot run gc-compaction because it violates the layer map LSN split assumption: layer {} intersects with LSN [{}]",
+                        desc.key(),
+                        intersects.into_iter().map(|lsn| lsn.to_string()).join(", ")
+                    );
+                }
+            }
         }
         // The maximum LSN we are processing in this compaction loop
         let end_lsn = layer_selection

pageserver/src/tenant/timeline/delete.rs

@@ -135,6 +135,25 @@ async fn delete_remote_layers_and_index(timeline: &Timeline) -> anyhow::Result<(
         .context("delete_all")
 }
 
+// This function removs remaining traces of a timeline on disk.
+// Namely: metadata file, timeline directory, delete mark.
+// Note: io::ErrorKind::NotFound are ignored for metadata and timeline dir.
+// delete mark should be present because it is the last step during deletion.
+// (nothing can fail after its deletion)
+async fn cleanup_remaining_timeline_fs_traces(
+    conf: &PageServerConf,
+    tenant_shard_id: TenantShardId,
+    timeline_id: TimelineId,
+) -> anyhow::Result<()> {
+    // Remove delete mark
+    // TODO: once we are confident that no more exist in the field, remove this
+    // line.  It cleans up a legacy marker file that might in rare cases be present.
+    tokio::fs::remove_file(conf.timeline_delete_mark_file_path(tenant_shard_id, timeline_id))
+        .await
+        .or_else(fs_ext::ignore_not_found)
+        .context("remove delete mark")
+}
+
 /// It is important that this gets called when DeletionGuard is being held.
 /// For more context see comments in [`DeleteTimelineFlow::prepare`]
 async fn remove_timeline_from_tenant(
@@ -175,10 +194,12 @@ async fn remove_timeline_from_tenant(
 /// 7. Delete mark file
 ///
 /// It is resumable from any step in case a crash/restart occurs.
-/// There are two entrypoints to the process:
+/// There are three entrypoints to the process:
 /// 1. [`DeleteTimelineFlow::run`] this is the main one called by a management api handler.
 /// 2. [`DeleteTimelineFlow::resume_deletion`] is called during restarts when local metadata is still present
 ///    and we possibly neeed to continue deletion of remote files.
+/// 3. [`DeleteTimelineFlow::cleanup_remaining_timeline_fs_traces`] is used when we deleted remote
+///    index but still have local metadata, timeline directory and delete mark.
 ///
 /// Note the only other place that messes around timeline delete mark is the logic that scans directory with timelines during tenant load.
 #[derive(Default)]
@@ -290,6 +311,18 @@ impl DeleteTimelineFlow {
         Ok(())
     }
 
+    #[instrument(skip_all, fields(%timeline_id))]
+    pub async fn cleanup_remaining_timeline_fs_traces(
+        tenant: &Tenant,
+        timeline_id: TimelineId,
+    ) -> anyhow::Result<()> {
+        let r =
+            cleanup_remaining_timeline_fs_traces(tenant.conf, tenant.tenant_shard_id, timeline_id)
+                .await;
+        info!("Done");
+        r
+    }
+
     fn prepare(
         tenant: &Tenant,
         timeline_id: TimelineId,

pgxn/neon/Makefile

@@ -23,7 +23,7 @@ SHLIB_LINK_INTERNAL = $(libpq)
 SHLIB_LINK = -lcurl
 
 EXTENSION = neon
-DATA = neon--1.0.sql neon--1.0--1.1.sql neon--1.1--1.2.sql neon--1.2--1.3.sql neon--1.3--1.2.sql neon--1.2--1.1.sql neon--1.1--1.0.sql  neon--1.3--1.4.sql neon--1.4--1.3.sql neon--1.4--1.5.sql neon--1.5--1.4.sql
+DATA = neon--1.0.sql neon--1.0--1.1.sql neon--1.1--1.2.sql neon--1.2--1.3.sql neon--1.3--1.2.sql neon--1.2--1.1.sql neon--1.1--1.0.sql  neon--1.3--1.4.sql neon--1.4--1.3.sql
 PGFILEDESC = "neon - cloud storage for PostgreSQL"
 
 EXTRA_CLEAN = \

pgxn/neon/file_cache.c

@@ -1263,7 +1263,7 @@ approximate_working_set_size_seconds(PG_FUNCTION_ARGS)
 		int32 dc;
 		time_t duration = PG_ARGISNULL(0) ? (time_t)-1 : PG_GETARG_INT32(0);
 		LWLockAcquire(lfc_lock, LW_SHARED);
-		dc = (int32) estimateSHLL(&lfc_ctl->wss_estimation, duration, 1.0);
+		dc = (int32) estimateSHLL(&lfc_ctl->wss_estimation, duration);
 		LWLockRelease(lfc_lock);
 		PG_RETURN_INT32(dc);
 	}
@@ -1280,7 +1280,7 @@ approximate_working_set_size(PG_FUNCTION_ARGS)
 		int32 dc;
 		bool reset = PG_GETARG_BOOL(0);
 		LWLockAcquire(lfc_lock, reset ? LW_EXCLUSIVE : LW_SHARED);
-		dc = (int32) estimateSHLL(&lfc_ctl->wss_estimation, (time_t)-1, 1.0);
+		dc = (int32) estimateSHLL(&lfc_ctl->wss_estimation, (time_t)-1);
 		if (reset)
 			memset(lfc_ctl->wss_estimation.regs, 0, sizeof lfc_ctl->wss_estimation.regs);
 		LWLockRelease(lfc_lock);
@@ -1288,21 +1288,3 @@ approximate_working_set_size(PG_FUNCTION_ARGS)
 	}
 	PG_RETURN_NULL();
 }
-
-PG_FUNCTION_INFO_V1(approximate_optimal_cache_size);
-
-Datum
-approximate_optimal_cache_size(PG_FUNCTION_ARGS)
-{
-	if (lfc_size_limit != 0)
-	{
-		int32 dc;
-		time_t duration = PG_ARGISNULL(0) ? (time_t)-1 : PG_GETARG_INT32(0);
-		double min_hit_ratio = PG_ARGISNULL(1) ? 1.0 : PG_GETARG_FLOAT8(1);
-		LWLockAcquire(lfc_lock, LW_SHARED);
-		dc = (int32) estimateSHLL(&lfc_ctl->wss_estimation, duration, min_hit_ratio);
-		LWLockRelease(lfc_lock);
-		PG_RETURN_INT32(dc);
-	}
-	PG_RETURN_NULL();
-}

pgxn/neon/hll.c

@@ -6,7 +6,7 @@
  * Portions Copyright (c) 2014-2023, PostgreSQL Global Development Group
  *
  * Implements https://hal.science/hal-00465313/document
- *
+ * 
  * Based on Hideaki Ohno's C++ implementation.  This is probably not ideally
  * suited to estimating the cardinality of very large sets;  in particular, we
  * have not attempted to further optimize the implementation as described in
@@ -126,78 +126,22 @@ addSHLL(HyperLogLogState *cState, uint32 hash)
 	/* Compute the rank of the remaining 32 - "k" (registerWidth) bits */
 	count = rho(hash << HLL_BIT_WIDTH, HLL_C_BITS);
 
-	if (cState->regs[index][count].ts)
-	{
-		/* update histgoram */
-		int64_t delta = (now - cState->regs[index][count].ts)/USECS_PER_SEC;
-		uint32_t new_histogram[HIST_SIZE] = {0};
-		for (int i = 0; i < HIST_SIZE; i++) {
-			/* Use middle point of interval */
-			uint32 interval_log2 = pg_ceil_log2_32((delta + (HIST_MIN_INTERVAL*((1<<i) + ((1<<i)/2))/2)) / HIST_MIN_INTERVAL);
-			uint32 cell = Min(interval_log2, HIST_SIZE-1);
-			new_histogram[cell] += cState->regs[index][count].histogram[i];
-		}
-		memcpy(cState->regs[index][count].histogram, new_histogram, sizeof new_histogram);
-	}
-	cState->regs[index][count].ts = now;
-	cState->regs[index][count].histogram[0] += 1; // most recent access always goes to first histogram backet
-}
-
-static uint32_t
-getAccessCount(const HyperLogLogRegister* reg, time_t duration)
-{
-	uint32_t count = 0;
-//  Simplest solution is to take in account all points fro overlapped interval
-//	for (size_t i = 0; i < HIST_SIZE && HIST_MIN_INTERVAL*((1 << i)/2) <= duration; i++) {
-	for (size_t i = 0; i < HIST_SIZE; i++) {
-		uint32_t high_boundary = HIST_MIN_INTERVAL*(1 << i);
-		uint32_t low_boundary = HIST_MIN_INTERVAL*((1 << i)/2);
-		if (high_boundary >= duration) {
-			// Assume uniform distribution of points within interval and use proportional number of points
-			Assert(duration >= low_boundary);
-			count += reg->histogram[i] * (duration - low_boundary) / (high_boundary - low_boundary);
-			break; // it's last interval within specified time range
-		} else {
-			count += reg->histogram[i];
-		}
-	}
-	return count;
+	cState->regs[index][count] = now;
 }
 
 static uint8
-getMaximum(const HyperLogLogRegister* reg, TimestampTz since, time_t duration, double min_hit_ratio)
+getMaximum(const TimestampTz* reg, TimestampTz since)
 {
 	uint8 max = 0;
-	size_t i, j;
-	if (min_hit_ratio == 1.0)
+
+	for (size_t i = 0; i < HLL_C_BITS + 1; i++)
 	{
-		for (i = 0; i < HLL_C_BITS + 1; i++)
+		if (reg[i] >= since)
 		{
-			if (reg[i].ts >= since)
-			{
-				max = i;
-			}
-		}
-	}
-	else
-	{
-		uint32_t total_count = 0;
-		for (i = 0; i < HLL_C_BITS + 1; i++)
-		{
-			total_count += getAccessCount(&reg[i], duration);
-		}
-		if (total_count != 0)
-		{
-			for (i = 0; i < HLL_C_BITS + 1; i++)
-			{
-				// Take in account only bits with access frequncy exceeding maximal miss rate (1 - hit rate)
-				if (reg[i].ts >= since && 1.0 - (double)getAccessCount(&reg[i], duration) / total_count <= min_hit_ratio)
-				{
-					max = i;
-				}
-			}
+			max = i;
 		}
 	}
+
 	return max;
 }
 
@@ -206,7 +150,7 @@ getMaximum(const HyperLogLogRegister* reg, TimestampTz since, time_t duration, d
  * Estimates cardinality, based on elements added so far
  */
 double
-estimateSHLL(HyperLogLogState *cState, time_t duration, double min_hit_ratio)
+estimateSHLL(HyperLogLogState *cState, time_t duration)
 {
 	double		result;
 	double		sum = 0.0;
@@ -217,7 +161,7 @@ estimateSHLL(HyperLogLogState *cState, time_t duration, double min_hit_ratio)
 
 	for (i = 0; i < HLL_N_REGISTERS; i++)
 	{
-		R[i] = getMaximum(cState->regs[i], since, duration, min_hit_ratio);
+		R[i] = getMaximum(cState->regs[i], since);
 		sum += 1.0 / pow(2.0, R[i]);
 	}
 

pgxn/neon/hll.h

@@ -53,14 +53,6 @@
 #define HLL_C_BITS      (32 - HLL_BIT_WIDTH)
 #define HLL_N_REGISTERS (1 << HLL_BIT_WIDTH)
 
-/*
- * Number of histogram cells. We use exponential histogram with first interval
- * equals to one minutes. Autoscaler request LFC  statistic with intervals 1,2,...,60 minutes
- * so 2^8=64 seems to be enough for our needs.
- */
-#define HIST_SIZE         8
-#define HIST_MIN_INTERVAL 60 /* seconds */
-
 /*
  * HyperLogLog is an approximate technique for computing the number of distinct
  * entries in a set.  Importantly, it does this by using a fixed amount of
@@ -77,21 +69,18 @@
  * modified timestamp >= the query timestamp. This value is the number of bits
  * for this register in the normal HLL calculation.
  *
- * The memory usage is 2^B * (C + 1) * sizeof(HyperLogLogRegister), or 920kiB.
+ * The memory usage is 2^B * (C + 1) * sizeof(TimetampTz), or 184kiB.
+ * Usage could be halved if we decide to reduce the required time dimension
+ * precision; as 32 bits in second precision should be enough for statistics.
+ * However, that is not yet implemented.
  */
-typedef struct
-{
-	TimestampTz ts; /* last access timestamp */
-	uint32_t    histogram[HIST_SIZE]; /* access counter exponential histogram */
-} HyperLogLogRegister;
-
 typedef struct HyperLogLogState
 {
-	HyperLogLogRegister regs[HLL_N_REGISTERS][HLL_C_BITS + 1];
+	TimestampTz regs[HLL_N_REGISTERS][HLL_C_BITS + 1];
 } HyperLogLogState;
 
 extern void   initSHLL(HyperLogLogState *cState);
 extern void   addSHLL(HyperLogLogState *cState, uint32 hash);
-extern double estimateSHLL(HyperLogLogState *cState, time_t dutration, double min_hit_ratio);
+extern double estimateSHLL(HyperLogLogState *cState, time_t dutration);
 
 #endif

pgxn/neon/neon--1.4--1.5.sql

@@ -1,10 +0,0 @@
-\echo Use "ALTER EXTENSION neon UPDATE TO '1.5'" to load this file. \quit
-
--- returns minimal LFC cache size (in 8kb pages) provided specified hit rate
-CREATE FUNCTION approximate_optimal_cache_size(duration_sec integer default null, min_hit_ration float8 default null)
-RETURNS integer
-AS 'MODULE_PATHNAME', 'approximate_optimal_cache_size'
-LANGUAGE C PARALLEL SAFE;
-
-GRANT EXECUTE ON FUNCTION approximate_optimal_cache_size(integer,float8) TO pg_monitor;
-

pgxn/neon/neon--1.5--1.4.sql

@@ -1 +0,0 @@
-DROP FUNCTION IF EXISTS approximate_optimal_cache_size(integer,float8) CASCADE;

proxy/src/auth/backend.rs

@@ -163,7 +163,6 @@ impl ComputeUserInfo {
 }
 
 pub(crate) enum ComputeCredentialKeys {
-    #[cfg(any(test, feature = "testing"))]
     Password(Vec<u8>),
     AuthKeys(AuthKeys),
     None,
@@ -294,10 +293,16 @@ async fn auth_quirks(
     // We now expect to see a very specific payload in the place of password.
     let (info, unauthenticated_password) = match user_info.try_into() {
         Err(info) => {
-            let (info, password) =
-                hacks::password_hack_no_authentication(ctx, info, client).await?;
-            ctx.set_endpoint_id(info.endpoint.clone());
-            (info, Some(password))
+            let res = hacks::password_hack_no_authentication(ctx, info, client).await?;
+
+            ctx.set_endpoint_id(res.info.endpoint.clone());
+            let password = match res.keys {
+                ComputeCredentialKeys::Password(p) => p,
+                ComputeCredentialKeys::AuthKeys(_) | ComputeCredentialKeys::None => {
+                    unreachable!("password hack should return a password")
+                }
+            };
+            (res.info, Some(password))
         }
         Ok(info) => (info, None),
     };

proxy/src/auth/backend/hacks.rs

@@ -1,4 +1,6 @@
-use super::{ComputeCredentials, ComputeUserInfo, ComputeUserInfoNoEndpoint};
+use super::{
+    ComputeCredentialKeys, ComputeCredentials, ComputeUserInfo, ComputeUserInfoNoEndpoint,
+};
 use crate::{
     auth::{self, AuthFlow},
     config::AuthenticationConfig,
@@ -61,7 +63,7 @@ pub(crate) async fn password_hack_no_authentication(
     ctx: &RequestMonitoring,
     info: ComputeUserInfoNoEndpoint,
     client: &mut stream::PqStream<Stream<impl AsyncRead + AsyncWrite + Unpin>>,
-) -> auth::Result<(ComputeUserInfo, Vec<u8>)> {
+) -> auth::Result<ComputeCredentials> {
     warn!("project not specified, resorting to the password hack auth flow");
     ctx.set_auth_method(crate::context::AuthMethod::Cleartext);
 
@@ -77,12 +79,12 @@ pub(crate) async fn password_hack_no_authentication(
     info!(project = &*payload.endpoint, "received missing parameter");
 
     // Report tentative success; compute node will check the password anyway.
-    Ok((
-        ComputeUserInfo {
+    Ok(ComputeCredentials {
+        info: ComputeUserInfo {
             user: info.user,
             options: info.options,
             endpoint: payload.endpoint,
         },
-        payload.password,
-    ))
+        keys: ComputeCredentialKeys::Password(payload.password),
+    })
 }

proxy/src/auth/backend/jwt.rs

@@ -25,8 +25,6 @@ const MAX_JWK_BODY_SIZE: usize = 64 * 1024;
 pub(crate) trait FetchAuthRules: Clone + Send + Sync + 'static {
     fn fetch_auth_rules(
         &self,
-        ctx: &RequestMonitoring,
-        endpoint: EndpointId,
         role_name: RoleName,
     ) -> impl Future<Output = anyhow::Result<Vec<AuthRule>>> + Send;
 }
@@ -103,9 +101,7 @@ impl JwkCacheEntryLock {
     async fn renew_jwks<F: FetchAuthRules>(
         &self,
         _permit: JwkRenewalPermit<'_>,
-        ctx: &RequestMonitoring,
         client: &reqwest::Client,
-        endpoint: EndpointId,
         role_name: RoleName,
         auth_rules: &F,
     ) -> anyhow::Result<Arc<JwkCacheEntry>> {
@@ -119,9 +115,7 @@ impl JwkCacheEntryLock {
             }
         }
 
-        let rules = auth_rules
-            .fetch_auth_rules(ctx, endpoint, role_name)
-            .await?;
+        let rules = auth_rules.fetch_auth_rules(role_name).await?;
         let mut key_sets =
             ahash::HashMap::with_capacity_and_hasher(rules.len(), ahash::RandomState::new());
         // TODO(conrad): run concurrently
@@ -172,7 +166,6 @@ impl JwkCacheEntryLock {
         self: &Arc<Self>,
         ctx: &RequestMonitoring,
         client: &reqwest::Client,
-        endpoint: EndpointId,
         role_name: RoleName,
         fetch: &F,
     ) -> Result<Arc<JwkCacheEntry>, anyhow::Error> {
@@ -183,9 +176,7 @@ impl JwkCacheEntryLock {
         let Some(cached) = guard else {
             let _paused = ctx.latency_timer_pause(crate::metrics::Waiting::Compute);
             let permit = self.acquire_permit().await;
-            return self
-                .renew_jwks(permit, ctx, client, endpoint, role_name, fetch)
-                .await;
+            return self.renew_jwks(permit, client, role_name, fetch).await;
         };
 
         let last_update = now.duration_since(cached.last_retrieved);
@@ -196,9 +187,7 @@ impl JwkCacheEntryLock {
             let permit = self.acquire_permit().await;
 
             // it's been too long since we checked the keys. wait for them to update.
-            return self
-                .renew_jwks(permit, ctx, client, endpoint, role_name, fetch)
-                .await;
+            return self.renew_jwks(permit, client, role_name, fetch).await;
         }
 
         // every 5 minutes we should spawn a job to eagerly update the token.
@@ -209,12 +198,8 @@ impl JwkCacheEntryLock {
                 let entry = self.clone();
                 let client = client.clone();
                 let fetch = fetch.clone();
-                let ctx = ctx.clone();
                 tokio::spawn(async move {
-                    if let Err(e) = entry
-                        .renew_jwks(permit, &ctx, &client, endpoint, role_name, &fetch)
-                        .await
-                    {
+                    if let Err(e) = entry.renew_jwks(permit, &client, role_name, &fetch).await {
                         tracing::warn!(error=?e, "could not fetch JWKs in background job");
                     }
                 });
@@ -231,7 +216,6 @@ impl JwkCacheEntryLock {
         ctx: &RequestMonitoring,
         jwt: &str,
         client: &reqwest::Client,
-        endpoint: EndpointId,
         role_name: RoleName,
         fetch: &F,
     ) -> Result<(), anyhow::Error> {
@@ -258,7 +242,7 @@ impl JwkCacheEntryLock {
         let kid = header.key_id.context("missing key id")?;
 
         let mut guard = self
-            .get_or_update_jwk_cache(ctx, client, endpoint.clone(), role_name.clone(), fetch)
+            .get_or_update_jwk_cache(ctx, client, role_name.clone(), fetch)
             .await?;
 
         // get the key from the JWKs if possible. If not, wait for the keys to update.
@@ -270,14 +254,7 @@ impl JwkCacheEntryLock {
 
                     let permit = self.acquire_permit().await;
                     guard = self
-                        .renew_jwks(
-                            permit,
-                            ctx,
-                            client,
-                            endpoint.clone(),
-                            role_name.clone(),
-                            fetch,
-                        )
+                        .renew_jwks(permit, client, role_name.clone(), fetch)
                         .await?;
                 }
                 _ => {
@@ -341,7 +318,7 @@ impl JwkCache {
         jwt: &str,
     ) -> Result<(), anyhow::Error> {
         // try with just a read lock first
-        let key = (endpoint.clone(), role_name.clone());
+        let key = (endpoint, role_name.clone());
         let entry = self.map.get(&key).as_deref().map(Arc::clone);
         let entry = entry.unwrap_or_else(|| {
             // acquire a write lock after to insert.
@@ -350,7 +327,7 @@ impl JwkCache {
         });
 
         entry
-            .check_jwt(ctx, jwt, &self.client, endpoint, role_name, fetch)
+            .check_jwt(ctx, jwt, &self.client, role_name, fetch)
             .await
     }
 }
@@ -711,8 +688,6 @@ X0n5X2/pBLJzxZc62ccvZYVnctBiFs6HbSnxpuMQCfkt/BcR/ttIepBQQIW86wHL
         impl FetchAuthRules for Fetch {
             async fn fetch_auth_rules(
                 &self,
-                _ctx: &RequestMonitoring,
-                _endpoint: EndpointId,
                 _role_name: RoleName,
             ) -> anyhow::Result<Vec<AuthRule>> {
                 Ok(vec![
@@ -731,7 +706,6 @@ X0n5X2/pBLJzxZc62ccvZYVnctBiFs6HbSnxpuMQCfkt/BcR/ttIepBQQIW86wHL
         }
 
         let role_name = RoleName::from("user");
-        let endpoint = EndpointId::from("ep");
 
         let jwk_cache = Arc::new(JwkCacheEntryLock::default());
 
@@ -741,7 +715,6 @@ X0n5X2/pBLJzxZc62ccvZYVnctBiFs6HbSnxpuMQCfkt/BcR/ttIepBQQIW86wHL
                     &RequestMonitoring::test(),
                     &token,
                     &client,
-                    endpoint.clone(),
                     role_name.clone(),
                     &Fetch(addr),
                 )

proxy/src/auth/backend/local.rs

@@ -9,9 +9,8 @@ use crate::{
         messages::{ColdStartInfo, EndpointJwksResponse, MetricsAuxInfo},
         NodeInfo,
     },
-    context::RequestMonitoring,
     intern::{BranchIdInt, BranchIdTag, EndpointIdTag, InternId, ProjectIdInt, ProjectIdTag},
-    EndpointId, RoleName,
+    RoleName,
 };
 
 use super::jwt::{AuthRule, FetchAuthRules, JwkCache};
@@ -58,12 +57,7 @@ pub struct JwksRoleSettings {
 }
 
 impl FetchAuthRules for StaticAuthRules {
-    async fn fetch_auth_rules(
-        &self,
-        _ctx: &RequestMonitoring,
-        _endpoint: EndpointId,
-        role_name: RoleName,
-    ) -> anyhow::Result<Vec<AuthRule>> {
+    async fn fetch_auth_rules(&self, role_name: RoleName) -> anyhow::Result<Vec<AuthRule>> {
         let mappings = JWKS_ROLE_MAP.load();
         let role_mappings = mappings
             .as_deref()

proxy/src/bin/proxy.rs

@@ -62,13 +62,12 @@ static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
 #[derive(Clone, Debug, ValueEnum)]
 enum AuthBackendType {
     Console,
+    #[cfg(feature = "testing")]
+    Postgres,
     // clap only shows the name, not the alias, in usage text.
     // TODO: swap name/alias and deprecate "link"
     #[value(name("link"), alias("web"))]
     Web,
-
-    #[cfg(feature = "testing")]
-    Postgres,
 }
 
 /// Neon proxy/router
@@ -640,19 +639,17 @@ fn build_config(args: &ProxyCliArgs) -> anyhow::Result<&'static ProxyConfig> {
             let api = console::provider::ConsoleBackend::Console(api);
             auth::Backend::Console(MaybeOwned::Owned(api), ())
         }
-
-        AuthBackendType::Web => {
-            let url = args.uri.parse()?;
-            auth::Backend::Web(MaybeOwned::Owned(url), ())
-        }
-
         #[cfg(feature = "testing")]
         AuthBackendType::Postgres => {
             let url = args.auth_endpoint.parse()?;
-            let api = console::provider::mock::Api::new(url, !args.is_private_access_proxy);
+            let api = console::provider::mock::Api::new(url);
             let api = console::provider::ConsoleBackend::Postgres(api);
             auth::Backend::Console(MaybeOwned::Owned(api), ())
         }
+        AuthBackendType::Web => {
+            let url = args.uri.parse()?;
+            auth::Backend::Web(MaybeOwned::Owned(url), ())
+        }
     };
 
     let config::ConcurrencyLockOptions {

proxy/src/console/provider.rs

@@ -303,7 +303,6 @@ impl NodeInfo {
 
     pub(crate) fn set_keys(&mut self, keys: &ComputeCredentialKeys) {
         match keys {
-            #[cfg(any(test, feature = "testing"))]
             ComputeCredentialKeys::Password(password) => self.config.password(password),
             ComputeCredentialKeys::AuthKeys(auth_keys) => self.config.auth_keys(*auth_keys),
             ComputeCredentialKeys::None => &mut self.config,

proxy/src/console/provider/mock.rs

@@ -41,15 +41,11 @@ impl From<tokio_postgres::Error> for ApiError {
 #[derive(Clone)]
 pub struct Api {
     endpoint: ApiUrl,
-    ip_allowlist_check_enabled: bool,
 }
 
 impl Api {
-    pub fn new(endpoint: ApiUrl, ip_allowlist_check_enabled: bool) -> Self {
-        Self {
-            endpoint,
-            ip_allowlist_check_enabled,
-        }
+    pub fn new(endpoint: ApiUrl) -> Self {
+        Self { endpoint }
     }
 
     pub(crate) fn url(&self) -> &str {
@@ -68,7 +64,6 @@ impl Api {
                 tokio_postgres::connect(self.endpoint.as_str(), tokio_postgres::NoTls).await?;
 
             tokio::spawn(connection);
-
             let secret = if let Some(entry) = get_execute_postgres_query(
                 &client,
                 "select rolpassword from pg_catalog.pg_authid where rolname = $1",
@@ -84,26 +79,21 @@ impl Api {
                 warn!("user '{}' does not exist", user_info.user);
                 None
             };
-
-            let allowed_ips = if self.ip_allowlist_check_enabled {
-                match get_execute_postgres_query(
-                    &client,
-                    "select allowed_ips from neon_control_plane.endpoints where endpoint_id = $1",
-                    &[&user_info.endpoint.as_str()],
-                    "allowed_ips",
-                )
-                .await?
-                {
-                    Some(s) => {
-                        info!("got allowed_ips: {s}");
-                        s.split(',')
-                            .map(|s| IpPattern::from_str(s).unwrap())
-                            .collect()
-                    }
-                    None => vec![],
+            let allowed_ips = match get_execute_postgres_query(
+                &client,
+                "select allowed_ips from neon_control_plane.endpoints where endpoint_id = $1",
+                &[&user_info.endpoint.as_str()],
+                "allowed_ips",
+            )
+            .await?
+            {
+                Some(s) => {
+                    info!("got allowed_ips: {s}");
+                    s.split(',')
+                        .map(|s| IpPattern::from_str(s).unwrap())
+                        .collect()
                 }
-            } else {
-                vec![]
+                None => vec![],
             };
 
             Ok((secret, allowed_ips))

proxy/src/context.rs

@@ -79,40 +79,6 @@ pub(crate) enum AuthMethod {
     Cleartext,
 }
 
-impl Clone for RequestMonitoring {
-    fn clone(&self) -> Self {
-        let inner = self.0.try_lock().expect("should not deadlock");
-        let new = RequestMonitoringInner {
-            peer_addr: inner.peer_addr,
-            session_id: inner.session_id,
-            protocol: inner.protocol,
-            first_packet: inner.first_packet,
-            region: inner.region,
-            span: info_span!("background_task"),
-
-            project: inner.project,
-            branch: inner.branch,
-            endpoint_id: inner.endpoint_id.clone(),
-            dbname: inner.dbname.clone(),
-            user: inner.user.clone(),
-            application: inner.application.clone(),
-            error_kind: inner.error_kind,
-            auth_method: inner.auth_method.clone(),
-            success: inner.success,
-            rejected: inner.rejected,
-            cold_start_info: inner.cold_start_info,
-            pg_options: inner.pg_options.clone(),
-
-            sender: None,
-            disconnect_sender: None,
-            latency_timer: LatencyTimer::noop(inner.protocol),
-            disconnect_timestamp: inner.disconnect_timestamp,
-        };
-
-        Self(TryLock::new(new))
-    }
-}
-
 impl RequestMonitoring {
     pub fn new(
         session_id: Uuid,

proxy/src/metrics.rs

@@ -397,8 +397,6 @@ pub struct LatencyTimer {
     protocol: Protocol,
     cold_start_info: ColdStartInfo,
     outcome: ConnectOutcome,
-
-    skip_reporting: bool,
 }
 
 impl LatencyTimer {
@@ -411,20 +409,6 @@ impl LatencyTimer {
             cold_start_info: ColdStartInfo::Unknown,
             // assume failed unless otherwise specified
             outcome: ConnectOutcome::Failed,
-            skip_reporting: false,
-        }
-    }
-
-    pub(crate) fn noop(protocol: Protocol) -> Self {
-        Self {
-            start: time::Instant::now(),
-            stop: None,
-            accumulated: Accumulated::default(),
-            protocol,
-            cold_start_info: ColdStartInfo::Unknown,
-            // assume failed unless otherwise specified
-            outcome: ConnectOutcome::Failed,
-            skip_reporting: true,
         }
     }
 
@@ -459,10 +443,6 @@ pub enum ConnectOutcome {
 
 impl Drop for LatencyTimer {
     fn drop(&mut self) {
-        if self.skip_reporting {
-            return;
-        }
-
         let duration = self
             .stop
             .unwrap_or_else(time::Instant::now)

proxy/src/serverless/backend.rs

@@ -27,7 +27,7 @@ use crate::{
     Host,
 };
 
-use super::conn_pool::{poll_client, Client, ConnInfo, GlobalConnPool};
+use super::conn_pool::{poll_client, AuthData, Client, ConnInfo, GlobalConnPool};
 
 pub(crate) struct PoolingBackend {
     pub(crate) pool: Arc<GlobalConnPool<tokio_postgres::Client>>,
@@ -274,6 +274,13 @@ impl ConnectMechanism for TokioMechanism {
             .dbname(&self.conn_info.dbname)
             .connect_timeout(timeout);
 
+        match &self.conn_info.auth {
+            AuthData::Jwt(_) => {}
+            AuthData::Password(pw) => {
+                config.password(pw);
+            }
+        }
+
         let pause = ctx.latency_timer_pause(crate::metrics::Waiting::Compute);
         let res = config.connect(tokio_postgres::NoTls).await;
         drop(pause);

proxy/src/serverless/conn_pool.rs

@@ -29,16 +29,11 @@ use tracing::{info, info_span, Instrument};
 
 use super::backend::HttpConnError;
 
-#[derive(Debug, Clone)]
-pub(crate) struct ConnInfoWithAuth {
-    pub(crate) conn_info: ConnInfo,
-    pub(crate) auth: AuthData,
-}
-
 #[derive(Debug, Clone)]
 pub(crate) struct ConnInfo {
     pub(crate) user_info: ComputeUserInfo,
     pub(crate) dbname: DbName,
+    pub(crate) auth: AuthData,
 }
 
 #[derive(Debug, Clone)]
@@ -792,6 +787,7 @@ mod tests {
                 options: NeonOptions::default(),
             },
             dbname: "dbname".into(),
+            auth: AuthData::Password("password".as_bytes().into()),
         };
         let ep_pool = Arc::downgrade(
             &pool.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key().unwrap()),
@@ -849,6 +845,7 @@ mod tests {
                 options: NeonOptions::default(),
             },
             dbname: "dbname".into(),
+            auth: AuthData::Password("password".as_bytes().into()),
         };
         let ep_pool = Arc::downgrade(
             &pool.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key().unwrap()),

proxy/src/serverless/sql_over_http.rs

@@ -60,7 +60,6 @@ use super::backend::PoolingBackend;
 use super::conn_pool::AuthData;
 use super::conn_pool::Client;
 use super::conn_pool::ConnInfo;
-use super::conn_pool::ConnInfoWithAuth;
 use super::http_util::json_response;
 use super::json::json_to_pg_text;
 use super::json::pg_text_row_to_json;
@@ -149,7 +148,7 @@ fn get_conn_info(
     ctx: &RequestMonitoring,
     headers: &HeaderMap,
     tls: Option<&TlsConfig>,
-) -> Result<ConnInfoWithAuth, ConnInfoError> {
+) -> Result<ConnInfo, ConnInfoError> {
     // HTTP only uses cleartext (for now and likely always)
     ctx.set_auth_method(crate::context::AuthMethod::Cleartext);
 
@@ -236,8 +235,11 @@ fn get_conn_info(
         options: options.unwrap_or_default(),
     };
 
-    let conn_info = ConnInfo { user_info, dbname };
-    Ok(ConnInfoWithAuth { conn_info, auth })
+    Ok(ConnInfo {
+        user_info,
+        dbname,
+        auth,
+    })
 }
 
 // TODO: return different http error codes
@@ -521,10 +523,7 @@ async fn handle_inner(
 
     // TLS config should be there.
     let conn_info = get_conn_info(ctx, headers, config.tls_config.as_ref())?;
-    info!(
-        user = conn_info.conn_info.user_info.user.as_str(),
-        "credentials"
-    );
+    info!(user = conn_info.user_info.user.as_str(), "credentials");
 
     // Allow connection pooling only if explicitly requested
     // or if we have decided that http pool is no longer opt-in
@@ -569,20 +568,20 @@ async fn handle_inner(
                         .authenticate_with_password(
                             ctx,
                             &config.authentication_config,
-                            &conn_info.conn_info.user_info,
+                            &conn_info.user_info,
                             pw,
                         )
                         .await?
                 }
                 AuthData::Jwt(jwt) => {
                     backend
-                        .authenticate_with_jwt(ctx, &conn_info.conn_info.user_info, jwt)
+                        .authenticate_with_jwt(ctx, &conn_info.user_info, jwt)
                         .await?
                 }
             };
 
             let client = backend
-                .connect_to_compute(ctx, conn_info.conn_info, keys, !allow_pool)
+                .connect_to_compute(ctx, conn_info, keys, !allow_pool)
                 .await?;
             // not strictly necessary to mark success here,
             // but it's just insurance for if we forget it somewhere else

safekeeper/src/debug_dump.rs

@@ -17,7 +17,6 @@ use postgres_ffi::MAX_SEND_SIZE;
 use serde::Deserialize;
 use serde::Serialize;
 
-use postgres_ffi::v14::xlog_utils::{IsPartialXLogFileName, IsXLogFileName};
 use sha2::{Digest, Sha256};
 use utils::id::NodeId;
 use utils::id::TenantTimelineId;
@@ -52,9 +51,6 @@ pub struct Args {
     /// Dump full term history. True by default.
     pub dump_term_history: bool,
 
-    /// Dump last modified time of WAL segments. Uses value of `dump_all` by default.
-    pub dump_wal_last_modified: bool,
-
     /// Filter timelines by tenant_id.
     pub tenant_id: Option<TenantId>,
 
@@ -132,19 +128,12 @@ async fn build_from_tli_dump(
         None
     };
 
-    let wal_last_modified = if args.dump_wal_last_modified {
-        get_wal_last_modified(timeline_dir).ok().flatten()
-    } else {
-        None
-    };
-
     Timeline {
         tenant_id: timeline.ttid.tenant_id,
         timeline_id: timeline.ttid.timeline_id,
         control_file,
         memory,
         disk_content,
-        wal_last_modified,
     }
 }
 
@@ -167,7 +156,6 @@ pub struct Timeline {
     pub control_file: Option<TimelinePersistentState>,
     pub memory: Option<Memory>,
     pub disk_content: Option<DiskContent>,
-    pub wal_last_modified: Option<DateTime<Utc>>,
 }
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -314,27 +302,6 @@ fn build_file_info(entry: DirEntry) -> Result<FileInfo> {
     })
 }
 
-/// Get highest modified time of WAL segments in the directory.
-fn get_wal_last_modified(path: &Utf8Path) -> Result<Option<DateTime<Utc>>> {
-    let mut res = None;
-    for entry in fs::read_dir(path)? {
-        if entry.is_err() {
-            continue;
-        }
-        let entry = entry?;
-        /* Ignore files that are not XLOG segments */
-        let fname = entry.file_name();
-        if !IsXLogFileName(&fname) && !IsPartialXLogFileName(&fname) {
-            continue;
-        }
-
-        let metadata = entry.metadata()?;
-        let modified: DateTime<Utc> = DateTime::from(metadata.modified()?);
-        res = std::cmp::max(res, Some(modified));
-    }
-    Ok(res)
-}
-
 /// Converts SafeKeeperConf to Config, filtering out the fields that are not
 /// supposed to be exposed.
 fn build_config(config: SafeKeeperConf) -> Config {

safekeeper/src/http/routes.rs

@@ -481,7 +481,6 @@ async fn dump_debug_handler(mut request: Request<Body>) -> Result<Response<Body>
     let mut dump_memory: Option<bool> = None;
     let mut dump_disk_content: Option<bool> = None;
     let mut dump_term_history: Option<bool> = None;
-    let mut dump_wal_last_modified: Option<bool> = None;
     let mut tenant_id: Option<TenantId> = None;
     let mut timeline_id: Option<TimelineId> = None;
 
@@ -495,7 +494,6 @@ async fn dump_debug_handler(mut request: Request<Body>) -> Result<Response<Body>
             "dump_memory" => dump_memory = Some(parse_kv_str(&k, &v)?),
             "dump_disk_content" => dump_disk_content = Some(parse_kv_str(&k, &v)?),
             "dump_term_history" => dump_term_history = Some(parse_kv_str(&k, &v)?),
-            "dump_wal_last_modified" => dump_wal_last_modified = Some(parse_kv_str(&k, &v)?),
             "tenant_id" => tenant_id = Some(parse_kv_str(&k, &v)?),
             "timeline_id" => timeline_id = Some(parse_kv_str(&k, &v)?),
             _ => Err(ApiError::BadRequest(anyhow::anyhow!(
@@ -510,7 +508,6 @@ async fn dump_debug_handler(mut request: Request<Body>) -> Result<Response<Body>
     let dump_memory = dump_memory.unwrap_or(dump_all);
     let dump_disk_content = dump_disk_content.unwrap_or(dump_all);
     let dump_term_history = dump_term_history.unwrap_or(true);
-    let dump_wal_last_modified = dump_wal_last_modified.unwrap_or(dump_all);
 
     let args = debug_dump::Args {
         dump_all,
@@ -518,7 +515,6 @@ async fn dump_debug_handler(mut request: Request<Body>) -> Result<Response<Body>
         dump_memory,
         dump_disk_content,
         dump_term_history,
-        dump_wal_last_modified,
         tenant_id,
         timeline_id,
     };

safekeeper/src/pull_timeline.rs

@@ -278,7 +278,7 @@ impl WalResidentTimeline {
 }
 
 /// pull_timeline request body.
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Serialize, Deserialize)]
 pub struct Request {
     pub tenant_id: TenantId,
     pub timeline_id: TimelineId,
@@ -293,7 +293,7 @@ pub struct Response {
 }
 
 /// Response for debug dump request.
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Serialize, Deserialize)]
 pub struct DebugDumpResponse {
     pub start_time: DateTime<Utc>,
     pub finish_time: DateTime<Utc>,

safekeeper/src/wal_storage.rs

@@ -539,17 +539,20 @@ async fn remove_segments_from_disk(
     while let Some(entry) = entries.next_entry().await? {
         let entry_path = entry.path();
         let fname = entry_path.file_name().unwrap();
-        /* Ignore files that are not XLOG segments */
-        if !IsXLogFileName(fname) && !IsPartialXLogFileName(fname) {
-            continue;
-        }
-        let (segno, _) = XLogFromFileName(fname, wal_seg_size)?;
-        if remove_predicate(segno) {
-            remove_file(entry_path).await?;
-            n_removed += 1;
-            min_removed = min(min_removed, segno);
-            max_removed = max(max_removed, segno);
-            REMOVED_WAL_SEGMENTS.inc();
+
+        if let Some(fname_str) = fname.to_str() {
+            /* Ignore files that are not XLOG segments */
+            if !IsXLogFileName(fname_str) && !IsPartialXLogFileName(fname_str) {
+                continue;
+            }
+            let (segno, _) = XLogFromFileName(fname_str, wal_seg_size);
+            if remove_predicate(segno) {
+                remove_file(entry_path).await?;
+                n_removed += 1;
+                min_removed = min(min_removed, segno);
+                max_removed = max(max_removed, segno);
+                REMOVED_WAL_SEGMENTS.inc();
+            }
         }
     }
 

storage_scrubber/src/checks.rs

@@ -1,8 +1,7 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap, HashSet};
 
 use anyhow::Context;
 use itertools::Itertools;
-use pageserver::tenant::checks::check_valid_layermap;
 use pageserver::tenant::layer_map::LayerMap;
 use pageserver::tenant::remote_timeline_client::index::LayerFileMetadata;
 use pageserver_api::shard::ShardIndex;
@@ -49,6 +48,56 @@ impl TimelineAnalysis {
     }
 }
 
+/// Checks whether a layer map is valid (i.e., is a valid result of the current compaction algorithm if nothing goes wrong).
+/// The function checks if we can split the LSN range of a delta layer only at the LSNs of the delta layers. For example,
+///
+/// ```plain
+/// |       |                 |       |
+/// |   1   |    |   2   |    |   3   |
+/// |       |    |       |    |       |
+/// ```
+///
+/// This is not a valid layer map because the LSN range of layer 1 intersects with the LSN range of layer 2. 1 and 2 should have
+/// the same LSN range.
+///
+/// The exception is that when layer 2 only contains a single key, it could be split over the LSN range. For example,
+///
+/// ```plain
+/// |       |    |   2   |    |       |
+/// |   1   |    |-------|    |   3   |
+/// |       |    |   4   |    |       |
+///
+/// If layer 2 and 4 contain the same single key, this is also a valid layer map.
+fn check_valid_layermap(metadata: &HashMap<LayerName, LayerFileMetadata>) -> Option<String> {
+    let mut lsn_split_point = BTreeSet::new(); // TODO: use a better data structure (range tree / range set?)
+    let mut all_delta_layers = Vec::new();
+    for (name, _) in metadata.iter() {
+        if let LayerName::Delta(layer) = name {
+            if layer.key_range.start.next() != layer.key_range.end {
+                all_delta_layers.push(layer.clone());
+            }
+        }
+    }
+    for layer in &all_delta_layers {
+        let lsn_range = &layer.lsn_range;
+        lsn_split_point.insert(lsn_range.start);
+        lsn_split_point.insert(lsn_range.end);
+    }
+    for layer in &all_delta_layers {
+        let lsn_range = layer.lsn_range.clone();
+        let intersects = lsn_split_point.range(lsn_range).collect_vec();
+        if intersects.len() > 1 {
+            let err = format!(
+                        "layer violates the layer map LSN split assumption: layer {} intersects with LSN [{}]",
+                        layer,
+                        intersects.into_iter().map(|lsn| lsn.to_string()).join(", ")
+                    );
+            return Some(err);
+        }
+    }
+    None
+}
+
 pub(crate) async fn branch_cleanup_and_check_errors(
     remote_client: &GenericRemoteStorage,
     id: &TenantShardTimelineId,
@@ -128,8 +177,7 @@ pub(crate) async fn branch_cleanup_and_check_errors(
                         }
                     }
 
-                    let layer_names = index_part.layer_metadata.keys().cloned().collect_vec();
-                    if let Some(err) = check_valid_layermap(&layer_names) {
+                    if let Some(err) = check_valid_layermap(&index_part.layer_metadata) {
                         result.errors.push(format!(
                             "index_part.json contains invalid layer map structure: {err}"
                         ));

storage_scrubber/src/main.rs

@@ -121,6 +121,8 @@ enum Command {
 async fn main() -> anyhow::Result<()> {
     let cli = Cli::parse();
 
+    tracing::info!("version: {}, build_tag {}", GIT_VERSION, BUILD_TAG);
+
     let bucket_config = BucketConfig::from_env()?;
 
     let command_log_name = match &cli.command {
@@ -140,8 +142,6 @@ async fn main() -> anyhow::Result<()> {
         chrono::Utc::now().format("%Y_%m_%d__%H_%M_%S")
     ));
 
-    tracing::info!("version: {}, build_tag {}", GIT_VERSION, BUILD_TAG);
-
     let controller_client = cli.controller_api.map(|controller_api| {
         ControllerClientConfig {
             controller_api,

test_runner/fixtures/neon_fixtures.py

@@ -3863,6 +3863,9 @@ def static_proxy(
     dbname = vanilla_pg.default_options["dbname"]
     auth_endpoint = f"postgres://proxy:password@{host}:{port}/{dbname}"
 
+    # require password for 'http_auth' user
+    vanilla_pg.edit_hba([f"host {dbname} http_auth {host} password"])
+
     # For simplicity, we use the same user for both `--auth-endpoint` and `safe_psql`
     vanilla_pg.start()
     vanilla_pg.safe_psql("create user proxy with login superuser password 'password'")

test_runner/regress/test_branch_and_gc.py

@@ -142,7 +142,6 @@ def test_branch_creation_before_gc(neon_simple_env: NeonEnv):
             "image_creation_threshold": "1",
             # set PITR interval to be small, so we can do GC
             "pitr_interval": "0 s",
-            "lsn_lease_length": "0s",
         }
     )
 

test_runner/regress/test_branch_behind.py

@@ -11,9 +11,7 @@ from fixtures.utils import print_gc_result, query_scalar
 #
 def test_branch_behind(neon_env_builder: NeonEnvBuilder):
     # Disable pitr, because here we want to test branch creation after GC
-    env = neon_env_builder.init_start(
-        initial_tenant_conf={"pitr_interval": "0 sec", "lsn_lease_length": "0s"}
-    )
+    env = neon_env_builder.init_start(initial_tenant_conf={"pitr_interval": "0 sec"})
 
     error_regexes = [
         ".*invalid branch start lsn.*",

test_runner/regress/test_branching.py

@@ -419,7 +419,7 @@ def test_duplicate_creation(neon_env_builder: NeonEnvBuilder):
 
 
 def test_branching_while_stuck_find_gc_cutoffs(neon_env_builder: NeonEnvBuilder):
-    env = neon_env_builder.init_start(initial_tenant_conf={"lsn_lease_length": "0s"})
+    env = neon_env_builder.init_start()
 
     client = env.pageserver.http_client()
 

test_runner/regress/test_compaction.py

@@ -240,7 +240,6 @@ def test_uploads_and_deletions(
         "image_creation_threshold": "1",
         "image_layer_creation_check_threshold": "0",
         "compaction_algorithm": json.dumps({"kind": compaction_algorithm.value}),
-        "lsn_lease_length": "0s",
     }
     env = neon_env_builder.init_start(initial_tenant_conf=tenant_conf)
 

test_runner/regress/test_hot_standby.py

@@ -222,7 +222,7 @@ def pgbench_accounts_initialized(ep):
 # Without hs feedback enabled we'd see 'User query might have needed to see row
 # versions that must be removed.' errors.
 def test_hot_standby_feedback(neon_env_builder: NeonEnvBuilder, pg_bin: PgBin):
-    env = neon_env_builder.init_start(initial_tenant_conf={"lsn_lease_length": "0s"})
+    env = neon_env_builder.init_start()
     agressive_vacuum_conf = [
         "log_autovacuum_min_duration = 0",
         "autovacuum_naptime = 10s",

test_runner/regress/test_layer_eviction.py

@@ -173,7 +173,6 @@ def test_gc_of_remote_layers(neon_env_builder: NeonEnvBuilder):
         # "image_creation_threshold": set at runtime
         "compaction_target_size": f"{128 * (1024**2)}",  # make it so that we only have 1 partition => image coverage for delta layers => enables gc of delta layers
         "image_layer_creation_check_threshold": "0",  # always check if a new image layer can be created
-        "lsn_lease_length": "0s",
     }
 
     def tenant_update_config(changes):

test_runner/regress/test_lfc_working_set_approximation.py

@@ -114,46 +114,3 @@ def test_sliding_working_set_approximation(neon_simple_env: NeonEnv):
 
     assert estimation_1k >= 20 and estimation_1k <= 40
     assert estimation_10k >= 200 and estimation_10k <= 400
-
-
-def test_optimal_cache_size_approximation(neon_simple_env: NeonEnv):
-    env = neon_simple_env
-
-    endpoint = env.endpoints.create_start(
-        branch_name="main",
-        config_lines=[
-            "autovacuum = off",
-            "shared_buffers=1MB",
-            "neon.max_file_cache_size=256MB",
-            "neon.file_cache_size_limit=245MB",
-        ],
-    )
-    conn = endpoint.connect()
-    cur = conn.cursor()
-    cur.execute("create extension neon version '1.5'")
-    cur.execute(
-        "create table t_huge(pk integer primary key, count integer default 0, payload text default repeat('?', 128))"
-    )
-    cur.execute(
-        "create table t_small(pk integer primary key, count integer default 0, payload text default repeat('?', 128))"
-    )
-    cur.execute(
-        "insert into t_huge(pk) values (generate_series(1,1000000))"
-    )  # table size is 21277 pages
-    cur.execute(
-        "insert into t_small(pk) values (generate_series(1,100000))"
-    )  # table size is 2128 pages
-    time.sleep(2)
-    before = time.monotonic()
-    for _ in range(100):
-        cur.execute("select sum(count) from t_small")
-    cur.execute("select sum(count) from t_huge")
-    after = time.monotonic()
-    cur.execute(f"select approximate_working_set_size_seconds({int(after - before + 1)})")
-    ws_estimation = cur.fetchall()[0][0]
-    log.info(f"Working set size estimaton {ws_estimation}")
-    cur.execute(f"select approximate_optimal_cache_size({int(after - before + 1)}, 0.99)")
-    optimal_cache_size = cur.fetchall()[0][0]
-    log.info(f"Optimal cache size for 99% hit rate {optimal_cache_size}")
-    assert ws_estimation >= 20000 and ws_estimation <= 30000
-    assert optimal_cache_size >= 2000 and optimal_cache_size <= 3000

test_runner/regress/test_pageserver_generations.py

@@ -53,7 +53,6 @@ TENANT_CONF = {
     # create image layers eagerly, so that GC can remove some layers
     "image_creation_threshold": "1",
     "image_layer_creation_check_threshold": "0",
-    "lsn_lease_length": "0s",
 }
 
 

test_runner/regress/test_postgres_version.py

@@ -30,8 +30,9 @@ def test_postgres_version(base_dir: Path, pg_bin: PgBin, pg_version: PgVersion):
     version = match.group("version")
     commit = match.group("commit")
 
-    assert (
-        pg_version.v_prefixed in expected_revisions
-    ), f"Released PostgreSQL version `{pg_version.v_prefixed}` doesn't exist in `vendor/revisions.json`, please update it if these changes are intentional"
-    msg = f"Unexpected Postgres {pg_version} version: `{output}`, please update `vendor/revisions.json` if these changes are intentional"
-    assert [version, commit] == expected_revisions[pg_version.v_prefixed], msg
+    if "." in version:
+        assert (
+            pg_version.v_prefixed in expected_revisions
+        ), f"Released PostgreSQL version `{pg_version.v_prefixed}` doesn't exist in `vendor/revisions.json`, please update it if these changes are intentional"
+        msg = f"Unexpected Postgres {pg_version} version: `{output}`, please update `vendor/revisions.json` if these changes are intentional"
+        assert [version, commit] == expected_revisions[pg_version.v_prefixed], msg

test_runner/regress/test_remote_storage.py

@@ -244,7 +244,6 @@ def test_remote_storage_upload_queue_retries(
             # create image layers eagerly, so that GC can remove some layers
             "image_creation_threshold": "1",
             "image_layer_creation_check_threshold": "0",
-            "lsn_lease_length": "0s",
         }
     )
 
@@ -392,7 +391,6 @@ def test_remote_timeline_client_calls_started_metric(
             # disable background compaction and GC. We invoke it manually when we want it to happen.
             "gc_period": "0s",
             "compaction_period": "0s",
-            "lsn_lease_length": "0s",
         }
     )
 

test_runner/regress/test_sharding.py

@@ -200,7 +200,6 @@ def test_sharding_split_compaction(neon_env_builder: NeonEnvBuilder, failpoint:
         # Disable automatic creation of image layers, as we will create them explicitly when we want them
         "image_creation_threshold": 9999,
         "image_layer_creation_check_threshold": 0,
-        "lsn_lease_length": "0s",
     }
 
     neon_env_builder.storage_controller_config = {

test_runner/regress/test_storage_controller.py

@@ -485,7 +485,7 @@ def test_storage_controller_compute_hook(
     httpserver.expect_request("/notify", method="PUT").respond_with_handler(handler)
 
     # Start running
-    env = neon_env_builder.init_start(initial_tenant_conf={"lsn_lease_length": "0s"})
+    env = neon_env_builder.init_start()
 
     # Initial notification from tenant creation
     assert len(notifications) == 1

test_runner/regress/test_storage_scrubber.py

@@ -204,7 +204,6 @@ def test_scrubber_physical_gc_ancestors(
             # No PITR, so that as soon as child shards generate an image layer, it covers ancestor deltas
             # and makes them GC'able
             "pitr_interval": "0s",
-            "lsn_lease_length": "0s",
         },
     )
 

test_runner/regress/test_tenant_detach.py

@@ -266,13 +266,13 @@ def test_tenant_reattach_while_busy(
 
 
 def test_tenant_detach_smoke(neon_env_builder: NeonEnvBuilder):
-    env = neon_env_builder.init_start(initial_tenant_conf={"lsn_lease_length": "0s"})
+    env = neon_env_builder.init_start()
     pageserver_http = env.pageserver.http_client()
 
     env.pageserver.allowed_errors.extend(PERMIT_PAGE_SERVICE_ERRORS)
 
     # create new nenant
-    tenant_id, timeline_id = env.initial_tenant, env.initial_timeline
+    tenant_id, timeline_id = env.neon_cli.create_tenant()
 
     # assert tenant exists on disk
     assert env.pageserver.tenant_dir(tenant_id).exists()

test_runner/regress/test_timeline_gc_blocking.py

@@ -45,10 +45,7 @@ def test_gc_blocking_by_timeline(neon_env_builder: NeonEnvBuilder, sharded: bool
     tenant_after = http.tenant_status(env.initial_tenant)
     assert tenant_before != tenant_after
     gc_blocking = tenant_after["gc_blocking"]
-    assert (
-        gc_blocking
-        == "BlockingReasons { tenant_blocked_by_lsn_lease_deadline: false, timelines: 1, reasons: EnumSet(Manual) }"
-    )
+    assert gc_blocking == "BlockingReasons { timelines: 1, reasons: EnumSet(Manual) }"
 
     wait_for_another_gc_round()
     pss.assert_log_contains(gc_skipped_line)

test_runner/regress/test_wal_acceptor.py

@@ -892,7 +892,6 @@ def test_timeline_status(neon_env_builder: NeonEnvBuilder, auth_enabled: bool):
     log.info(f"debug_dump before reboot {debug_dump_0}")
     assert debug_dump_0["timelines_count"] == 1
     assert debug_dump_0["timelines"][0]["timeline_id"] == str(timeline_id)
-    assert debug_dump_0["timelines"][0]["wal_last_modified"] != ""
 
     endpoint.safe_psql("create table t(i int)")
 

vendor/revisions.json

@@ -1,8 +1,4 @@
 {
-  "v17": [
-    "17rc1",
-    "9156d63ce253bed9d1f76355ceec610e444eaffa"
-  ],
   "v16": [
     "16.4",
     "0baa7346dfd42d61912eeca554c9bb0a190f0a1e"