disable flush time measurement

.github/actionlint.yml

@@ -21,5 +21,3 @@ config-variables:
   - SLACK_UPCOMING_RELEASE_CHANNEL_ID
   - DEV_AWS_OIDC_ROLE_ARN
   - BENCHMARK_INGEST_TARGET_PROJECTID
-  - PGREGRESS_PG16_PROJECT_ID
-  - PGREGRESS_PG17_PROJECT_ID

.github/workflows/_build-and-test-locally.yml

@@ -283,7 +283,7 @@ jobs:
           submodules: true
 
       - name: Pytest regression tests
-        continue-on-error: ${{ matrix.lfc_state == 'with-lfc' && inputs.build-type == 'debug' }}
+        continue-on-error: ${{ matrix.lfc_state == 'with-lfc' }}
         uses: ./.github/actions/run-python-test-set
         timeout-minutes: 60
         with:

.github/workflows/build_and_test.yml

@@ -255,17 +255,15 @@ jobs:
       build-tools-image: ${{ needs.build-build-tools-image.outputs.image }}-bookworm
       build-tag: ${{ needs.tag.outputs.build-tag }}
       build-type: ${{ matrix.build-type }}
-      # Run tests on all Postgres versions in release builds and only on the latest version in debug builds.
-      # Run without LFC on v17 release and debug builds only. For all the other cases LFC is enabled. Failure on the
-      # debug build with LFC enabled doesn't block merging.
+      # Run tests on all Postgres versions in release builds and only on the latest version in debug builds
+      # run without LFC on v17 release only
       test-cfg: |
-        ${{ matrix.build-type == 'release' && '[{"pg_version":"v14", "lfc_state": "with-lfc"},
-                                                {"pg_version":"v15", "lfc_state": "with-lfc"},
-                                                {"pg_version":"v16", "lfc_state": "with-lfc"},
-                                                {"pg_version":"v17", "lfc_state": "with-lfc"},
-                                                {"pg_version":"v17", "lfc_state": "without-lfc"}]'
-                                           || '[{"pg_version":"v17", "lfc_state": "without-lfc"},
-                                                {"pg_version":"v17", "lfc_state": "with-lfc" }]' }}
+        ${{ matrix.build-type == 'release' && '[{"pg_version":"v14", "lfc_state": "without-lfc"},
+                                                {"pg_version":"v15", "lfc_state": "without-lfc"},
+                                                {"pg_version":"v16", "lfc_state": "without-lfc"},
+                                                {"pg_version":"v17", "lfc_state": "without-lfc"},
+                                                {"pg_version":"v17", "lfc_state": "with-lfc"}]'
+                                           || '[{"pg_version":"v17", "lfc_state": "without-lfc"}]' }}
     secrets: inherit
 
   # Keep `benchmarks` job outside of `build-and-test-locally` workflow to make job failures non-blocking
@@ -1068,70 +1066,6 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
-      - name: Create git tag and GitHub release
-        if: github.ref_name == 'release' || github.ref_name == 'release-proxy' || github.ref_name == 'release-compute'
-        uses: actions/github-script@v7
-        with:
-          retries: 5
-          script: |
-            const tag = "${{ needs.tag.outputs.build-tag }}";
-
-            try {
-              const existingRef = await github.rest.git.getRef({
-                owner: context.repo.owner,
-                repo: context.repo.repo,
-                ref: `tags/${tag}`,
-              });
-
-              if (existingRef.data.object.sha !== context.sha) {
-                throw new Error(`Tag ${tag} already exists but points to a different commit (expected: ${context.sha}, actual: ${existingRef.data.object.sha}).`);
-              }
-
-              console.log(`Tag ${tag} already exists and points to ${context.sha} as expected.`);
-            } catch (error) {
-              if (error.status !== 404) {
-                throw error;
-              }
-
-              console.log(`Tag ${tag} does not exist. Creating it...`);
-              await github.rest.git.createRef({
-                owner: context.repo.owner,
-                repo: context.repo.repo,
-                ref: `refs/tags/${tag}`,
-                sha: context.sha,
-              });
-              console.log(`Tag ${tag} created successfully.`);
-            }
-
-            # TODO: check how GitHub releases looks for proxy/compute releases and enable them if they're ok
-            if (context.ref !== 'refs/heads/release') {
-              console.log(`GitHub release skipped for ${context.ref}.`);
-              return;
-            }
-
-            try {
-              const existingRelease = await github.rest.repos.getReleaseByTag({
-                owner: context.repo.owner,
-                repo: context.repo.repo,
-                tag: tag,
-              });
-
-              console.log(`Release for tag ${tag} already exists (ID: ${existingRelease.data.id}).`);
-            } catch (error) {
-              if (error.status !== 404) {
-                throw error;
-              }
-
-              console.log(`Release for tag ${tag} does not exist. Creating it...`);
-              await github.rest.repos.createRelease({
-                owner: context.repo.owner,
-                repo: context.repo.repo,
-                tag_name: tag,
-                generate_release_notes: true,
-              });
-              console.log(`Release for tag ${tag} created successfully.`);
-            }
-
       - name: Trigger deploy workflow
         env:
           GH_TOKEN: ${{ secrets.CI_ACCESS_TOKEN }}
@@ -1181,6 +1115,35 @@ jobs:
             exit 1
           fi
 
+      - name: Create git tag
+        if: github.ref_name == 'release' || github.ref_name == 'release-proxy' || github.ref_name == 'release-compute'
+        uses: actions/github-script@v7
+        with:
+          # Retry script for 5XX server errors: https://github.com/actions/github-script#retries
+          retries: 5
+          script: |
+            await github.rest.git.createRef({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              ref: "refs/tags/${{ needs.tag.outputs.build-tag }}",
+              sha: context.sha,
+            })
+
+      # TODO: check how GitHub releases looks for proxy releases and enable it if it's ok
+      - name: Create GitHub release
+        if: github.ref_name == 'release'
+        uses: actions/github-script@v7
+        with:
+          # Retry script for 5XX server errors: https://github.com/actions/github-script#retries
+          retries: 5
+          script: |
+            await github.rest.repos.createRelease({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              tag_name: "${{ needs.tag.outputs.build-tag }}",
+              generate_release_notes: true,
+            })
+
   # 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 ]

.github/workflows/cloud-regress.yml

@@ -23,14 +23,11 @@ jobs:
   regress:
     env:
       POSTGRES_DISTRIB_DIR: /tmp/neon/pg_install
+      DEFAULT_PG_VERSION: 16
       TEST_OUTPUT: /tmp/test_output
       BUILD_TYPE: remote
       AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_DEV }}
       AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_KEY_DEV }}
-    strategy:
-      fail-fast: false
-      matrix:
-        pg-version: [16, 17]
 
     runs-on: us-east-2
     container:
@@ -43,11 +40,9 @@ jobs:
           submodules: true
 
       - name: Patch the test
-        env:
-          PG_VERSION: ${{matrix.pg-version}}
         run: |
-          cd "vendor/postgres-v${PG_VERSION}"
-          patch -p1 < "../../compute/patches/cloud_regress_pg${PG_VERSION}.patch"
+          cd "vendor/postgres-v${DEFAULT_PG_VERSION}"
+          patch -p1 < "../../compute/patches/cloud_regress_pg${DEFAULT_PG_VERSION}.patch"
 
       - name: Generate a random password
         id: pwgen
@@ -60,9 +55,8 @@ jobs:
       - name: Change tests according to the generated password
         env:
           DBPASS: ${{ steps.pwgen.outputs.DBPASS }}
-          PG_VERSION: ${{matrix.pg-version}}
         run: |
-          cd vendor/postgres-v"${PG_VERSION}"/src/test/regress
+          cd vendor/postgres-v"${DEFAULT_PG_VERSION}"/src/test/regress
           for fname in sql/*.sql expected/*.out; do
             sed -i.bak s/NEON_PASSWORD_PLACEHOLDER/"'${DBPASS}'"/ "${fname}"
           done
@@ -79,29 +73,15 @@ jobs:
           path: /tmp/neon/
           prefix: latest
 
-      - name: Create a new branch
-        id: create-branch
-        uses: ./.github/actions/neon-branch-create
-        with:
-          api_key: ${{ secrets.NEON_STAGING_API_KEY }}
-          project_id: ${{ vars[format('PGREGRESS_PG{0}_PROJECT_ID', matrix.pg-version)] }}
-
       - name: Run the regression tests
         uses: ./.github/actions/run-python-test-set
         with:
           build_type: ${{ env.BUILD_TYPE }}
           test_selection: cloud_regress
-          pg_version: ${{matrix.pg-version}}
+          pg_version: ${{ env.DEFAULT_PG_VERSION }}
           extra_params: -m remote_cluster
         env:
-          BENCHMARK_CONNSTR: ${{steps.create-branch.outputs.dsn}}
-
-      - name: Delete branch
-        uses: ./.github/actions/neon-branch-delete
-        with:
-          api_key: ${{ secrets.NEON_STAGING_API_KEY }}
-          project_id: ${{ vars[format('PGREGRESS_PG{0}_PROJECT_ID', matrix.pg-version)] }}
-          branch_id: ${{steps.create-branch.outputs.branch_id}}
+          BENCHMARK_CONNSTR: ${{ secrets.PG_REGRESS_CONNSTR }}
 
       - name: Create Allure report
         id: create-allure-report

Cargo.toml

@@ -51,6 +51,10 @@ anyhow = { version = "1.0", features = ["backtrace"] }
 arc-swap = "1.6"
 async-compression = { version = "0.4.0", features = ["tokio", "gzip", "zstd"] }
 atomic-take = "1.1.0"
+azure_core = { version = "0.19", default-features = false, features = ["enable_reqwest_rustls", "hmac_rust"] }
+azure_identity = { version = "0.19", default-features = false, features = ["enable_reqwest_rustls"] }
+azure_storage = { version = "0.19", default-features = false, features = ["enable_reqwest_rustls"] }
+azure_storage_blobs = { version = "0.19", default-features = false, features = ["enable_reqwest_rustls"] }
 flate2 = "1.0.26"
 async-stream = "0.3"
 async-trait = "0.1"
@@ -212,12 +216,6 @@ postgres-protocol = { git = "https://github.com/neondatabase/rust-postgres.git",
 postgres-types = { git = "https://github.com/neondatabase/rust-postgres.git", branch = "neon" }
 tokio-postgres = { git = "https://github.com/neondatabase/rust-postgres.git", branch = "neon" }
 
-## Azure SDK crates
-azure_core = { git = "https://github.com/neondatabase/azure-sdk-for-rust.git", branch = "neon", default-features = false, features = ["enable_reqwest_rustls", "hmac_rust"] }
-azure_identity = { git = "https://github.com/neondatabase/azure-sdk-for-rust.git", branch = "neon", default-features = false, features = ["enable_reqwest_rustls"] }
-azure_storage = { git = "https://github.com/neondatabase/azure-sdk-for-rust.git", branch = "neon", default-features = false, features = ["enable_reqwest_rustls"] }
-azure_storage_blobs = { git = "https://github.com/neondatabase/azure-sdk-for-rust.git", branch = "neon", default-features = false, features = ["enable_reqwest_rustls"] }
-
 ## Local libraries
 compute_api = { version = "0.1", path = "./libs/compute_api/" }
 consumption_metrics = { version = "0.1", path = "./libs/consumption_metrics/" }

compute_tools/src/bin/compute_ctl.rs

@@ -246,48 +246,47 @@ fn try_spec_from_cli(
     let compute_id = matches.get_one::<String>("compute-id");
     let control_plane_uri = matches.get_one::<String>("control-plane-uri");
 
-    // First, try to get cluster spec from the cli argument
-    if let Some(spec_json) = spec_json {
-        info!("got spec from cli argument {}", spec_json);
-        return Ok(CliSpecParams {
-            spec: Some(serde_json::from_str(spec_json)?),
-            live_config_allowed: false,
-        });
-    }
-
-    // Second, try to read it from the file if path is provided
-    if let Some(spec_path) = spec_path {
-        let file = File::open(Path::new(spec_path))?;
-        return Ok(CliSpecParams {
-            spec: Some(serde_json::from_reader(file)?),
-            live_config_allowed: true,
-        });
-    }
-
-    let Some(compute_id) = compute_id else {
-        panic!(
-            "compute spec should be provided by one of the following ways: \
-                --spec OR --spec-path OR --control-plane-uri and --compute-id"
-        );
-    };
-    let Some(control_plane_uri) = control_plane_uri else {
-        panic!("must specify both --control-plane-uri and --compute-id or none");
-    };
-
-    match get_spec_from_control_plane(control_plane_uri, compute_id) {
-        Ok(spec) => Ok(CliSpecParams {
-            spec,
-            live_config_allowed: true,
-        }),
-        Err(e) => {
-            error!(
-                "cannot get response from control plane: {}\n\
-                neither spec nor confirmation that compute is in the Empty state was received",
-                e
-            );
-            Err(e)
+    let spec;
+    let mut live_config_allowed = false;
+    match spec_json {
+        // First, try to get cluster spec from the cli argument
+        Some(json) => {
+            info!("got spec from cli argument {}", json);
+            spec = Some(serde_json::from_str(json)?);
         }
-    }
+        None => {
+            // Second, try to read it from the file if path is provided
+            if let Some(sp) = spec_path {
+                let path = Path::new(sp);
+                let file = File::open(path)?;
+                spec = Some(serde_json::from_reader(file)?);
+                live_config_allowed = true;
+            } else if let Some(id) = compute_id {
+                if let Some(cp_base) = control_plane_uri {
+                    live_config_allowed = true;
+                    spec = match get_spec_from_control_plane(cp_base, id) {
+                        Ok(s) => s,
+                        Err(e) => {
+                            error!("cannot get response from control plane: {}", e);
+                            panic!("neither spec nor confirmation that compute is in the Empty state was received");
+                        }
+                    };
+                } else {
+                    panic!("must specify both --control-plane-uri and --compute-id or none");
+                }
+            } else {
+                panic!(
+                    "compute spec should be provided by one of the following ways: \
+                    --spec OR --spec-path OR --control-plane-uri and --compute-id"
+                );
+            }
+        }
+    };
+
+    Ok(CliSpecParams {
+        spec,
+        live_config_allowed,
+    })
 }
 
 struct CliSpecParams {

compute_tools/src/http/openapi_spec.yaml

@@ -537,14 +537,12 @@ components:
             properties:
               extname:
                 type: string
-              version:
-                type: string
+              versions:
+                type: array
                 items:
                   type: string
               n_databases:
                 type: integer
-              owned_by_superuser:
-                type: integer
 
     SetRoleGrantsRequest:
       type: object

compute_tools/src/installed_extensions.rs

@@ -1,6 +1,7 @@
 use compute_api::responses::{InstalledExtension, InstalledExtensions};
 use metrics::proto::MetricFamily;
 use std::collections::HashMap;
+use std::collections::HashSet;
 
 use anyhow::Result;
 use postgres::{Client, NoTls};
@@ -37,77 +38,61 @@ fn list_dbs(client: &mut Client) -> Result<Vec<String>> {
 /// Connect to every database (see list_dbs above) and get the list of installed extensions.
 ///
 /// Same extension can be installed in multiple databases with different versions,
-/// so we report a separate metric (number of databases where it is installed)
-/// for each extension version.
+/// we only keep the highest and lowest version across all databases.
 pub fn get_installed_extensions(mut conf: postgres::config::Config) -> Result<InstalledExtensions> {
     conf.application_name("compute_ctl:get_installed_extensions");
     let mut client = conf.connect(NoTls)?;
+
     let databases: Vec<String> = list_dbs(&mut client)?;
 
-    let mut extensions_map: HashMap<(String, String, String), InstalledExtension> = HashMap::new();
+    let mut extensions_map: HashMap<String, InstalledExtension> = HashMap::new();
     for db in databases.iter() {
         conf.dbname(db);
         let mut db_client = conf.connect(NoTls)?;
-        let extensions: Vec<(String, String, i32)> = db_client
+        let extensions: Vec<(String, String)> = db_client
             .query(
-                "SELECT extname, extversion, extowner::integer FROM pg_catalog.pg_extension",
+                "SELECT extname, extversion FROM pg_catalog.pg_extension;",
                 &[],
             )?
             .iter()
-            .map(|row| {
-                (
-                    row.get("extname"),
-                    row.get("extversion"),
-                    row.get("extowner"),
-                )
-            })
+            .map(|row| (row.get("extname"), row.get("extversion")))
             .collect();
 
-        for (extname, v, extowner) in extensions.iter() {
+        for (extname, v) in extensions.iter() {
             let version = v.to_string();
 
-            // check if the extension is owned by superuser
-            // 10 is the oid of superuser
-            let owned_by_superuser = if *extowner == 10 { "1" } else { "0" };
+            // increment the number of databases where the version of extension is installed
+            INSTALLED_EXTENSIONS
+                .with_label_values(&[extname, &version])
+                .inc();
 
             extensions_map
-                .entry((
-                    extname.to_string(),
-                    version.clone(),
-                    owned_by_superuser.to_string(),
-                ))
+                .entry(extname.to_string())
                 .and_modify(|e| {
+                    e.versions.insert(version.clone());
                     // count the number of databases where the extension is installed
                     e.n_databases += 1;
                 })
                 .or_insert(InstalledExtension {
                     extname: extname.to_string(),
-                    version: version.clone(),
+                    versions: HashSet::from([version.clone()]),
                     n_databases: 1,
-                    owned_by_superuser: owned_by_superuser.to_string(),
                 });
         }
     }
 
-    for (key, ext) in extensions_map.iter() {
-        let (extname, version, owned_by_superuser) = key;
-        let n_databases = ext.n_databases as u64;
-
-        INSTALLED_EXTENSIONS
-            .with_label_values(&[extname, version, owned_by_superuser])
-            .set(n_databases);
-    }
-
-    Ok(InstalledExtensions {
+    let res = InstalledExtensions {
         extensions: extensions_map.into_values().collect(),
-    })
+    };
+
+    Ok(res)
 }
 
 static INSTALLED_EXTENSIONS: Lazy<UIntGaugeVec> = Lazy::new(|| {
     register_uint_gauge_vec!(
         "compute_installed_extensions",
         "Number of databases where the version of extension is installed",
-        &["extension_name", "version", "owned_by_superuser"]
+        &["extension_name", "version"]
     )
     .expect("failed to define a metric")
 });

control_plane/src/endpoint.rs

@@ -810,7 +810,7 @@ impl Endpoint {
         }
 
         let client = reqwest::Client::builder()
-            .timeout(Duration::from_secs(120))
+            .timeout(Duration::from_secs(30))
             .build()
             .unwrap();
         let response = client

control_plane/src/pageserver.rs

@@ -435,7 +435,7 @@ impl PageServerNode {
     ) -> anyhow::Result<()> {
         let config = Self::parse_config(settings)?;
         self.http_client
-            .set_tenant_config(&models::TenantConfigRequest { tenant_id, config })
+            .tenant_config(&models::TenantConfigRequest { tenant_id, config })
             .await?;
 
         Ok(())

control_plane/storcon_cli/src/main.rs

@@ -9,8 +9,8 @@ use pageserver_api::{
     },
     models::{
         EvictionPolicy, EvictionPolicyLayerAccessThreshold, LocationConfigSecondary,
-        ShardParameters, TenantConfig, TenantConfigPatchRequest, TenantConfigRequest,
-        TenantShardSplitRequest, TenantShardSplitResponse,
+        ShardParameters, TenantConfig, TenantConfigRequest, TenantShardSplitRequest,
+        TenantShardSplitResponse,
     },
     shard::{ShardStripeSize, TenantShardId},
 };
@@ -116,19 +116,9 @@ enum Command {
         #[arg(long)]
         tenant_shard_id: TenantShardId,
     },
-    /// Set the pageserver tenant configuration of a tenant: this is the configuration structure
+    /// Modify the pageserver tenant configuration of a tenant: this is the configuration structure
     /// that is passed through to pageservers, and does not affect storage controller behavior.
-    /// Any previous tenant configs are overwritten.
-    SetTenantConfig {
-        #[arg(long)]
-        tenant_id: TenantId,
-        #[arg(long)]
-        config: String,
-    },
-    /// Patch the pageserver tenant configuration of a tenant. Any fields with null values in the
-    /// provided JSON are unset from the tenant config and all fields with non-null values are set.
-    /// Unspecified fields are not changed.
-    PatchTenantConfig {
+    TenantConfig {
         #[arg(long)]
         tenant_id: TenantId,
         #[arg(long)]
@@ -559,21 +549,11 @@ async fn main() -> anyhow::Result<()> {
                 )
                 .await?;
         }
-        Command::SetTenantConfig { tenant_id, config } => {
+        Command::TenantConfig { tenant_id, config } => {
             let tenant_conf = serde_json::from_str(&config)?;
 
             vps_client
-                .set_tenant_config(&TenantConfigRequest {
-                    tenant_id,
-                    config: tenant_conf,
-                })
-                .await?;
-        }
-        Command::PatchTenantConfig { tenant_id, config } => {
-            let tenant_conf = serde_json::from_str(&config)?;
-
-            vps_client
-                .patch_tenant_config(&TenantConfigPatchRequest {
+                .tenant_config(&TenantConfigRequest {
                     tenant_id,
                     config: tenant_conf,
                 })
@@ -756,7 +736,7 @@ async fn main() -> anyhow::Result<()> {
             threshold,
         } => {
             vps_client
-                .set_tenant_config(&TenantConfigRequest {
+                .tenant_config(&TenantConfigRequest {
                     tenant_id,
                     config: TenantConfig {
                         eviction_policy: Some(EvictionPolicy::LayerAccessThreshold(

deny.toml

@@ -42,7 +42,6 @@ allow = [
     "MPL-2.0",
     "OpenSSL",
     "Unicode-DFS-2016",
-    "Unicode-3.0",
 ]
 confidence-threshold = 0.8
 exceptions = [

libs/compute_api/src/responses.rs

@@ -1,5 +1,6 @@
 //! Structs representing the JSON formats used in the compute_ctl's HTTP API.
 
+use std::collections::HashSet;
 use std::fmt::Display;
 
 use chrono::{DateTime, Utc};
@@ -162,9 +163,8 @@ pub enum ControlPlaneComputeStatus {
 #[derive(Clone, Debug, Default, Serialize)]
 pub struct InstalledExtension {
     pub extname: String,
-    pub version: String,
+    pub versions: HashSet<String>,
     pub n_databases: u32, // Number of databases using this extension
-    pub owned_by_superuser: String,
 }
 
 #[derive(Clone, Debug, Default, Serialize)]

libs/pageserver_api/src/controller_api.rs

@@ -245,17 +245,6 @@ impl From<NodeAvailability> for NodeAvailabilityWrapper {
     }
 }
 
-/// Scheduling policy enables us to selectively disable some automatic actions that the
-/// controller performs on a tenant shard. This is only set to a non-default value by
-/// human intervention, and it is reset to the default value (Active) when the tenant's
-/// placement policy is modified away from Attached.
-///
-/// The typical use of a non-Active scheduling policy is one of:
-/// - Pinnning a shard to a node (i.e. migrating it there & setting a non-Active scheduling policy)
-/// - Working around a bug (e.g. if something is flapping and we need to stop it until the bug is fixed)
-///
-/// If you're not sure which policy to use to pin a shard to its current location, you probably
-/// want Pause.
 #[derive(Serialize, Deserialize, Clone, Copy, Eq, PartialEq, Debug)]
 pub enum ShardSchedulingPolicy {
     // Normal mode: the tenant's scheduled locations may be updated at will, including

libs/pageserver_api/src/key.rs

@@ -24,7 +24,7 @@ pub struct Key {
 
 /// When working with large numbers of Keys in-memory, it is more efficient to handle them as i128 than as
 /// a struct of fields.
-#[derive(Clone, Copy, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize, Deserialize, Debug)]
+#[derive(Clone, Copy, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize, Deserialize)]
 pub struct CompactKey(i128);
 
 /// The storage key size.

libs/pageserver_api/src/models.rs

@@ -17,7 +17,7 @@ use std::{
 
 use byteorder::{BigEndian, ReadBytesExt};
 use postgres_ffi::BLCKSZ;
-use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
 use utils::{
     completion,
@@ -325,115 +325,6 @@ impl Default for ShardParameters {
     }
 }
 
-#[derive(Debug, Default, Clone, Eq, PartialEq)]
-pub enum FieldPatch<T> {
-    Upsert(T),
-    Remove,
-    #[default]
-    Noop,
-}
-
-impl<T> FieldPatch<T> {
-    fn is_noop(&self) -> bool {
-        matches!(self, FieldPatch::Noop)
-    }
-
-    pub fn apply(self, target: &mut Option<T>) {
-        match self {
-            Self::Upsert(v) => *target = Some(v),
-            Self::Remove => *target = None,
-            Self::Noop => {}
-        }
-    }
-
-    pub fn map<U, E, F: FnOnce(T) -> Result<U, E>>(self, map: F) -> Result<FieldPatch<U>, E> {
-        match self {
-            Self::Upsert(v) => Ok(FieldPatch::<U>::Upsert(map(v)?)),
-            Self::Remove => Ok(FieldPatch::<U>::Remove),
-            Self::Noop => Ok(FieldPatch::<U>::Noop),
-        }
-    }
-}
-
-impl<'de, T: Deserialize<'de>> Deserialize<'de> for FieldPatch<T> {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        Option::deserialize(deserializer).map(|opt| match opt {
-            None => FieldPatch::Remove,
-            Some(val) => FieldPatch::Upsert(val),
-        })
-    }
-}
-
-impl<T: Serialize> Serialize for FieldPatch<T> {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        match self {
-            FieldPatch::Upsert(val) => serializer.serialize_some(val),
-            FieldPatch::Remove => serializer.serialize_none(),
-            FieldPatch::Noop => unreachable!(),
-        }
-    }
-}
-
-#[derive(Serialize, Deserialize, Debug, Default, Clone, Eq, PartialEq)]
-#[serde(default)]
-pub struct TenantConfigPatch {
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub checkpoint_distance: FieldPatch<u64>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub checkpoint_timeout: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub compaction_target_size: FieldPatch<u64>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub compaction_period: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub compaction_threshold: FieldPatch<usize>,
-    // defer parsing compaction_algorithm, like eviction_policy
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub compaction_algorithm: FieldPatch<CompactionAlgorithmSettings>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub gc_horizon: FieldPatch<u64>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub gc_period: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub image_creation_threshold: FieldPatch<usize>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub pitr_interval: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub walreceiver_connect_timeout: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub lagging_wal_timeout: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub max_lsn_wal_lag: FieldPatch<NonZeroU64>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub eviction_policy: FieldPatch<EvictionPolicy>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub min_resident_size_override: FieldPatch<u64>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub evictions_low_residence_duration_metric_threshold: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub heatmap_period: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub lazy_slru_download: FieldPatch<bool>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub timeline_get_throttle: FieldPatch<ThrottleConfig>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub image_layer_creation_check_threshold: FieldPatch<u8>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub lsn_lease_length: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub lsn_lease_length_for_ts: FieldPatch<String>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub timeline_offloading: FieldPatch<bool>,
-    #[serde(skip_serializing_if = "FieldPatch::is_noop")]
-    pub wal_receiver_protocol_override: FieldPatch<PostgresClientProtocol>,
-}
-
 /// An alternative representation of `pageserver::tenant::TenantConf` with
 /// simpler types.
 #[derive(Serialize, Deserialize, Debug, Default, Clone, Eq, PartialEq)]
@@ -465,107 +356,6 @@ pub struct TenantConfig {
     pub wal_receiver_protocol_override: Option<PostgresClientProtocol>,
 }
 
-impl TenantConfig {
-    pub fn apply_patch(self, patch: TenantConfigPatch) -> TenantConfig {
-        let Self {
-            mut checkpoint_distance,
-            mut checkpoint_timeout,
-            mut compaction_target_size,
-            mut compaction_period,
-            mut compaction_threshold,
-            mut compaction_algorithm,
-            mut gc_horizon,
-            mut gc_period,
-            mut image_creation_threshold,
-            mut pitr_interval,
-            mut walreceiver_connect_timeout,
-            mut lagging_wal_timeout,
-            mut max_lsn_wal_lag,
-            mut eviction_policy,
-            mut min_resident_size_override,
-            mut evictions_low_residence_duration_metric_threshold,
-            mut heatmap_period,
-            mut lazy_slru_download,
-            mut timeline_get_throttle,
-            mut image_layer_creation_check_threshold,
-            mut lsn_lease_length,
-            mut lsn_lease_length_for_ts,
-            mut timeline_offloading,
-            mut wal_receiver_protocol_override,
-        } = self;
-
-        patch.checkpoint_distance.apply(&mut checkpoint_distance);
-        patch.checkpoint_timeout.apply(&mut checkpoint_timeout);
-        patch
-            .compaction_target_size
-            .apply(&mut compaction_target_size);
-        patch.compaction_period.apply(&mut compaction_period);
-        patch.compaction_threshold.apply(&mut compaction_threshold);
-        patch.compaction_algorithm.apply(&mut compaction_algorithm);
-        patch.gc_horizon.apply(&mut gc_horizon);
-        patch.gc_period.apply(&mut gc_period);
-        patch
-            .image_creation_threshold
-            .apply(&mut image_creation_threshold);
-        patch.pitr_interval.apply(&mut pitr_interval);
-        patch
-            .walreceiver_connect_timeout
-            .apply(&mut walreceiver_connect_timeout);
-        patch.lagging_wal_timeout.apply(&mut lagging_wal_timeout);
-        patch.max_lsn_wal_lag.apply(&mut max_lsn_wal_lag);
-        patch.eviction_policy.apply(&mut eviction_policy);
-        patch
-            .min_resident_size_override
-            .apply(&mut min_resident_size_override);
-        patch
-            .evictions_low_residence_duration_metric_threshold
-            .apply(&mut evictions_low_residence_duration_metric_threshold);
-        patch.heatmap_period.apply(&mut heatmap_period);
-        patch.lazy_slru_download.apply(&mut lazy_slru_download);
-        patch
-            .timeline_get_throttle
-            .apply(&mut timeline_get_throttle);
-        patch
-            .image_layer_creation_check_threshold
-            .apply(&mut image_layer_creation_check_threshold);
-        patch.lsn_lease_length.apply(&mut lsn_lease_length);
-        patch
-            .lsn_lease_length_for_ts
-            .apply(&mut lsn_lease_length_for_ts);
-        patch.timeline_offloading.apply(&mut timeline_offloading);
-        patch
-            .wal_receiver_protocol_override
-            .apply(&mut wal_receiver_protocol_override);
-
-        Self {
-            checkpoint_distance,
-            checkpoint_timeout,
-            compaction_target_size,
-            compaction_period,
-            compaction_threshold,
-            compaction_algorithm,
-            gc_horizon,
-            gc_period,
-            image_creation_threshold,
-            pitr_interval,
-            walreceiver_connect_timeout,
-            lagging_wal_timeout,
-            max_lsn_wal_lag,
-            eviction_policy,
-            min_resident_size_override,
-            evictions_low_residence_duration_metric_threshold,
-            heatmap_period,
-            lazy_slru_download,
-            timeline_get_throttle,
-            image_layer_creation_check_threshold,
-            lsn_lease_length,
-            lsn_lease_length_for_ts,
-            timeline_offloading,
-            wal_receiver_protocol_override,
-        }
-    }
-}
-
 /// The policy for the aux file storage.
 ///
 /// It can be switched through `switch_aux_file_policy` tenant config.
@@ -896,14 +686,6 @@ impl TenantConfigRequest {
     }
 }
 
-#[derive(Serialize, Deserialize, Debug)]
-#[serde(deny_unknown_fields)]
-pub struct TenantConfigPatchRequest {
-    pub tenant_id: TenantId,
-    #[serde(flatten)]
-    pub config: TenantConfigPatch, // as we have a flattened field, we should reject all unknown fields in it
-}
-
 /// See [`TenantState::attachment_status`] and the OpenAPI docs for context.
 #[derive(Serialize, Deserialize, Clone)]
 #[serde(tag = "slug", content = "data", rename_all = "snake_case")]
@@ -1917,45 +1699,4 @@ mod tests {
             );
         }
     }
-
-    #[test]
-    fn test_tenant_config_patch_request_serde() {
-        let patch_request = TenantConfigPatchRequest {
-            tenant_id: TenantId::from_str("17c6d121946a61e5ab0fe5a2fd4d8215").unwrap(),
-            config: TenantConfigPatch {
-                checkpoint_distance: FieldPatch::Upsert(42),
-                gc_horizon: FieldPatch::Remove,
-                compaction_threshold: FieldPatch::Noop,
-                ..TenantConfigPatch::default()
-            },
-        };
-
-        let json = serde_json::to_string(&patch_request).unwrap();
-
-        let expected = r#"{"tenant_id":"17c6d121946a61e5ab0fe5a2fd4d8215","checkpoint_distance":42,"gc_horizon":null}"#;
-        assert_eq!(json, expected);
-
-        let decoded: TenantConfigPatchRequest = serde_json::from_str(&json).unwrap();
-        assert_eq!(decoded.tenant_id, patch_request.tenant_id);
-        assert_eq!(decoded.config, patch_request.config);
-
-        // Now apply the patch to a config to demonstrate semantics
-
-        let base = TenantConfig {
-            checkpoint_distance: Some(28),
-            gc_horizon: Some(100),
-            compaction_target_size: Some(1024),
-            ..Default::default()
-        };
-
-        let expected = TenantConfig {
-            checkpoint_distance: Some(42),
-            gc_horizon: None,
-            ..base.clone()
-        };
-
-        let patched = base.apply_patch(decoded.config);
-
-        assert_eq!(patched, expected);
-    }
 }

libs/remote_storage/src/azure_blob.rs

@@ -8,14 +8,15 @@ use std::io;
 use std::num::NonZeroU32;
 use std::pin::Pin;
 use std::str::FromStr;
+use std::sync::Arc;
 use std::time::Duration;
 use std::time::SystemTime;
 
 use super::REMOTE_STORAGE_PREFIX_SEPARATOR;
-use anyhow::Context;
 use anyhow::Result;
 use azure_core::request_options::{IfMatchCondition, MaxResults, Metadata, Range};
 use azure_core::{Continuable, RetryOptions};
+use azure_identity::DefaultAzureCredential;
 use azure_storage::StorageCredentials;
 use azure_storage_blobs::blob::CopyStatus;
 use azure_storage_blobs::prelude::ClientBuilder;
@@ -75,9 +76,8 @@ impl AzureBlobStorage {
         let credentials = if let Ok(access_key) = env::var("AZURE_STORAGE_ACCESS_KEY") {
             StorageCredentials::access_key(account.clone(), access_key)
         } else {
-            let token_credential = azure_identity::create_default_credential()
-                .context("trying to obtain Azure default credentials")?;
-            StorageCredentials::token_credential(token_credential)
+            let token_credential = DefaultAzureCredential::default();
+            StorageCredentials::token_credential(Arc::new(token_credential))
         };
 
         // we have an outer retry
@@ -624,10 +624,6 @@ impl RemoteStorage for AzureBlobStorage {
         res
     }
 
-    fn max_keys_per_delete(&self) -> usize {
-        super::MAX_KEYS_PER_DELETE_AZURE
-    }
-
     async fn copy(
         &self,
         from: &RemotePath,

libs/remote_storage/src/lib.rs

@@ -70,14 +70,7 @@ pub const DEFAULT_REMOTE_STORAGE_AZURE_CONCURRENCY_LIMIT: usize = 100;
 pub const DEFAULT_MAX_KEYS_PER_LIST_RESPONSE: Option<i32> = None;
 
 /// As defined in S3 docs
-///
-/// <https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html>
-pub const MAX_KEYS_PER_DELETE_S3: usize = 1000;
-
-/// As defined in Azure docs
-///
-/// <https://learn.microsoft.com/en-us/rest/api/storageservices/blob-batch>
-pub const MAX_KEYS_PER_DELETE_AZURE: usize = 256;
+pub const MAX_KEYS_PER_DELETE: usize = 1000;
 
 const REMOTE_STORAGE_PREFIX_SEPARATOR: char = '/';
 
@@ -347,14 +340,6 @@ pub trait RemoteStorage: Send + Sync + 'static {
         cancel: &CancellationToken,
     ) -> anyhow::Result<()>;
 
-    /// Returns the maximum number of keys that a call to [`Self::delete_objects`] can delete without chunking
-    ///
-    /// The value returned is only an optimization hint, One can pass larger number of objects to
-    /// `delete_objects` as well.
-    ///
-    /// The value is guaranteed to be >= 1.
-    fn max_keys_per_delete(&self) -> usize;
-
     /// Deletes all objects matching the given prefix.
     ///
     /// NB: this uses NoDelimiter and will match partial prefixes. For example, the prefix /a/b will
@@ -548,16 +533,6 @@ impl<Other: RemoteStorage> GenericRemoteStorage<Arc<Other>> {
         }
     }
 
-    /// [`RemoteStorage::max_keys_per_delete`]
-    pub fn max_keys_per_delete(&self) -> usize {
-        match self {
-            Self::LocalFs(s) => s.max_keys_per_delete(),
-            Self::AwsS3(s) => s.max_keys_per_delete(),
-            Self::AzureBlob(s) => s.max_keys_per_delete(),
-            Self::Unreliable(s) => s.max_keys_per_delete(),
-        }
-    }
-
     /// See [`RemoteStorage::delete_prefix`]
     pub async fn delete_prefix(
         &self,

libs/remote_storage/src/local_fs.rs

@@ -573,10 +573,6 @@ impl RemoteStorage for LocalFs {
         Ok(())
     }
 
-    fn max_keys_per_delete(&self) -> usize {
-        super::MAX_KEYS_PER_DELETE_S3
-    }
-
     async fn copy(
         &self,
         from: &RemotePath,

libs/remote_storage/src/s3_bucket.rs

@@ -48,7 +48,7 @@ use crate::{
     metrics::{start_counting_cancelled_wait, start_measuring_requests},
     support::PermitCarrying,
     ConcurrencyLimiter, Download, DownloadError, DownloadOpts, Listing, ListingMode, ListingObject,
-    RemotePath, RemoteStorage, TimeTravelError, TimeoutOrCancel, MAX_KEYS_PER_DELETE_S3,
+    RemotePath, RemoteStorage, TimeTravelError, TimeoutOrCancel, MAX_KEYS_PER_DELETE,
     REMOTE_STORAGE_PREFIX_SEPARATOR,
 };
 
@@ -355,7 +355,7 @@ impl S3Bucket {
         let kind = RequestKind::Delete;
         let mut cancel = std::pin::pin!(cancel.cancelled());
 
-        for chunk in delete_objects.chunks(MAX_KEYS_PER_DELETE_S3) {
+        for chunk in delete_objects.chunks(MAX_KEYS_PER_DELETE) {
             let started_at = start_measuring_requests(kind);
 
             let req = self
@@ -832,10 +832,6 @@ impl RemoteStorage for S3Bucket {
         self.delete_oids(&permit, &delete_objects, cancel).await
     }
 
-    fn max_keys_per_delete(&self) -> usize {
-        MAX_KEYS_PER_DELETE_S3
-    }
-
     async fn delete(&self, path: &RemotePath, cancel: &CancellationToken) -> anyhow::Result<()> {
         let paths = std::array::from_ref(path);
         self.delete_objects(paths, cancel).await

libs/remote_storage/src/simulate_failures.rs

@@ -203,10 +203,6 @@ impl RemoteStorage for UnreliableWrapper {
         Ok(())
     }
 
-    fn max_keys_per_delete(&self) -> usize {
-        self.inner.max_keys_per_delete()
-    }
-
     async fn copy(
         &self,
         from: &RemotePath,

libs/utils/src/lib.rs

@@ -94,8 +94,6 @@ pub mod toml_edit_ext;
 
 pub mod circuit_breaker;
 
-pub mod try_rcu;
-
 // Re-export used in macro. Avoids adding git-version as dep in target crates.
 #[doc(hidden)]
 pub use git_version;

libs/utils/src/try_rcu.rs

@@ -1,77 +0,0 @@
-//! Try RCU extension lifted from <https://github.com/vorner/arc-swap/issues/94#issuecomment-1987154023>
-
-pub trait ArcSwapExt<T> {
-    /// [`ArcSwap::rcu`](arc_swap::ArcSwap::rcu), but with Result that short-circuits on error.
-    fn try_rcu<R, F, E>(&self, f: F) -> Result<T, E>
-    where
-        F: FnMut(&T) -> Result<R, E>,
-        R: Into<T>;
-}
-
-impl<T, S> ArcSwapExt<T> for arc_swap::ArcSwapAny<T, S>
-where
-    T: arc_swap::RefCnt,
-    S: arc_swap::strategy::CaS<T>,
-{
-    fn try_rcu<R, F, E>(&self, mut f: F) -> Result<T, E>
-    where
-        F: FnMut(&T) -> Result<R, E>,
-        R: Into<T>,
-    {
-        fn ptr_eq<Base, A, B>(a: A, b: B) -> bool
-        where
-            A: arc_swap::AsRaw<Base>,
-            B: arc_swap::AsRaw<Base>,
-        {
-            let a = a.as_raw();
-            let b = b.as_raw();
-            std::ptr::eq(a, b)
-        }
-
-        let mut cur = self.load();
-        loop {
-            let new = f(&cur)?.into();
-            let prev = self.compare_and_swap(&*cur, new);
-            let swapped = ptr_eq(&*cur, &*prev);
-            if swapped {
-                return Ok(arc_swap::Guard::into_inner(prev));
-            } else {
-                cur = prev;
-            }
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use arc_swap::ArcSwap;
-    use std::sync::Arc;
-
-    #[test]
-    fn test_try_rcu_success() {
-        let swap = ArcSwap::from(Arc::new(42));
-
-        let result = swap.try_rcu(|value| -> Result<_, String> { Ok(**value + 1) });
-
-        assert!(result.is_ok());
-        assert_eq!(**swap.load(), 43);
-    }
-
-    #[test]
-    fn test_try_rcu_error() {
-        let swap = ArcSwap::from(Arc::new(42));
-
-        let result = swap.try_rcu(|value| -> Result<i32, _> {
-            if **value == 42 {
-                Err("err")
-            } else {
-                Ok(**value + 1)
-            }
-        });
-
-        assert!(result.is_err());
-        assert_eq!(result.unwrap_err(), "err");
-        assert_eq!(**swap.load(), 42);
-    }
-}

libs/wal_decoder/proto/interpreted_wal.proto

@@ -37,7 +37,7 @@ message ValueMeta {
 }
 
 message CompactKey {
-  uint64 high = 1;
-  uint64 low = 2;
+  int64 high = 1;
+  int64 low = 2;
 }
 

libs/wal_decoder/src/wire_format.rs

@@ -236,8 +236,8 @@ impl From<ValueMeta> for proto::ValueMeta {
 impl From<CompactKey> for proto::CompactKey {
     fn from(value: CompactKey) -> Self {
         proto::CompactKey {
-            high: (value.raw() >> 64) as u64,
-            low: value.raw() as u64,
+            high: (value.raw() >> 64) as i64,
+            low: value.raw() as i64,
         }
     }
 }
@@ -354,64 +354,3 @@ impl From<proto::CompactKey> for CompactKey {
         (((value.high as i128) << 64) | (value.low as i128)).into()
     }
 }
-
-#[test]
-fn test_compact_key_with_large_relnode() {
-    use pageserver_api::key::Key;
-
-    let inputs = vec![
-        Key {
-            field1: 0,
-            field2: 0x100,
-            field3: 0x200,
-            field4: 0,
-            field5: 0x10,
-            field6: 0x5,
-        },
-        Key {
-            field1: 0,
-            field2: 0x100,
-            field3: 0x200,
-            field4: 0x007FFFFF,
-            field5: 0x10,
-            field6: 0x5,
-        },
-        Key {
-            field1: 0,
-            field2: 0x100,
-            field3: 0x200,
-            field4: 0x00800000,
-            field5: 0x10,
-            field6: 0x5,
-        },
-        Key {
-            field1: 0,
-            field2: 0x100,
-            field3: 0x200,
-            field4: 0x00800001,
-            field5: 0x10,
-            field6: 0x5,
-        },
-        Key {
-            field1: 0,
-            field2: 0xFFFFFFFF,
-            field3: 0xFFFFFFFF,
-            field4: 0xFFFFFFFF,
-            field5: 0x0,
-            field6: 0x0,
-        },
-    ];
-
-    for input in inputs {
-        assert!(input.is_valid_key_on_write_path());
-        let compact = input.to_compact();
-        let proto: proto::CompactKey = compact.into();
-        let from_proto: CompactKey = proto.into();
-
-        assert_eq!(
-            compact, from_proto,
-            "Round trip failed for key with relnode={:#x}",
-            input.field4
-        );
-    }
-}

libs/walproposer/build.rs

@@ -30,9 +30,9 @@ fn main() -> anyhow::Result<()> {
     let pgxn_neon = std::fs::canonicalize(pgxn_neon)?;
     let pgxn_neon = pgxn_neon.to_str().ok_or(anyhow!("Bad non-UTF path"))?;
 
-    println!("cargo:rustc-link-lib=static=walproposer");
     println!("cargo:rustc-link-lib=static=pgport");
     println!("cargo:rustc-link-lib=static=pgcommon");
+    println!("cargo:rustc-link-lib=static=walproposer");
     println!("cargo:rustc-link-search={walproposer_lib_search_str}");
 
     // Rebuild crate when libwalproposer.a changes

pageserver/client/src/mgmt_api.rs

@@ -270,18 +270,12 @@ impl Client {
         Ok(body)
     }
 
-    pub async fn set_tenant_config(&self, req: &TenantConfigRequest) -> Result<()> {
+    pub async fn tenant_config(&self, req: &TenantConfigRequest) -> Result<()> {
         let uri = format!("{}/v1/tenant/config", self.mgmt_api_endpoint);
         self.request(Method::PUT, &uri, req).await?;
         Ok(())
     }
 
-    pub async fn patch_tenant_config(&self, req: &TenantConfigPatchRequest) -> Result<()> {
-        let uri = format!("{}/v1/tenant/config", self.mgmt_api_endpoint);
-        self.request(Method::PATCH, &uri, req).await?;
-        Ok(())
-    }
-
     pub async fn tenant_secondary_download(
         &self,
         tenant_id: TenantShardId,

pageserver/pagebench/src/cmd/aux_files.rs

@@ -64,7 +64,7 @@ async fn main_impl(args: Args) -> anyhow::Result<()> {
     println!("operating on timeline {}", timeline);
 
     mgmt_api_client
-        .set_tenant_config(&TenantConfigRequest {
+        .tenant_config(&TenantConfigRequest {
             tenant_id: timeline.tenant_id,
             config: TenantConfig::default(),
         })

pageserver/src/deletion_queue/deleter.rs

@@ -9,6 +9,7 @@
 use remote_storage::GenericRemoteStorage;
 use remote_storage::RemotePath;
 use remote_storage::TimeoutOrCancel;
+use remote_storage::MAX_KEYS_PER_DELETE;
 use std::time::Duration;
 use tokio_util::sync::CancellationToken;
 use tracing::info;
@@ -130,8 +131,7 @@ impl Deleter {
     }
 
     pub(super) async fn background(&mut self) -> Result<(), DeletionQueueError> {
-        let max_keys_per_delete = self.remote_storage.max_keys_per_delete();
-        self.accumulator.reserve(max_keys_per_delete);
+        self.accumulator.reserve(MAX_KEYS_PER_DELETE);
 
         loop {
             if self.cancel.is_cancelled() {
@@ -156,14 +156,14 @@ impl Deleter {
 
             match msg {
                 DeleterMessage::Delete(mut list) => {
-                    while !list.is_empty() || self.accumulator.len() == max_keys_per_delete {
-                        if self.accumulator.len() == max_keys_per_delete {
+                    while !list.is_empty() || self.accumulator.len() == MAX_KEYS_PER_DELETE {
+                        if self.accumulator.len() == MAX_KEYS_PER_DELETE {
                             self.flush().await?;
                             // If we have received this number of keys, proceed with attempting to execute
                             assert_eq!(self.accumulator.len(), 0);
                         }
 
-                        let available_slots = max_keys_per_delete - self.accumulator.len();
+                        let available_slots = MAX_KEYS_PER_DELETE - self.accumulator.len();
                         let take_count = std::cmp::min(available_slots, list.len());
                         for path in list.drain(list.len() - take_count..) {
                             self.accumulator.push(path);

pageserver/src/http/openapi_spec.yml

@@ -767,27 +767,7 @@ paths:
   /v1/tenant/config:
     put:
       description: |
-        Update tenant's config by setting it to the provided value
-
-        Invalid fields in the tenant config will cause the request to be rejected with status 400.
-      requestBody:
-        content:
-          application/json:
-            schema:
-              $ref: "#/components/schemas/TenantConfigRequest"
-      responses:
-        "200":
-          description: OK
-          content:
-            application/json:
-              schema:
-                type: array
-                items:
-                  $ref: "#/components/schemas/TenantInfo"
-    patch:
-      description: |
-        Update tenant's config additively by patching the updated fields provided.
-        Null values unset the field and non-null values upsert it.
+        Update tenant's config.
 
         Invalid fields in the tenant config will cause the request to be rejected with status 400.
       requestBody:

pageserver/src/http/routes.rs

@@ -28,7 +28,6 @@ use pageserver_api::models::LsnLease;
 use pageserver_api::models::LsnLeaseRequest;
 use pageserver_api::models::OffloadedTimelineInfo;
 use pageserver_api::models::ShardParameters;
-use pageserver_api::models::TenantConfigPatchRequest;
 use pageserver_api::models::TenantDetails;
 use pageserver_api::models::TenantLocationConfigRequest;
 use pageserver_api::models::TenantLocationConfigResponse;
@@ -1696,47 +1695,7 @@ async fn update_tenant_config_handler(
     crate::tenant::Tenant::persist_tenant_config(state.conf, &tenant_shard_id, &location_conf)
         .await
         .map_err(|e| ApiError::InternalServerError(anyhow::anyhow!(e)))?;
-
-    let _ = tenant
-        .update_tenant_config(|_crnt| Ok(new_tenant_conf.clone()))
-        .expect("Closure returns Ok()");
-
-    json_response(StatusCode::OK, ())
-}
-
-async fn patch_tenant_config_handler(
-    mut request: Request<Body>,
-    _cancel: CancellationToken,
-) -> Result<Response<Body>, ApiError> {
-    let request_data: TenantConfigPatchRequest = json_request(&mut request).await?;
-    let tenant_id = request_data.tenant_id;
-    check_permission(&request, Some(tenant_id))?;
-
-    let state = get_state(&request);
-
-    let tenant_shard_id = TenantShardId::unsharded(tenant_id);
-
-    let tenant = state
-        .tenant_manager
-        .get_attached_tenant_shard(tenant_shard_id)?;
-    tenant.wait_to_become_active(ACTIVE_TENANT_TIMEOUT).await?;
-
-    let updated = tenant
-        .update_tenant_config(|crnt| crnt.apply_patch(request_data.config.clone()))
-        .map_err(ApiError::BadRequest)?;
-
-    // This is a legacy API that only operates on attached tenants: the preferred
-    // API to use is the location_config/ endpoint, which lets the caller provide
-    // the full LocationConf.
-    let location_conf = LocationConf::attached_single(
-        updated,
-        tenant.get_generation(),
-        &ShardParameters::default(),
-    );
-
-    crate::tenant::Tenant::persist_tenant_config(state.conf, &tenant_shard_id, &location_conf)
-        .await
-        .map_err(|e| ApiError::InternalServerError(anyhow::anyhow!(e)))?;
+    tenant.set_new_tenant_config(new_tenant_conf);
 
     json_response(StatusCode::OK, ())
 }
@@ -2102,7 +2061,7 @@ async fn timeline_compact_handler(
             let tenant = state
                 .tenant_manager
                 .get_attached_tenant_shard(tenant_shard_id)?;
-            let rx = tenant.schedule_compaction(timeline_id, options).await.map_err(ApiError::InternalServerError)?;
+            let rx = tenant.schedule_compaction(timeline_id, options).await;
             if wait_until_scheduled_compaction_done {
                 // It is possible that this will take a long time, dropping the HTTP request will not cancel the compaction.
                 rx.await.ok();
@@ -3329,9 +3288,6 @@ pub fn make_router(
         .get("/v1/tenant/:tenant_shard_id/synthetic_size", |r| {
             api_handler(r, tenant_size_handler)
         })
-        .patch("/v1/tenant/config", |r| {
-            api_handler(r, patch_tenant_config_handler)
-        })
         .put("/v1/tenant/config", |r| {
             api_handler(r, update_tenant_config_handler)
         })

pageserver/src/metrics.rs

@@ -16,6 +16,7 @@ use postgres_backend::{is_expected_io_error, QueryError};
 use pq_proto::framed::ConnectionError;
 use strum::{EnumCount, VariantNames};
 use strum_macros::{IntoStaticStr, VariantNames};
+use tracing::warn;
 use utils::id::TimelineId;
 
 /// Prometheus histogram buckets (in seconds) for operations in the critical
@@ -1224,58 +1225,32 @@ pub(crate) mod virtual_file_io_engine {
 
 pub(crate) struct SmgrOpTimer(Option<SmgrOpTimerInner>);
 pub(crate) struct SmgrOpTimerInner {
-    global_execution_latency_histo: Histogram,
-    per_timeline_execution_latency_histo: Option<Histogram>,
+    global_latency_histo: Histogram,
 
-    global_batch_wait_time: Histogram,
-    per_timeline_batch_wait_time: Histogram,
+    // Optional because not all op types are tracked per-timeline
+    per_timeline_latency_histo: Option<Histogram>,
 
     global_flush_in_progress_micros: IntCounter,
     per_timeline_flush_in_progress_micros: IntCounter,
 
-    timings: SmgrOpTimerState,
-}
-
-#[derive(Debug)]
-enum SmgrOpTimerState {
-    Received {
-        received_at: Instant,
-    },
-    ThrottleDoneExecutionStarting {
-        received_at: Instant,
-        throttle_started_at: Instant,
-        started_execution_at: Instant,
-    },
+    start: Instant,
+    throttled: Duration,
+    op: SmgrQueryType,
 }
 
 pub(crate) struct SmgrOpFlushInProgress {
-    flush_started_at: Instant,
+    base: Instant,
     global_micros: IntCounter,
     per_timeline_micros: IntCounter,
 }
 
 impl SmgrOpTimer {
-    pub(crate) fn observe_throttle_done_execution_starting(&mut self, throttle: &ThrottleResult) {
+    pub(crate) fn deduct_throttle(&mut self, throttle: &Option<Duration>) {
+        let Some(throttle) = throttle else {
+            return;
+        };
         let inner = self.0.as_mut().expect("other public methods consume self");
-        match (&mut inner.timings, throttle) {
-            (SmgrOpTimerState::Received { received_at }, throttle) => match throttle {
-                ThrottleResult::NotThrottled { start } => {
-                    inner.timings = SmgrOpTimerState::ThrottleDoneExecutionStarting {
-                        received_at: *received_at,
-                        throttle_started_at: *start,
-                        started_execution_at: *start,
-                    };
-                }
-                ThrottleResult::Throttled { start, end } => {
-                    inner.timings = SmgrOpTimerState::ThrottleDoneExecutionStarting {
-                        received_at: *start,
-                        throttle_started_at: *start,
-                        started_execution_at: *end,
-                    };
-                }
-            },
-            (x, _) => panic!("called in unexpected state: {x:?}"),
-        }
+        inner.throttled += *throttle;
     }
 
     pub(crate) fn observe_smgr_op_completion_and_start_flushing(mut self) -> SmgrOpFlushInProgress {
@@ -1288,7 +1263,7 @@ impl SmgrOpTimer {
             ..
         } = inner;
         SmgrOpFlushInProgress {
-            flush_started_at: flush_start,
+            base: flush_start,
             global_micros: global_flush_in_progress_micros,
             per_timeline_micros: per_timeline_flush_in_progress_micros,
         }
@@ -1299,42 +1274,32 @@ impl SmgrOpTimer {
         let inner = self.0.take()?;
 
         let now = Instant::now();
+        let elapsed = now - inner.start;
 
-        let batch;
-        let execution;
-        let throttle;
-        match inner.timings {
-            SmgrOpTimerState::Received { received_at } => {
-                batch = (now - received_at).as_secs_f64();
-                // TODO: use label for dropped requests.
-                // This is quite rare in practice, only during tenant/pageservers shutdown.
-                throttle = Duration::ZERO;
-                execution = Duration::ZERO.as_secs_f64();
+        let elapsed = match elapsed.checked_sub(inner.throttled) {
+            Some(elapsed) => elapsed,
+            None => {
+                use utils::rate_limit::RateLimit;
+                static LOGGED: Lazy<Mutex<enum_map::EnumMap<SmgrQueryType, RateLimit>>> =
+                    Lazy::new(|| {
+                        Mutex::new(enum_map::EnumMap::from_array(std::array::from_fn(|_| {
+                            RateLimit::new(Duration::from_secs(10))
+                        })))
+                    });
+                let mut guard = LOGGED.lock().unwrap();
+                let rate_limit = &mut guard[inner.op];
+                rate_limit.call(|| {
+                    warn!(op=?inner.op, ?elapsed, ?inner.throttled, "implementation error: time spent throttled exceeds total request wall clock time");
+                });
+                elapsed // un-throttled time, more info than just saturating to 0
             }
-            SmgrOpTimerState::ThrottleDoneExecutionStarting {
-                received_at,
-                throttle_started_at,
-                started_execution_at,
-            } => {
-                batch = (throttle_started_at - received_at).as_secs_f64();
-                throttle = started_execution_at - throttle_started_at;
-                execution = (now - started_execution_at).as_secs_f64();
-            }
-        }
+        };
 
-        // update time spent in batching
-        inner.global_batch_wait_time.observe(batch);
-        inner.per_timeline_batch_wait_time.observe(batch);
+        let elapsed = elapsed.as_secs_f64();
 
-        // time spent in throttle metric is updated by throttle impl
-        let _ = throttle;
-
-        // update metrics for execution latency
-        inner.global_execution_latency_histo.observe(execution);
-        if let Some(per_timeline_execution_latency_histo) =
-            &inner.per_timeline_execution_latency_histo
-        {
-            per_timeline_execution_latency_histo.observe(execution);
+        inner.global_latency_histo.observe(elapsed);
+        if let Some(per_timeline_getpage_histo) = &inner.per_timeline_latency_histo {
+            per_timeline_getpage_histo.observe(elapsed);
         }
 
         Some((now, inner))
@@ -1360,12 +1325,12 @@ impl SmgrOpFlushInProgress {
         // Last call is tracked in `now`.
         let mut observe_guard = scopeguard::guard(
             || {
-                let elapsed = now - self.flush_started_at;
+                let elapsed = now - self.base;
                 self.global_micros
                     .inc_by(u64::try_from(elapsed.as_micros()).unwrap());
                 self.per_timeline_micros
                     .inc_by(u64::try_from(elapsed.as_micros()).unwrap());
-                self.flush_started_at = now;
+                self.base = now;
             },
             |mut observe| {
                 observe();
@@ -1412,8 +1377,6 @@ pub(crate) struct SmgrQueryTimePerTimeline {
     per_timeline_batch_size: Histogram,
     global_flush_in_progress_micros: IntCounter,
     per_timeline_flush_in_progress_micros: IntCounter,
-    global_batch_wait_time: Histogram,
-    per_timeline_batch_wait_time: Histogram,
 }
 
 static SMGR_QUERY_STARTED_GLOBAL: Lazy<IntCounterVec> = Lazy::new(|| {
@@ -1436,15 +1399,12 @@ static SMGR_QUERY_STARTED_PER_TENANT_TIMELINE: Lazy<IntCounterVec> = Lazy::new(|
     .expect("failed to define a metric")
 });
 
-// Alias so all histograms recording per-timeline smgr timings use the same buckets.
-static SMGR_QUERY_TIME_PER_TENANT_TIMELINE_BUCKETS: &[f64] = CRITICAL_OP_BUCKETS;
-
 static SMGR_QUERY_TIME_PER_TENANT_TIMELINE: Lazy<HistogramVec> = Lazy::new(|| {
     register_histogram_vec!(
         "pageserver_smgr_query_seconds",
-        "Time spent _executing_ smgr query handling, excluding batch and throttle delays.",
+        "Time spent on smgr query handling, aggegated by query type and tenant/timeline.",
         &["smgr_query_type", "tenant_id", "shard_id", "timeline_id"],
-        SMGR_QUERY_TIME_PER_TENANT_TIMELINE_BUCKETS.into(),
+        CRITICAL_OP_BUCKETS.into(),
     )
     .expect("failed to define a metric")
 });
@@ -1502,7 +1462,7 @@ static SMGR_QUERY_TIME_GLOBAL_BUCKETS: Lazy<Vec<f64>> = Lazy::new(|| {
 static SMGR_QUERY_TIME_GLOBAL: Lazy<HistogramVec> = Lazy::new(|| {
     register_histogram_vec!(
         "pageserver_smgr_query_seconds_global",
-        "Like pageserver_smgr_query_seconds, but aggregated to instance level.",
+        "Time spent on smgr query handling, aggregated by query type.",
         &["smgr_query_type"],
         SMGR_QUERY_TIME_GLOBAL_BUCKETS.clone(),
     )
@@ -1599,25 +1559,6 @@ static PAGE_SERVICE_SMGR_FLUSH_INPROGRESS_MICROS_GLOBAL: Lazy<IntCounter> = Lazy
     .expect("failed to define a metric")
 });
 
-static PAGE_SERVICE_SMGR_BATCH_WAIT_TIME: Lazy<HistogramVec> = Lazy::new(|| {
-    register_histogram_vec!(
-        "pageserver_page_service_pagestream_batch_wait_time_seconds",
-        "Time a request spent waiting in its batch until the batch moved to throttle&execution.",
-        &["tenant_id", "shard_id", "timeline_id"],
-        SMGR_QUERY_TIME_PER_TENANT_TIMELINE_BUCKETS.into(),
-    )
-    .expect("failed to define a metric")
-});
-
-static PAGE_SERVICE_SMGR_BATCH_WAIT_TIME_GLOBAL: Lazy<Histogram> = Lazy::new(|| {
-    register_histogram!(
-        "pageserver_page_service_pagestream_batch_wait_time_seconds_global",
-        "Like pageserver_page_service_pagestream_batch_wait_time_seconds, but aggregated to instance level.",
-        SMGR_QUERY_TIME_GLOBAL_BUCKETS.to_vec(),
-    )
-    .expect("failed to define a metric")
-});
-
 impl SmgrQueryTimePerTimeline {
     pub(crate) fn new(tenant_shard_id: &TenantShardId, timeline_id: &TimelineId) -> Self {
         let tenant_id = tenant_shard_id.tenant_id.to_string();
@@ -1658,11 +1599,6 @@ impl SmgrQueryTimePerTimeline {
             .get_metric_with_label_values(&[&tenant_id, &shard_slug, &timeline_id])
             .unwrap();
 
-        let global_batch_wait_time = PAGE_SERVICE_SMGR_BATCH_WAIT_TIME_GLOBAL.clone();
-        let per_timeline_batch_wait_time = PAGE_SERVICE_SMGR_BATCH_WAIT_TIME
-            .get_metric_with_label_values(&[&tenant_id, &shard_slug, &timeline_id])
-            .unwrap();
-
         let global_flush_in_progress_micros =
             PAGE_SERVICE_SMGR_FLUSH_INPROGRESS_MICROS_GLOBAL.clone();
         let per_timeline_flush_in_progress_micros = PAGE_SERVICE_SMGR_FLUSH_INPROGRESS_MICROS
@@ -1678,11 +1614,9 @@ impl SmgrQueryTimePerTimeline {
             per_timeline_batch_size,
             global_flush_in_progress_micros,
             per_timeline_flush_in_progress_micros,
-            global_batch_wait_time,
-            per_timeline_batch_wait_time,
         }
     }
-    pub(crate) fn start_smgr_op(&self, op: SmgrQueryType, received_at: Instant) -> SmgrOpTimer {
+    pub(crate) fn start_smgr_op(&self, op: SmgrQueryType, started_at: Instant) -> SmgrOpTimer {
         self.global_started[op as usize].inc();
 
         let per_timeline_latency_histo = if matches!(op, SmgrQueryType::GetPageAtLsn) {
@@ -1693,15 +1627,15 @@ impl SmgrQueryTimePerTimeline {
         };
 
         SmgrOpTimer(Some(SmgrOpTimerInner {
-            global_execution_latency_histo: self.global_latency[op as usize].clone(),
-            per_timeline_execution_latency_histo: per_timeline_latency_histo,
-            timings: SmgrOpTimerState::Received { received_at },
+            global_latency_histo: self.global_latency[op as usize].clone(),
+            per_timeline_latency_histo,
+            start: started_at,
+            op,
+            throttled: Duration::ZERO,
             global_flush_in_progress_micros: self.global_flush_in_progress_micros.clone(),
             per_timeline_flush_in_progress_micros: self
                 .per_timeline_flush_in_progress_micros
                 .clone(),
-            global_batch_wait_time: self.global_batch_wait_time.clone(),
-            per_timeline_batch_wait_time: self.per_timeline_batch_wait_time.clone(),
         }))
     }
 
@@ -2955,11 +2889,6 @@ impl TimelineMetrics {
             shard_id,
             timeline_id,
         ]);
-        let _ = PAGE_SERVICE_SMGR_BATCH_WAIT_TIME.remove_label_values(&[
-            tenant_id,
-            shard_id,
-            timeline_id,
-        ]);
     }
 }
 
@@ -2990,7 +2919,6 @@ use crate::context::{PageContentKind, RequestContext};
 use crate::task_mgr::TaskKind;
 use crate::tenant::mgr::TenantSlot;
 use crate::tenant::tasks::BackgroundLoopKind;
-use crate::tenant::throttle::ThrottleResult;
 use crate::tenant::Timeline;
 
 /// Maintain a per timeline gauge in addition to the global gauge.
@@ -3845,7 +3773,6 @@ pub fn preinitialize_metrics(conf: &'static PageServerConf) {
         &REMOTE_ONDEMAND_DOWNLOADED_BYTES,
         &CIRCUIT_BREAKERS_BROKEN,
         &CIRCUIT_BREAKERS_UNBROKEN,
-        &PAGE_SERVICE_SMGR_FLUSH_INPROGRESS_MICROS_GLOBAL,
     ]
     .into_iter()
     .for_each(|c| {
@@ -3893,7 +3820,6 @@ pub fn preinitialize_metrics(conf: &'static PageServerConf) {
         &WAL_REDO_BYTES_HISTOGRAM,
         &WAL_REDO_PROCESS_LAUNCH_DURATION_HISTOGRAM,
         &PAGE_SERVICE_BATCH_SIZE_GLOBAL,
-        &PAGE_SERVICE_SMGR_BATCH_WAIT_TIME_GLOBAL,
     ]
     .into_iter()
     .for_each(|h| {

pageserver/src/page_service.rs

@@ -575,10 +575,7 @@ enum BatchedFeMessage {
 }
 
 impl BatchedFeMessage {
-    async fn throttle_and_record_start_processing(
-        &mut self,
-        cancel: &CancellationToken,
-    ) -> Result<(), QueryError> {
+    async fn throttle(&mut self, cancel: &CancellationToken) -> Result<(), QueryError> {
         let (shard, tokens, timers) = match self {
             BatchedFeMessage::Exists { shard, timer, .. }
             | BatchedFeMessage::Nblocks { shard, timer, .. }
@@ -606,7 +603,7 @@ impl BatchedFeMessage {
             }
         };
         for timer in timers {
-            timer.observe_throttle_done_execution_starting(&throttled);
+            timer.deduct_throttle(&throttled);
         }
         Ok(())
     }
@@ -1080,12 +1077,12 @@ impl PageServerHandler {
             // what we want to do
             let flush_fut = pgb_writer.flush();
             // metric for how long flushing takes
-            let flush_fut = match flushing_timer {
-                Some(flushing_timer) => {
-                    futures::future::Either::Left(flushing_timer.measure(flush_fut))
-                }
-                None => futures::future::Either::Right(flush_fut),
-            };
+            // let flush_fut = match flushing_timer {
+            //     Some(flushing_timer) => {
+            //         futures::future::Either::Left(flushing_timer.measure(flush_fut))
+            //     }
+            //     None => futures::future::Either::Right(flush_fut),
+            // };
             // do it while respecting cancellation
             let _: () = async move {
                 tokio::select! {
@@ -1102,7 +1099,7 @@ impl PageServerHandler {
                 Ok(())
             }
             // and log the info! line inside the request span
-            .instrument(span.clone())
+            // .instrument(span.clone())
             .await?;
         }
         Ok(())
@@ -1233,7 +1230,7 @@ impl PageServerHandler {
                 }
             };
 
-            if let Err(cancelled) = msg.throttle_and_record_start_processing(&self.cancel).await {
+            if let Err(cancelled) = msg.throttle(&self.cancel).await {
                 break cancelled;
             }
 
@@ -1400,9 +1397,7 @@ impl PageServerHandler {
                             return Err(e);
                         }
                     };
-                    batch
-                        .throttle_and_record_start_processing(&self.cancel)
-                        .await?;
+                    batch.throttle(&self.cancel).await?;
                     self.pagesteam_handle_batched_message(pgb_writer, batch, &cancel, &ctx)
                         .await?;
                 }

pageserver/src/tenant.rs

@@ -68,7 +68,6 @@ use utils::sync::gate::Gate;
 use utils::sync::gate::GateGuard;
 use utils::timeout::timeout_cancellable;
 use utils::timeout::TimeoutCancellableError;
-use utils::try_rcu::ArcSwapExt;
 use utils::zstd::create_zst_tarball;
 use utils::zstd::extract_zst_tarball;
 
@@ -3029,23 +3028,14 @@ impl Tenant {
                                 let mut guard = self.scheduled_compaction_tasks.lock().unwrap();
                                 let tline_pending_tasks = guard.entry(*timeline_id).or_default();
                                 for (idx, job) in jobs.into_iter().enumerate() {
-                                    tline_pending_tasks.push_back(if idx == jobs_len - 1 {
-                                        ScheduledCompactionTask {
-                                            options: job,
-                                            // The last job in the queue sends the signal and releases the gc guard
-                                            result_tx: next_scheduled_compaction_task
-                                                .result_tx
-                                                .take(),
-                                            gc_block: next_scheduled_compaction_task
-                                                .gc_block
-                                                .take(),
-                                        }
-                                    } else {
-                                        ScheduledCompactionTask {
-                                            options: job,
-                                            result_tx: None,
-                                            gc_block: None,
-                                        }
+                                    tline_pending_tasks.push_back(ScheduledCompactionTask {
+                                        options: job,
+                                        result_tx: if idx == jobs_len - 1 {
+                                            // The last compaction job sends the completion signal
+                                            next_scheduled_compaction_task.result_tx.take()
+                                        } else {
+                                            None
+                                        },
                                     });
                                 }
                                 info!("scheduled enhanced gc bottom-most compaction with sub-compaction, split into {} jobs", jobs_len);
@@ -3105,22 +3095,15 @@ impl Tenant {
         &self,
         timeline_id: TimelineId,
         options: CompactOptions,
-    ) -> anyhow::Result<tokio::sync::oneshot::Receiver<()>> {
-        let gc_guard = match self.gc_block.start().await {
-            Ok(guard) => guard,
-            Err(e) => {
-                bail!("cannot run gc-compaction because gc is blocked: {}", e);
-            }
-        };
+    ) -> tokio::sync::oneshot::Receiver<()> {
         let (tx, rx) = tokio::sync::oneshot::channel();
         let mut guard = self.scheduled_compaction_tasks.lock().unwrap();
         let tline_pending_tasks = guard.entry(timeline_id).or_default();
         tline_pending_tasks.push_back(ScheduledCompactionTask {
             options,
             result_tx: Some(tx),
-            gc_block: Some(gc_guard),
         });
-        Ok(rx)
+        rx
     }
 
     // Call through to all timelines to freeze ephemeral layers if needed.  Usually
@@ -3922,28 +3905,25 @@ impl Tenant {
         }
     }
 
-    pub fn update_tenant_config<F: Fn(TenantConfOpt) -> anyhow::Result<TenantConfOpt>>(
-        &self,
-        update: F,
-    ) -> anyhow::Result<TenantConfOpt> {
+    pub fn set_new_tenant_config(&self, new_tenant_conf: TenantConfOpt) {
         // Use read-copy-update in order to avoid overwriting the location config
         // state if this races with [`Tenant::set_new_location_config`]. Note that
         // this race is not possible if both request types come from the storage
         // controller (as they should!) because an exclusive op lock is required
         // on the storage controller side.
 
-        self.tenant_conf
-            .try_rcu(|attached_conf| -> Result<_, anyhow::Error> {
-                Ok(Arc::new(AttachedTenantConf {
-                    tenant_conf: update(attached_conf.tenant_conf.clone())?,
-                    location: attached_conf.location,
-                    lsn_lease_deadline: attached_conf.lsn_lease_deadline,
-                }))
-            })?;
+        self.tenant_conf.rcu(|inner| {
+            Arc::new(AttachedTenantConf {
+                tenant_conf: new_tenant_conf.clone(),
+                location: inner.location,
+                // Attached location is not changed, no need to update lsn lease deadline.
+                lsn_lease_deadline: inner.lsn_lease_deadline,
+            })
+        });
 
-        let updated = self.tenant_conf.load();
+        let updated = self.tenant_conf.load().clone();
 
-        self.tenant_conf_updated(&updated.tenant_conf);
+        self.tenant_conf_updated(&new_tenant_conf);
         // Don't hold self.timelines.lock() during the notifies.
         // There's no risk of deadlock right now, but there could be if we consolidate
         // mutexes in struct Timeline in the future.
@@ -3951,8 +3931,6 @@ impl Tenant {
         for timeline in timelines {
             timeline.tenant_conf_updated(&updated);
         }
-
-        Ok(updated.tenant_conf.clone())
     }
 
     pub(crate) fn set_new_location_config(&self, new_conf: AttachedTenantConf) {
@@ -4512,12 +4490,7 @@ impl Tenant {
                 // - this timeline was created while we were finding cutoffs
                 // - lsn for timestamp search fails for this timeline repeatedly
                 if let Some(cutoffs) = gc_cutoffs.get(&timeline.timeline_id) {
-                    let original_cutoffs = target.cutoffs.clone();
-                    // GC cutoffs should never go back
-                    target.cutoffs = GcCutoffs {
-                        space: Lsn(cutoffs.space.0.max(original_cutoffs.space.0)),
-                        time: Lsn(cutoffs.time.0.max(original_cutoffs.time.0)),
-                    }
+                    target.cutoffs = cutoffs.clone();
                 }
             }
 
@@ -8177,12 +8150,6 @@ mod tests {
             )
             .await?;
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x30))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             guard.cutoffs.time = Lsn(0x30);
@@ -8285,12 +8252,6 @@ mod tests {
 
         // increase GC horizon and compact again
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x40))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             guard.cutoffs.time = Lsn(0x40);
@@ -8671,12 +8632,6 @@ mod tests {
                 .await?
         };
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x30))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             *guard = GcInfo {
@@ -8758,12 +8713,6 @@ mod tests {
 
         // increase GC horizon and compact again
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x40))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             guard.cutoffs.time = Lsn(0x40);
@@ -9211,12 +9160,6 @@ mod tests {
             )
             .await?;
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x30))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             *guard = GcInfo {
@@ -9359,12 +9302,6 @@ mod tests {
 
         // increase GC horizon and compact again
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x38))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             guard.cutoffs.time = Lsn(0x38);
@@ -9460,12 +9397,6 @@ mod tests {
             )
             .await?;
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x30))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             *guard = GcInfo {
@@ -9710,12 +9641,6 @@ mod tests {
         branch_tline.add_extra_test_dense_keyspace(KeySpace::single(get_key(0)..get_key(10)));
 
         {
-            parent_tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x10))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = parent_tline.gc_info.write().unwrap();
             *guard = GcInfo {
@@ -9730,12 +9655,6 @@ mod tests {
         }
 
         {
-            branch_tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x50))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = branch_tline.gc_info.write().unwrap();
             *guard = GcInfo {
@@ -10065,12 +9984,6 @@ mod tests {
             .await?;
 
         {
-            tline
-                .latest_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x30))
-                .wait()
-                .await;
             // Update GC info
             let mut guard = tline.gc_info.write().unwrap();
             *guard = GcInfo {

pageserver/src/tenant/config.rs

@@ -11,7 +11,7 @@
 pub(crate) use pageserver_api::config::TenantConfigToml as TenantConf;
 use pageserver_api::models::CompactionAlgorithmSettings;
 use pageserver_api::models::EvictionPolicy;
-use pageserver_api::models::{self, TenantConfigPatch, ThrottleConfig};
+use pageserver_api::models::{self, ThrottleConfig};
 use pageserver_api::shard::{ShardCount, ShardIdentity, ShardNumber, ShardStripeSize};
 use serde::de::IntoDeserializer;
 use serde::{Deserialize, Serialize};
@@ -427,129 +427,6 @@ impl TenantConfOpt {
                 .or(global_conf.wal_receiver_protocol_override),
         }
     }
-
-    pub fn apply_patch(self, patch: TenantConfigPatch) -> anyhow::Result<TenantConfOpt> {
-        let Self {
-            mut checkpoint_distance,
-            mut checkpoint_timeout,
-            mut compaction_target_size,
-            mut compaction_period,
-            mut compaction_threshold,
-            mut compaction_algorithm,
-            mut gc_horizon,
-            mut gc_period,
-            mut image_creation_threshold,
-            mut pitr_interval,
-            mut walreceiver_connect_timeout,
-            mut lagging_wal_timeout,
-            mut max_lsn_wal_lag,
-            mut eviction_policy,
-            mut min_resident_size_override,
-            mut evictions_low_residence_duration_metric_threshold,
-            mut heatmap_period,
-            mut lazy_slru_download,
-            mut timeline_get_throttle,
-            mut image_layer_creation_check_threshold,
-            mut lsn_lease_length,
-            mut lsn_lease_length_for_ts,
-            mut timeline_offloading,
-            mut wal_receiver_protocol_override,
-        } = self;
-
-        patch.checkpoint_distance.apply(&mut checkpoint_distance);
-        patch
-            .checkpoint_timeout
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut checkpoint_timeout);
-        patch
-            .compaction_target_size
-            .apply(&mut compaction_target_size);
-        patch
-            .compaction_period
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut compaction_period);
-        patch.compaction_threshold.apply(&mut compaction_threshold);
-        patch.compaction_algorithm.apply(&mut compaction_algorithm);
-        patch.gc_horizon.apply(&mut gc_horizon);
-        patch
-            .gc_period
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut gc_period);
-        patch
-            .image_creation_threshold
-            .apply(&mut image_creation_threshold);
-        patch
-            .pitr_interval
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut pitr_interval);
-        patch
-            .walreceiver_connect_timeout
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut walreceiver_connect_timeout);
-        patch
-            .lagging_wal_timeout
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut lagging_wal_timeout);
-        patch.max_lsn_wal_lag.apply(&mut max_lsn_wal_lag);
-        patch.eviction_policy.apply(&mut eviction_policy);
-        patch
-            .min_resident_size_override
-            .apply(&mut min_resident_size_override);
-        patch
-            .evictions_low_residence_duration_metric_threshold
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut evictions_low_residence_duration_metric_threshold);
-        patch
-            .heatmap_period
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut heatmap_period);
-        patch.lazy_slru_download.apply(&mut lazy_slru_download);
-        patch
-            .timeline_get_throttle
-            .apply(&mut timeline_get_throttle);
-        patch
-            .image_layer_creation_check_threshold
-            .apply(&mut image_layer_creation_check_threshold);
-        patch
-            .lsn_lease_length
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut lsn_lease_length);
-        patch
-            .lsn_lease_length_for_ts
-            .map(|v| humantime::parse_duration(&v))?
-            .apply(&mut lsn_lease_length_for_ts);
-        patch.timeline_offloading.apply(&mut timeline_offloading);
-        patch
-            .wal_receiver_protocol_override
-            .apply(&mut wal_receiver_protocol_override);
-
-        Ok(Self {
-            checkpoint_distance,
-            checkpoint_timeout,
-            compaction_target_size,
-            compaction_period,
-            compaction_threshold,
-            compaction_algorithm,
-            gc_horizon,
-            gc_period,
-            image_creation_threshold,
-            pitr_interval,
-            walreceiver_connect_timeout,
-            lagging_wal_timeout,
-            max_lsn_wal_lag,
-            eviction_policy,
-            min_resident_size_override,
-            evictions_low_residence_duration_metric_threshold,
-            heatmap_period,
-            lazy_slru_download,
-            timeline_get_throttle,
-            image_layer_creation_check_threshold,
-            lsn_lease_length,
-            lsn_lease_length_for_ts,
-            timeline_offloading,
-            wal_receiver_protocol_override,
-        })
-    }
 }
 
 impl TryFrom<&'_ models::TenantConfig> for TenantConfOpt {

pageserver/src/tenant/gc_block.rs

@@ -1,4 +1,4 @@
-use std::{collections::HashMap, sync::Arc};
+use std::collections::HashMap;
 
 use utils::id::TimelineId;
 
@@ -20,7 +20,7 @@ pub(crate) struct GcBlock {
     /// 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: Arc<tokio::sync::Mutex<()>>,
+    blocking: tokio::sync::Mutex<()>,
 }
 
 impl GcBlock {
@@ -30,7 +30,7 @@ impl GcBlock {
     /// it's ending, or if not currently possible, a value describing the reasons why not.
     ///
     /// Cancellation safe.
-    pub(super) async fn start(&self) -> Result<Guard, BlockingReasons> {
+    pub(super) async fn start(&self) -> Result<Guard<'_>, BlockingReasons> {
         let reasons = {
             let g = self.reasons.lock().unwrap();
 
@@ -44,7 +44,7 @@ impl GcBlock {
             Err(reasons)
         } else {
             Ok(Guard {
-                _inner: self.blocking.clone().lock_owned().await,
+                _inner: self.blocking.lock().await,
             })
         }
     }
@@ -170,8 +170,8 @@ impl GcBlock {
     }
 }
 
-pub(crate) struct Guard {
-    _inner: tokio::sync::OwnedMutexGuard<()>,
+pub(super) struct Guard<'a> {
+    _inner: tokio::sync::MutexGuard<'a, ()>,
 }
 
 #[derive(Debug)]

pageserver/src/tenant/throttle.rs

@@ -58,11 +58,6 @@ pub struct Stats {
     pub sum_throttled_usecs: u64,
 }
 
-pub enum ThrottleResult {
-    NotThrottled { start: Instant },
-    Throttled { start: Instant, end: Instant },
-}
-
 impl<M> Throttle<M>
 where
     M: Metric,
@@ -127,15 +122,15 @@ where
         self.inner.load().rate_limiter.steady_rps()
     }
 
-    pub async fn throttle(&self, key_count: usize) -> ThrottleResult {
+    pub async fn throttle(&self, key_count: usize) -> Option<Duration> {
         let inner = self.inner.load_full(); // clones the `Inner` Arc
 
-        let start = std::time::Instant::now();
-
         if !inner.enabled {
-            return ThrottleResult::NotThrottled { start };
+            return None;
         }
 
+        let start = std::time::Instant::now();
+
         self.metric.accounting_start();
         self.count_accounted_start.fetch_add(1, Ordering::Relaxed);
         let did_throttle = inner.rate_limiter.acquire(key_count).await;
@@ -150,9 +145,9 @@ where
                 .fetch_add(wait_time.as_micros() as u64, Ordering::Relaxed);
             let observation = Observation { wait_time };
             self.metric.observe_throttling(&observation);
-            ThrottleResult::Throttled { start, end: now }
+            Some(wait_time)
         } else {
-            ThrottleResult::NotThrottled { start }
+            None
         }
     }
 }

pageserver/src/tenant/timeline/compaction.rs

@@ -41,7 +41,7 @@ use crate::tenant::storage_layer::{
 use crate::tenant::timeline::ImageLayerCreationOutcome;
 use crate::tenant::timeline::{drop_rlock, DeltaLayerWriter, ImageLayerWriter};
 use crate::tenant::timeline::{Layer, ResidentLayer};
-use crate::tenant::{gc_block, DeltaLayer, MaybeOffloaded};
+use crate::tenant::{DeltaLayer, MaybeOffloaded};
 use crate::virtual_file::{MaybeFatalIo, VirtualFile};
 use pageserver_api::config::tenant_conf_defaults::{
     DEFAULT_CHECKPOINT_DISTANCE, DEFAULT_COMPACTION_THRESHOLD,
@@ -63,12 +63,9 @@ use super::CompactionError;
 const COMPACTION_DELTA_THRESHOLD: usize = 5;
 
 /// A scheduled compaction task.
-pub(crate) struct ScheduledCompactionTask {
+pub struct ScheduledCompactionTask {
     pub options: CompactOptions,
-    /// The channel to send the compaction result. If this is a subcompaction, the last compaction job holds the sender.
     pub result_tx: Option<tokio::sync::oneshot::Sender<()>>,
-    /// Hold the GC block. If this is a subcompaction, the last compaction job holds the gc block guard.
-    pub gc_block: Option<gc_block::Guard>,
 }
 
 pub struct GcCompactionJobDescription {
@@ -1771,7 +1768,8 @@ impl Timeline {
         let compact_below_lsn = if let Some(compact_below_lsn) = options.compact_below_lsn {
             compact_below_lsn
         } else {
-            *self.get_latest_gc_cutoff_lsn() // use the real gc cutoff
+            let gc_info = self.gc_info.read().unwrap();
+            gc_info.cutoffs.select_min() // use the real gc cutoff
         };
         let mut compact_jobs = Vec::new();
         // For now, we simply use the key partitioning information; we should do a more fine-grained partitioning
@@ -1964,11 +1962,7 @@ impl Timeline {
             let gc_info = self.gc_info.read().unwrap();
             let mut retain_lsns_below_horizon = Vec::new();
             let gc_cutoff = {
-                // Currently, gc-compaction only kicks in after the legacy gc has updated the gc_cutoff.
-                // Therefore, it can only clean up data that cannot be cleaned up with legacy gc, instead of
-                // cleaning everything that theoritically it could. In the future, it should use `self.gc_info`
-                // to get the truth data.
-                let real_gc_cutoff = *self.get_latest_gc_cutoff_lsn();
+                let real_gc_cutoff = gc_info.cutoffs.select_min();
                 // The compaction algorithm will keep all keys above the gc_cutoff while keeping only necessary keys below the gc_cutoff for
                 // each of the retain_lsn. Therefore, if the user-provided `compact_below_lsn` is larger than the real gc cutoff, we will use
                 // the real cutoff.

pageserver/src/walingest.rs

@@ -877,24 +877,22 @@ impl WalIngest {
         // will block waiting for the last valid LSN to advance up to
         // it. So we use the previous record's LSN in the get calls
         // instead.
-        if modification.tline.get_shard_identity().is_shard_zero() {
-            for segno in modification
-                .tline
-                .list_slru_segments(SlruKind::Clog, Version::Modified(modification), ctx)
-                .await?
-            {
-                let segpage = segno * pg_constants::SLRU_PAGES_PER_SEGMENT;
+        for segno in modification
+            .tline
+            .list_slru_segments(SlruKind::Clog, Version::Modified(modification), ctx)
+            .await?
+        {
+            let segpage = segno * pg_constants::SLRU_PAGES_PER_SEGMENT;
 
-                let may_delete = dispatch_pgversion!(modification.tline.pg_version, {
-                    pgv::nonrelfile_utils::slru_may_delete_clogsegment(segpage, pageno)
-                });
+            let may_delete = dispatch_pgversion!(modification.tline.pg_version, {
+                pgv::nonrelfile_utils::slru_may_delete_clogsegment(segpage, pageno)
+            });
 
-                if may_delete {
-                    modification
-                        .drop_slru_segment(SlruKind::Clog, segno, ctx)
-                        .await?;
-                    trace!("Drop CLOG segment {:>04X}", segno);
-                }
+            if may_delete {
+                modification
+                    .drop_slru_segment(SlruKind::Clog, segno, ctx)
+                    .await?;
+                trace!("Drop CLOG segment {:>04X}", segno);
             }
         }
 
@@ -1049,18 +1047,16 @@ impl WalIngest {
 
         // Delete all the segments except the last one. The last segment can still
         // contain, possibly partially, valid data.
-        if modification.tline.get_shard_identity().is_shard_zero() {
-            while segment != endsegment {
-                modification
-                    .drop_slru_segment(SlruKind::MultiXactMembers, segment as u32, ctx)
-                    .await?;
+        while segment != endsegment {
+            modification
+                .drop_slru_segment(SlruKind::MultiXactMembers, segment as u32, ctx)
+                .await?;
 
-                /* move to next segment, handling wraparound correctly */
-                if segment == maxsegment {
-                    segment = 0;
-                } else {
-                    segment += 1;
-                }
+            /* move to next segment, handling wraparound correctly */
+            if segment == maxsegment {
+                segment = 0;
+            } else {
+                segment += 1;
             }
         }
 

pgxn/neon/libpagestore.c

@@ -22,7 +22,6 @@
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
 #include "pgstat.h"
-#include "portability/instr_time.h"
 #include "postmaster/interrupt.h"
 #include "storage/buf_internals.h"
 #include "storage/ipc.h"
@@ -119,11 +118,6 @@ typedef struct
 	 */
 	PSConnectionState state;
 	PGconn		   *conn;
-
-	/* request / response counters for debugging */
-	uint64			nrequests_sent;
-	uint64			nresponses_received;
-
 	/*---
 	 * WaitEventSet containing:
 	 *	- WL_SOCKET_READABLE on 'conn'
@@ -634,8 +628,6 @@ pageserver_connect(shardno_t shard_no, int elevel)
 		}
 
 		shard->state = PS_Connected;
-		shard->nrequests_sent = 0;
-		shard->nresponses_received = 0;
 	}
 	/* FALLTHROUGH */
 	case PS_Connected:
@@ -664,27 +656,6 @@ call_PQgetCopyData(shardno_t shard_no, char **buffer)
 	int			ret;
 	PageServer *shard = &page_servers[shard_no];
 	PGconn	   *pageserver_conn = shard->conn;
-	instr_time	now,
-				start_ts,
-				since_start,
-				last_log_ts,
-				since_last_log;
-	bool		logged = false;
-
-	/*
-	 * As a debugging aid, if we don't get a response for a long time, print a
-	 * log message.
-	 *
-	 * 10 s is a very generous threshold, normally we expect a response in a
-	 * few milliseconds. We have metrics to track latencies in normal ranges,
-	 * but in the cases that take exceptionally long, it's useful to log the
-	 * exact timestamps.
-	 */
-#define LOG_INTERVAL_US		UINT64CONST(10 * 1000000)
-
-	INSTR_TIME_SET_CURRENT(now);
-	start_ts = last_log_ts = now;
-	INSTR_TIME_SET_ZERO(since_last_log);
 
 retry:
 	ret = PQgetCopyData(pageserver_conn, buffer, 1 /* async */ );
@@ -692,12 +663,9 @@ retry:
 	if (ret == 0)
 	{
 		WaitEvent	event;
-		long		timeout;
-
-		timeout = Min(0, LOG_INTERVAL_US - INSTR_TIME_GET_MICROSEC(since_last_log));
 
 		/* Sleep until there's something to do */
-		(void) WaitEventSetWait(shard->wes_read, timeout, &event, 1,
+		(void) WaitEventSetWait(shard->wes_read, -1L, &event, 1,
 								WAIT_EVENT_NEON_PS_READ);
 		ResetLatch(MyLatch);
 
@@ -716,40 +684,9 @@ retry:
 			}
 		}
 
-		/*
-		 * Print a message to the log if a long time has passed with no
-		 * response.
-		 */
-		INSTR_TIME_SET_CURRENT(now);
-		since_last_log = now;
-		INSTR_TIME_SUBTRACT(since_last_log, last_log_ts);
-		if (INSTR_TIME_GET_MICROSEC(since_last_log) >= LOG_INTERVAL_US)
-		{
-			since_start = now;
-			INSTR_TIME_SUBTRACT(since_start, start_ts);
-			neon_shard_log(shard_no, LOG, "no response received from pageserver for %0.3f s, still waiting (sent " UINT64_FORMAT " requests, received " UINT64_FORMAT " responses)",
-						   INSTR_TIME_GET_DOUBLE(since_start),
-						   shard->nrequests_sent, shard->nresponses_received);
-			last_log_ts = now;
-			logged = true;
-		}
-
 		goto retry;
 	}
 
-	/*
-	 * If we logged earlier that the response is taking a long time, log
-	 * another message when the response is finally received.
-	 */
-	if (logged)
-	{
-		INSTR_TIME_SET_CURRENT(now);
-		since_start = now;
-		INSTR_TIME_SUBTRACT(since_start, start_ts);
-		neon_shard_log(shard_no, LOG, "received response from pageserver after %0.3f s",
-					   INSTR_TIME_GET_DOUBLE(since_start));
-	}
-
 	return ret;
 }
 
@@ -849,7 +786,6 @@ pageserver_send(shardno_t shard_no, NeonRequest *request)
 	 * PGRES_POLLING_WRITING state. It's kinda dirty to disconnect at this
 	 * point, but on the grand scheme of things it's only a small issue.
 	 */
-	shard->nrequests_sent++;
 	if (PQputCopyData(pageserver_conn, req_buff.data, req_buff.len) <= 0)
 	{
 		char	   *msg = pchomp(PQerrorMessage(pageserver_conn));
@@ -942,7 +878,6 @@ pageserver_receive(shardno_t shard_no)
 		neon_shard_log(shard_no, ERROR, "pageserver_receive disconnect: unexpected PQgetCopyData return value: %d", rc);
 	}
 
-	shard->nresponses_received++;
 	return (NeonResponse *) resp;
 }
 

pgxn/neon/pagestore_smgr.c

@@ -423,11 +423,7 @@ readahead_buffer_resize(int newsize, void *extra)
 	 * ensuring we have received all but the last n requests (n = newsize).
 	 */
 	if (MyPState->n_requests_inflight > newsize)
-	{
-		Assert(MyPState->ring_unused >= MyPState->n_requests_inflight - newsize);
-		prefetch_wait_for(MyPState->ring_unused - (MyPState->n_requests_inflight - newsize));
-		Assert(MyPState->n_requests_inflight <= newsize);
-	}
+		prefetch_wait_for(MyPState->ring_unused - newsize);
 
 	/* construct the new PrefetchState, and copy over the memory contexts */
 	newPState = MemoryContextAllocZero(TopMemoryContext, newprfs_size);
@@ -442,6 +438,7 @@ readahead_buffer_resize(int newsize, void *extra)
 	newPState->ring_last = newsize;
 	newPState->ring_unused = newsize;
 	newPState->ring_receive = newsize;
+	newPState->ring_flush = newsize;
 	newPState->max_shard_no = MyPState->max_shard_no;
 	memcpy(newPState->shard_bitmap, MyPState->shard_bitmap, sizeof(MyPState->shard_bitmap));
 
@@ -492,7 +489,6 @@ readahead_buffer_resize(int newsize, void *extra)
 		}
 		newPState->n_unused -= 1;
 	}
-	newPState->ring_flush = newPState->ring_receive;
 
 	MyNeonCounters->getpage_prefetches_buffered =
 		MyPState->n_responses_buffered;
@@ -502,7 +498,6 @@ readahead_buffer_resize(int newsize, void *extra)
 	for (; end >= MyPState->ring_last && end != UINT64_MAX; end -= 1)
 	{
 		PrefetchRequest *slot = GetPrfSlot(end);
-		Assert(slot->status != PRFS_REQUESTED);
 		if (slot->status == PRFS_RECEIVED)
 		{
 			pfree(slot->response);

proxy/src/cancellation.rs

@@ -115,8 +115,7 @@ impl<P: CancellationPublisher> CancellationHandler<P> {
                 IpAddr::V6(ip) => IpNet::V6(Ipv6Net::new_assert(ip, 64).trunc()),
             };
             if !self.limiter.lock().unwrap().check(subnet_key, 1) {
-                // log only the subnet part of the IP address to know which subnet is rate limited
-                tracing::warn!("Rate limit exceeded. Skipping cancellation message, {subnet_key}");
+                tracing::debug!("Rate limit exceeded. Skipping cancellation message");
                 Metrics::get()
                     .proxy
                     .cancellation_requests_total

proxy/src/console_redirect_proxy.rs

@@ -163,36 +163,32 @@ pub(crate) async fn handle_client<S: AsyncRead + AsyncWrite + Unpin>(
     let pause = ctx.latency_timer_pause(crate::metrics::Waiting::Client);
     let do_handshake = handshake(ctx, stream, tls, record_handshake_error);
 
-    let (mut stream, params) = match tokio::time::timeout(config.handshake_timeout, do_handshake)
-        .await??
-    {
-        HandshakeData::Startup(stream, params) => (stream, params),
-        HandshakeData::Cancel(cancel_key_data) => {
-            // spawn a task to cancel the session, but don't wait for it
-            cancellations.spawn({
-                let cancellation_handler_clone = Arc::clone(&cancellation_handler);
-                let session_id = ctx.session_id();
-                let peer_ip = ctx.peer_addr();
-                let cancel_span = tracing::span!(parent: None, tracing::Level::INFO, "cancel_session", session_id = ?session_id);
-                cancel_span.follows_from(tracing::Span::current());
-                async move {
-                    drop(
-                        cancellation_handler_clone
-                            .cancel_session(
-                                cancel_key_data,
-                                session_id,
-                                peer_ip,
-                                config.authentication_config.ip_allowlist_check_enabled,
-                            )
-                            .instrument(cancel_span)
-                            .await,
-                    );
-                }
-            });
+    let (mut stream, params) =
+        match tokio::time::timeout(config.handshake_timeout, do_handshake).await?? {
+            HandshakeData::Startup(stream, params) => (stream, params),
+            HandshakeData::Cancel(cancel_key_data) => {
+                // spawn a task to cancel the session, but don't wait for it
+                cancellations.spawn({
+                    let cancellation_handler_clone = Arc::clone(&cancellation_handler);
+                    let session_id = ctx.session_id();
+                    let peer_ip = ctx.peer_addr();
+                    async move {
+                        drop(
+                            cancellation_handler_clone
+                                .cancel_session(
+                                    cancel_key_data,
+                                    session_id,
+                                    peer_ip,
+                                    config.authentication_config.ip_allowlist_check_enabled,
+                                )
+                                .await,
+                        );
+                    }
+                });
 
-            return Ok(None);
-        }
-    };
+                return Ok(None);
+            }
+        };
     drop(pause);
 
     ctx.set_db_options(params.clone());

proxy/src/proxy/mod.rs

@@ -272,36 +272,32 @@ pub(crate) async fn handle_client<S: AsyncRead + AsyncWrite + Unpin>(
     let pause = ctx.latency_timer_pause(crate::metrics::Waiting::Client);
     let do_handshake = handshake(ctx, stream, mode.handshake_tls(tls), record_handshake_error);
 
-    let (mut stream, params) = match tokio::time::timeout(config.handshake_timeout, do_handshake)
-        .await??
-    {
-        HandshakeData::Startup(stream, params) => (stream, params),
-        HandshakeData::Cancel(cancel_key_data) => {
-            // spawn a task to cancel the session, but don't wait for it
-            cancellations.spawn({
-                let cancellation_handler_clone = Arc::clone(&cancellation_handler);
-                let session_id = ctx.session_id();
-                let peer_ip = ctx.peer_addr();
-                let cancel_span = tracing::span!(parent: None, tracing::Level::INFO, "cancel_session", session_id = ?session_id);
-                cancel_span.follows_from(tracing::Span::current());
-                async move {
-                    drop(
-                        cancellation_handler_clone
-                            .cancel_session(
-                                cancel_key_data,
-                                session_id,
-                                peer_ip,
-                                config.authentication_config.ip_allowlist_check_enabled,
-                            )
-                            .instrument(cancel_span)
-                            .await,
-                    );
-                }
-            });
+    let (mut stream, params) =
+        match tokio::time::timeout(config.handshake_timeout, do_handshake).await?? {
+            HandshakeData::Startup(stream, params) => (stream, params),
+            HandshakeData::Cancel(cancel_key_data) => {
+                // spawn a task to cancel the session, but don't wait for it
+                cancellations.spawn({
+                    let cancellation_handler_clone = Arc::clone(&cancellation_handler);
+                    let session_id = ctx.session_id();
+                    let peer_ip = ctx.peer_addr();
+                    async move {
+                        drop(
+                            cancellation_handler_clone
+                                .cancel_session(
+                                    cancel_key_data,
+                                    session_id,
+                                    peer_ip,
+                                    config.authentication_config.ip_allowlist_check_enabled,
+                                )
+                                .await,
+                        );
+                    }
+                });
 
-            return Ok(None);
-        }
-    };
+                return Ok(None);
+            }
+        };
     drop(pause);
 
     ctx.set_db_options(params.clone());

proxy/src/redis/notifications.rs

@@ -13,7 +13,6 @@ use crate::cache::project_info::ProjectInfoCache;
 use crate::cancellation::{CancelMap, CancellationHandler};
 use crate::intern::{ProjectIdInt, RoleNameInt};
 use crate::metrics::{Metrics, RedisErrors, RedisEventsCount};
-use tracing::Instrument;
 
 const CPLANE_CHANNEL_NAME: &str = "neondb-proxy-ws-updates";
 pub(crate) const PROXY_CHANNEL_NAME: &str = "neondb-proxy-to-proxy-updates";
@@ -144,8 +143,6 @@ impl<C: ProjectInfoCache + Send + Sync + 'static> MessageHandler<C> {
                 let peer_addr = cancel_session
                     .peer_addr
                     .unwrap_or(std::net::IpAddr::V4(std::net::Ipv4Addr::UNSPECIFIED));
-                let cancel_span = tracing::span!(parent: None, tracing::Level::INFO, "cancel_session", session_id = ?cancel_session.session_id);
-                cancel_span.follows_from(tracing::Span::current());
                 // This instance of cancellation_handler doesn't have a RedisPublisherClient so it can't publish the message.
                 match self
                     .cancellation_handler
@@ -155,7 +152,6 @@ impl<C: ProjectInfoCache + Send + Sync + 'static> MessageHandler<C> {
                         peer_addr,
                         cancel_session.peer_addr.is_some(),
                     )
-                    .instrument(cancel_span)
                     .await
                 {
                     Ok(()) => {}

safekeeper/benches/benchutils.rs

@@ -83,20 +83,14 @@ impl Env {
         node_id: NodeId,
         ttid: TenantTimelineId,
     ) -> anyhow::Result<Arc<Timeline>> {
-        let conf = Arc::new(self.make_conf(node_id));
+        let conf = self.make_conf(node_id);
         let timeline_dir = get_timeline_dir(&conf, &ttid);
         let remote_path = remote_timeline_path(&ttid)?;
 
         let safekeeper = self.make_safekeeper(node_id, ttid).await?;
         let shared_state = SharedState::new(StateSK::Loaded(safekeeper));
 
-        let timeline = Timeline::new(
-            ttid,
-            &timeline_dir,
-            &remote_path,
-            shared_state,
-            conf.clone(),
-        );
+        let timeline = Timeline::new(ttid, &timeline_dir, &remote_path, shared_state);
         timeline.bootstrap(
             &mut timeline.write_shared_state().await,
             &conf,

safekeeper/src/bin/safekeeper.rs

@@ -338,7 +338,7 @@ async fn main() -> anyhow::Result<()> {
         }
     };
 
-    let conf = Arc::new(SafeKeeperConf {
+    let conf = SafeKeeperConf {
         workdir,
         my_id: id,
         listen_pg_addr: args.listen_pg,
@@ -368,7 +368,7 @@ async fn main() -> anyhow::Result<()> {
         control_file_save_interval: args.control_file_save_interval,
         partial_backup_concurrency: args.partial_backup_concurrency,
         eviction_min_resident: args.eviction_min_resident,
-    });
+    };
 
     // initialize sentry if SENTRY_DSN is provided
     let _sentry_guard = init_sentry(
@@ -382,7 +382,7 @@ async fn main() -> anyhow::Result<()> {
 /// complete, e.g. panicked, inner is error produced by task itself.
 type JoinTaskRes = Result<anyhow::Result<()>, JoinError>;
 
-async fn start_safekeeper(conf: Arc<SafeKeeperConf>) -> Result<()> {
+async fn start_safekeeper(conf: SafeKeeperConf) -> Result<()> {
     // fsync the datadir to make sure we have a consistent state on disk.
     if !conf.no_sync {
         let dfd = File::open(&conf.workdir).context("open datadir for syncfs")?;
@@ -428,11 +428,9 @@ async fn start_safekeeper(conf: Arc<SafeKeeperConf>) -> Result<()> {
         e
     })?;
 
-    let global_timelines = Arc::new(GlobalTimelines::new(conf.clone()));
-
     // Register metrics collector for active timelines. It's important to do this
     // after daemonizing, otherwise process collector will be upset.
-    let timeline_collector = safekeeper::metrics::TimelineCollector::new(global_timelines.clone());
+    let timeline_collector = safekeeper::metrics::TimelineCollector::new();
     metrics::register_internal(Box::new(timeline_collector))?;
 
     wal_backup::init_remote_storage(&conf).await;
@@ -449,8 +447,9 @@ async fn start_safekeeper(conf: Arc<SafeKeeperConf>) -> Result<()> {
         .then(|| Handle::try_current().expect("no runtime in main"));
 
     // Load all timelines from disk to memory.
-    global_timelines.init().await?;
+    GlobalTimelines::init(conf.clone()).await?;
 
+    let conf_ = conf.clone();
     // Run everything in current thread rt, if asked.
     if conf.current_thread_runtime {
         info!("running in current thread runtime");
@@ -460,16 +459,14 @@ async fn start_safekeeper(conf: Arc<SafeKeeperConf>) -> Result<()> {
         .as_ref()
         .unwrap_or_else(|| WAL_SERVICE_RUNTIME.handle())
         .spawn(wal_service::task_main(
-            conf.clone(),
+            conf_,
             pg_listener,
             Scope::SafekeeperData,
-            global_timelines.clone(),
         ))
         // wrap with task name for error reporting
         .map(|res| ("WAL service main".to_owned(), res));
     tasks_handles.push(Box::pin(wal_service_handle));
 
-    let global_timelines_ = global_timelines.clone();
     let timeline_housekeeping_handle = current_thread_rt
         .as_ref()
         .unwrap_or_else(|| WAL_SERVICE_RUNTIME.handle())
@@ -477,45 +474,40 @@ async fn start_safekeeper(conf: Arc<SafeKeeperConf>) -> Result<()> {
             const TOMBSTONE_TTL: Duration = Duration::from_secs(3600 * 24);
             loop {
                 tokio::time::sleep(TOMBSTONE_TTL).await;
-                global_timelines_.housekeeping(&TOMBSTONE_TTL);
+                GlobalTimelines::housekeeping(&TOMBSTONE_TTL);
             }
         })
         .map(|res| ("Timeline map housekeeping".to_owned(), res));
     tasks_handles.push(Box::pin(timeline_housekeeping_handle));
 
     if let Some(pg_listener_tenant_only) = pg_listener_tenant_only {
+        let conf_ = conf.clone();
         let wal_service_handle = current_thread_rt
             .as_ref()
             .unwrap_or_else(|| WAL_SERVICE_RUNTIME.handle())
             .spawn(wal_service::task_main(
-                conf.clone(),
+                conf_,
                 pg_listener_tenant_only,
                 Scope::Tenant,
-                global_timelines.clone(),
             ))
             // wrap with task name for error reporting
             .map(|res| ("WAL service tenant only main".to_owned(), res));
         tasks_handles.push(Box::pin(wal_service_handle));
     }
 
+    let conf_ = conf.clone();
     let http_handle = current_thread_rt
         .as_ref()
         .unwrap_or_else(|| HTTP_RUNTIME.handle())
-        .spawn(http::task_main(
-            conf.clone(),
-            http_listener,
-            global_timelines.clone(),
-        ))
+        .spawn(http::task_main(conf_, http_listener))
         .map(|res| ("HTTP service main".to_owned(), res));
     tasks_handles.push(Box::pin(http_handle));
 
+    let conf_ = conf.clone();
     let broker_task_handle = current_thread_rt
         .as_ref()
         .unwrap_or_else(|| BROKER_RUNTIME.handle())
-        .spawn(
-            broker::task_main(conf.clone(), global_timelines.clone())
-                .instrument(info_span!("broker")),
-        )
+        .spawn(broker::task_main(conf_).instrument(info_span!("broker")))
         .map(|res| ("broker main".to_owned(), res));
     tasks_handles.push(Box::pin(broker_task_handle));
 

safekeeper/src/broker.rs

@@ -39,17 +39,14 @@ const RETRY_INTERVAL_MSEC: u64 = 1000;
 const PUSH_INTERVAL_MSEC: u64 = 1000;
 
 /// Push once in a while data about all active timelines to the broker.
-async fn push_loop(
-    conf: Arc<SafeKeeperConf>,
-    global_timelines: Arc<GlobalTimelines>,
-) -> anyhow::Result<()> {
+async fn push_loop(conf: SafeKeeperConf) -> anyhow::Result<()> {
     if conf.disable_periodic_broker_push {
         info!("broker push_loop is disabled, doing nothing...");
         futures::future::pending::<()>().await; // sleep forever
         return Ok(());
     }
 
-    let active_timelines_set = global_timelines.get_global_broker_active_set();
+    let active_timelines_set = GlobalTimelines::get_global_broker_active_set();
 
     let mut client =
         storage_broker::connect(conf.broker_endpoint.clone(), conf.broker_keepalive_interval)?;
@@ -90,13 +87,8 @@ async fn push_loop(
 
 /// Subscribe and fetch all the interesting data from the broker.
 #[instrument(name = "broker_pull", skip_all)]
-async fn pull_loop(
-    conf: Arc<SafeKeeperConf>,
-    global_timelines: Arc<GlobalTimelines>,
-    stats: Arc<BrokerStats>,
-) -> Result<()> {
-    let mut client =
-        storage_broker::connect(conf.broker_endpoint.clone(), conf.broker_keepalive_interval)?;
+async fn pull_loop(conf: SafeKeeperConf, stats: Arc<BrokerStats>) -> Result<()> {
+    let mut client = storage_broker::connect(conf.broker_endpoint, conf.broker_keepalive_interval)?;
 
     // TODO: subscribe only to local timelines instead of all
     let request = SubscribeSafekeeperInfoRequest {
@@ -121,7 +113,7 @@ async fn pull_loop(
             .as_ref()
             .ok_or_else(|| anyhow!("missing tenant_timeline_id"))?;
         let ttid = parse_proto_ttid(proto_ttid)?;
-        if let Ok(tli) = global_timelines.get(ttid) {
+        if let Ok(tli) = GlobalTimelines::get(ttid) {
             // Note that we also receive *our own* info. That's
             // important, as it is used as an indication of live
             // connection to the broker.
@@ -143,11 +135,7 @@ async fn pull_loop(
 
 /// Process incoming discover requests. This is done in a separate task to avoid
 /// interfering with the normal pull/push loops.
-async fn discover_loop(
-    conf: Arc<SafeKeeperConf>,
-    global_timelines: Arc<GlobalTimelines>,
-    stats: Arc<BrokerStats>,
-) -> Result<()> {
+async fn discover_loop(conf: SafeKeeperConf, stats: Arc<BrokerStats>) -> Result<()> {
     let mut client =
         storage_broker::connect(conf.broker_endpoint.clone(), conf.broker_keepalive_interval)?;
 
@@ -183,7 +171,7 @@ async fn discover_loop(
                     .as_ref()
                     .ok_or_else(|| anyhow!("missing tenant_timeline_id"))?;
                 let ttid = parse_proto_ttid(proto_ttid)?;
-                if let Ok(tli) = global_timelines.get(ttid) {
+                if let Ok(tli) = GlobalTimelines::get(ttid) {
                     // we received a discovery request for a timeline we know about
                     discover_counter.inc();
 
@@ -222,10 +210,7 @@ async fn discover_loop(
     bail!("end of stream");
 }
 
-pub async fn task_main(
-    conf: Arc<SafeKeeperConf>,
-    global_timelines: Arc<GlobalTimelines>,
-) -> anyhow::Result<()> {
+pub async fn task_main(conf: SafeKeeperConf) -> anyhow::Result<()> {
     info!("started, broker endpoint {:?}", conf.broker_endpoint);
 
     let mut ticker = tokio::time::interval(Duration::from_millis(RETRY_INTERVAL_MSEC));
@@ -276,13 +261,13 @@ pub async fn task_main(
                 },
                 _ = ticker.tick() => {
                     if push_handle.is_none() {
-                        push_handle = Some(tokio::spawn(push_loop(conf.clone(), global_timelines.clone())));
+                        push_handle = Some(tokio::spawn(push_loop(conf.clone())));
                     }
                     if pull_handle.is_none() {
-                        pull_handle = Some(tokio::spawn(pull_loop(conf.clone(), global_timelines.clone(), stats.clone())));
+                        pull_handle = Some(tokio::spawn(pull_loop(conf.clone(), stats.clone())));
                     }
                     if discover_handle.is_none() {
-                        discover_handle = Some(tokio::spawn(discover_loop(conf.clone(), global_timelines.clone(), stats.clone())));
+                        discover_handle = Some(tokio::spawn(discover_loop(conf.clone(), stats.clone())));
                     }
                 },
                 _ = &mut stats_task => {}

safekeeper/src/copy_timeline.rs

@@ -1,7 +1,9 @@
+use std::sync::Arc;
+
 use anyhow::{bail, Result};
 use camino::Utf8PathBuf;
+
 use postgres_ffi::{MAX_SEND_SIZE, WAL_SEGMENT_SIZE};
-use std::sync::Arc;
 use tokio::{
     fs::OpenOptions,
     io::{AsyncSeekExt, AsyncWriteExt},
@@ -12,7 +14,7 @@ use utils::{id::TenantTimelineId, lsn::Lsn};
 use crate::{
     control_file::FileStorage,
     state::TimelinePersistentState,
-    timeline::{TimelineError, WalResidentTimeline},
+    timeline::{Timeline, TimelineError, WalResidentTimeline},
     timelines_global_map::{create_temp_timeline_dir, validate_temp_timeline},
     wal_backup::copy_s3_segments,
     wal_storage::{wal_file_paths, WalReader},
@@ -23,19 +25,16 @@ use crate::{
 const MAX_BACKUP_LAG: u64 = 10 * WAL_SEGMENT_SIZE as u64;
 
 pub struct Request {
-    pub source_ttid: TenantTimelineId,
+    pub source: Arc<Timeline>,
     pub until_lsn: Lsn,
     pub destination_ttid: TenantTimelineId,
 }
 
-pub async fn handle_request(
-    request: Request,
-    global_timelines: Arc<GlobalTimelines>,
-) -> Result<()> {
+pub async fn handle_request(request: Request) -> Result<()> {
     // TODO: request.until_lsn MUST be a valid LSN, and we cannot check it :(
     //   if LSN will point to the middle of a WAL record, timeline will be in "broken" state
 
-    match global_timelines.get(request.destination_ttid) {
+    match GlobalTimelines::get(request.destination_ttid) {
         // timeline already exists. would be good to check that this timeline is the copy
         // of the source timeline, but it isn't obvious how to do that
         Ok(_) => return Ok(()),
@@ -47,10 +46,9 @@ pub async fn handle_request(
         }
     }
 
-    let source = global_timelines.get(request.source_ttid)?;
-    let source_tli = source.wal_residence_guard().await?;
+    let source_tli = request.source.wal_residence_guard().await?;
 
-    let conf = &global_timelines.get_global_config();
+    let conf = &GlobalTimelines::get_global_config();
     let ttid = request.destination_ttid;
 
     let (_tmp_dir, tli_dir_path) = create_temp_timeline_dir(conf, ttid).await?;
@@ -129,7 +127,7 @@ pub async fn handle_request(
 
     copy_s3_segments(
         wal_seg_size,
-        &request.source_ttid,
+        &request.source.ttid,
         &request.destination_ttid,
         first_segment,
         first_ondisk_segment,
@@ -160,9 +158,7 @@ pub async fn handle_request(
 
     // now we have a ready timeline in a temp directory
     validate_temp_timeline(conf, request.destination_ttid, &tli_dir_path).await?;
-    global_timelines
-        .load_temp_timeline(request.destination_ttid, &tli_dir_path, true)
-        .await?;
+    GlobalTimelines::load_temp_timeline(request.destination_ttid, &tli_dir_path, true).await?;
 
     Ok(())
 }

safekeeper/src/debug_dump.rs

@@ -207,23 +207,23 @@ pub struct FileInfo {
 }
 
 /// Build debug dump response, using the provided [`Args`] filters.
-pub async fn build(args: Args, global_timelines: Arc<GlobalTimelines>) -> Result<Response> {
+pub async fn build(args: Args) -> Result<Response> {
     let start_time = Utc::now();
-    let timelines_count = global_timelines.timelines_count();
-    let config = global_timelines.get_global_config();
+    let timelines_count = GlobalTimelines::timelines_count();
+    let config = GlobalTimelines::get_global_config();
 
     let ptrs_snapshot = if args.tenant_id.is_some() && args.timeline_id.is_some() {
         // If both tenant_id and timeline_id are specified, we can just get the
         // timeline directly, without taking a snapshot of the whole list.
         let ttid = TenantTimelineId::new(args.tenant_id.unwrap(), args.timeline_id.unwrap());
-        if let Ok(tli) = global_timelines.get(ttid) {
+        if let Ok(tli) = GlobalTimelines::get(ttid) {
             vec![tli]
         } else {
             vec![]
         }
     } else {
         // Otherwise, take a snapshot of the whole list.
-        global_timelines.get_all()
+        GlobalTimelines::get_all()
     };
 
     let mut timelines = Vec::new();
@@ -344,12 +344,12 @@ fn get_wal_last_modified(path: &Utf8Path) -> Result<Option<DateTime<Utc>>> {
 
 /// Converts SafeKeeperConf to Config, filtering out the fields that are not
 /// supposed to be exposed.
-fn build_config(config: Arc<SafeKeeperConf>) -> Config {
+fn build_config(config: SafeKeeperConf) -> Config {
     Config {
         id: config.my_id,
-        workdir: config.workdir.clone().into(),
-        listen_pg_addr: config.listen_pg_addr.clone(),
-        listen_http_addr: config.listen_http_addr.clone(),
+        workdir: config.workdir.into(),
+        listen_pg_addr: config.listen_pg_addr,
+        listen_http_addr: config.listen_http_addr,
         no_sync: config.no_sync,
         max_offloader_lag_bytes: config.max_offloader_lag_bytes,
         wal_backup_enabled: config.wal_backup_enabled,

safekeeper/src/handler.rs

@@ -33,7 +33,7 @@ use utils::{
 
 /// Safekeeper handler of postgres commands
 pub struct SafekeeperPostgresHandler {
-    pub conf: Arc<SafeKeeperConf>,
+    pub conf: SafeKeeperConf,
     /// assigned application name
     pub appname: Option<String>,
     pub tenant_id: Option<TenantId>,
@@ -43,7 +43,6 @@ pub struct SafekeeperPostgresHandler {
     pub protocol: Option<PostgresClientProtocol>,
     /// Unique connection id is logged in spans for observability.
     pub conn_id: ConnectionId,
-    pub global_timelines: Arc<GlobalTimelines>,
     /// Auth scope allowed on the connections and public key used to check auth tokens. None if auth is not configured.
     auth: Option<(Scope, Arc<JwtAuth>)>,
     claims: Option<Claims>,
@@ -315,11 +314,10 @@ impl<IO: AsyncRead + AsyncWrite + Unpin + Send> postgres_backend::Handler<IO>
 
 impl SafekeeperPostgresHandler {
     pub fn new(
-        conf: Arc<SafeKeeperConf>,
+        conf: SafeKeeperConf,
         conn_id: u32,
         io_metrics: Option<TrafficMetrics>,
         auth: Option<(Scope, Arc<JwtAuth>)>,
-        global_timelines: Arc<GlobalTimelines>,
     ) -> Self {
         SafekeeperPostgresHandler {
             conf,
@@ -333,7 +331,6 @@ impl SafekeeperPostgresHandler {
             claims: None,
             auth,
             io_metrics,
-            global_timelines,
         }
     }
 
@@ -363,7 +360,7 @@ impl SafekeeperPostgresHandler {
         pgb: &mut PostgresBackend<IO>,
     ) -> Result<(), QueryError> {
         // Get timeline, handling "not found" error
-        let tli = match self.global_timelines.get(self.ttid) {
+        let tli = match GlobalTimelines::get(self.ttid) {
             Ok(tli) => Ok(Some(tli)),
             Err(TimelineError::NotFound(_)) => Ok(None),
             Err(e) => Err(QueryError::Other(e.into())),
@@ -397,10 +394,7 @@ impl SafekeeperPostgresHandler {
         &mut self,
         pgb: &mut PostgresBackend<IO>,
     ) -> Result<(), QueryError> {
-        let tli = self
-            .global_timelines
-            .get(self.ttid)
-            .map_err(|e| QueryError::Other(e.into()))?;
+        let tli = GlobalTimelines::get(self.ttid).map_err(|e| QueryError::Other(e.into()))?;
 
         let lsn = if self.is_walproposer_recovery() {
             // walproposer should get all local WAL until flush_lsn

safekeeper/src/http/mod.rs

@@ -3,16 +3,14 @@ pub mod routes;
 pub use routes::make_router;
 
 pub use safekeeper_api::models;
-use std::sync::Arc;
 
-use crate::{GlobalTimelines, SafeKeeperConf};
+use crate::SafeKeeperConf;
 
 pub async fn task_main(
-    conf: Arc<SafeKeeperConf>,
+    conf: SafeKeeperConf,
     http_listener: std::net::TcpListener,
-    global_timelines: Arc<GlobalTimelines>,
 ) -> anyhow::Result<()> {
-    let router = make_router(conf, global_timelines)
+    let router = make_router(conf)
         .build()
         .map_err(|err| anyhow::anyhow!(err))?;
     let service = utils::http::RouterService::new(router).unwrap();

safekeeper/src/http/routes.rs

@@ -66,13 +66,6 @@ fn get_conf(request: &Request<Body>) -> &SafeKeeperConf {
         .as_ref()
 }
 
-fn get_global_timelines(request: &Request<Body>) -> Arc<GlobalTimelines> {
-    request
-        .data::<Arc<GlobalTimelines>>()
-        .expect("unknown state type")
-        .clone()
-}
-
 /// Same as TermLsn, but serializes LSN using display serializer
 /// in Postgres format, i.e. 0/FFFFFFFF. Used only for the API response.
 #[derive(Debug, Clone, Copy, Serialize, Deserialize)]
@@ -130,11 +123,9 @@ async fn tenant_delete_handler(mut request: Request<Body>) -> Result<Response<Bo
     let only_local = parse_query_param(&request, "only_local")?.unwrap_or(false);
     check_permission(&request, Some(tenant_id))?;
     ensure_no_body(&mut request).await?;
-    let global_timelines = get_global_timelines(&request);
     // FIXME: `delete_force_all_for_tenant` can return an error for multiple different reasons;
     // Using an `InternalServerError` should be fixed when the types support it
-    let delete_info = global_timelines
-        .delete_force_all_for_tenant(&tenant_id, only_local)
+    let delete_info = GlobalTimelines::delete_force_all_for_tenant(&tenant_id, only_local)
         .await
         .map_err(ApiError::InternalServerError)?;
     json_response(
@@ -165,9 +156,7 @@ async fn timeline_create_handler(mut request: Request<Body>) -> Result<Response<
             .commit_lsn
             .segment_lsn(server_info.wal_seg_size as usize)
     });
-    let global_timelines = get_global_timelines(&request);
-    global_timelines
-        .create(ttid, server_info, request_data.commit_lsn, local_start_lsn)
+    GlobalTimelines::create(ttid, server_info, request_data.commit_lsn, local_start_lsn)
         .await
         .map_err(ApiError::InternalServerError)?;
 
@@ -178,9 +167,7 @@ async fn timeline_create_handler(mut request: Request<Body>) -> Result<Response<
 /// Note: it is possible to do the same with debug_dump.
 async fn timeline_list_handler(request: Request<Body>) -> Result<Response<Body>, ApiError> {
     check_permission(&request, None)?;
-    let global_timelines = get_global_timelines(&request);
-    let res: Vec<TenantTimelineId> = global_timelines
-        .get_all()
+    let res: Vec<TenantTimelineId> = GlobalTimelines::get_all()
         .iter()
         .map(|tli| tli.ttid)
         .collect();
@@ -195,8 +182,7 @@ async fn timeline_status_handler(request: Request<Body>) -> Result<Response<Body
     );
     check_permission(&request, Some(ttid.tenant_id))?;
 
-    let global_timelines = get_global_timelines(&request);
-    let tli = global_timelines.get(ttid).map_err(ApiError::from)?;
+    let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
     let (inmem, state) = tli.get_state().await;
     let flush_lsn = tli.get_flush_lsn().await;
 
@@ -247,11 +233,9 @@ async fn timeline_delete_handler(mut request: Request<Body>) -> Result<Response<
     let only_local = parse_query_param(&request, "only_local")?.unwrap_or(false);
     check_permission(&request, Some(ttid.tenant_id))?;
     ensure_no_body(&mut request).await?;
-    let global_timelines = get_global_timelines(&request);
     // FIXME: `delete_force` can fail from both internal errors and bad requests. Add better
     // error handling here when we're able to.
-    let resp = global_timelines
-        .delete(&ttid, only_local)
+    let resp = GlobalTimelines::delete(&ttid, only_local)
         .await
         .map_err(ApiError::InternalServerError)?;
     json_response(StatusCode::OK, resp)
@@ -263,9 +247,8 @@ async fn timeline_pull_handler(mut request: Request<Body>) -> Result<Response<Bo
 
     let data: pull_timeline::Request = json_request(&mut request).await?;
     let conf = get_conf(&request);
-    let global_timelines = get_global_timelines(&request);
 
-    let resp = pull_timeline::handle_request(data, conf.sk_auth_token.clone(), global_timelines)
+    let resp = pull_timeline::handle_request(data, conf.sk_auth_token.clone())
         .await
         .map_err(ApiError::InternalServerError)?;
     json_response(StatusCode::OK, resp)
@@ -280,8 +263,7 @@ async fn timeline_snapshot_handler(request: Request<Body>) -> Result<Response<Bo
     );
     check_permission(&request, Some(ttid.tenant_id))?;
 
-    let global_timelines = get_global_timelines(&request);
-    let tli = global_timelines.get(ttid).map_err(ApiError::from)?;
+    let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
 
     // To stream the body use wrap_stream which wants Stream of Result<Bytes>,
     // so create the chan and write to it in another task.
@@ -311,19 +293,19 @@ async fn timeline_copy_handler(mut request: Request<Body>) -> Result<Response<Bo
     check_permission(&request, None)?;
 
     let request_data: TimelineCopyRequest = json_request(&mut request).await?;
-    let source_ttid = TenantTimelineId::new(
+    let ttid = TenantTimelineId::new(
         parse_request_param(&request, "tenant_id")?,
         parse_request_param(&request, "source_timeline_id")?,
     );
 
-    let global_timelines = get_global_timelines(&request);
+    let source = GlobalTimelines::get(ttid)?;
 
     copy_timeline::handle_request(copy_timeline::Request{
-        source_ttid,
+        source,
         until_lsn: request_data.until_lsn,
-        destination_ttid: TenantTimelineId::new(source_ttid.tenant_id, request_data.target_timeline_id),
-    }, global_timelines)
-        .instrument(info_span!("copy_timeline", from=%source_ttid, to=%request_data.target_timeline_id, until_lsn=%request_data.until_lsn))
+        destination_ttid: TenantTimelineId::new(ttid.tenant_id, request_data.target_timeline_id),
+    })
+        .instrument(info_span!("copy_timeline", from=%ttid, to=%request_data.target_timeline_id, until_lsn=%request_data.until_lsn))
         .await
         .map_err(ApiError::InternalServerError)?;
 
@@ -340,8 +322,7 @@ async fn patch_control_file_handler(
         parse_request_param(&request, "timeline_id")?,
     );
 
-    let global_timelines = get_global_timelines(&request);
-    let tli = global_timelines.get(ttid).map_err(ApiError::from)?;
+    let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
 
     let patch_request: patch_control_file::Request = json_request(&mut request).await?;
     let response = patch_control_file::handle_request(tli, patch_request)
@@ -360,8 +341,7 @@ async fn timeline_checkpoint_handler(request: Request<Body>) -> Result<Response<
         parse_request_param(&request, "timeline_id")?,
     );
 
-    let global_timelines = get_global_timelines(&request);
-    let tli = global_timelines.get(ttid)?;
+    let tli = GlobalTimelines::get(ttid)?;
     tli.write_shared_state()
         .await
         .sk
@@ -379,7 +359,6 @@ async fn timeline_digest_handler(request: Request<Body>) -> Result<Response<Body
     );
     check_permission(&request, Some(ttid.tenant_id))?;
 
-    let global_timelines = get_global_timelines(&request);
     let from_lsn: Option<Lsn> = parse_query_param(&request, "from_lsn")?;
     let until_lsn: Option<Lsn> = parse_query_param(&request, "until_lsn")?;
 
@@ -392,7 +371,7 @@ async fn timeline_digest_handler(request: Request<Body>) -> Result<Response<Body
         )))?,
     };
 
-    let tli = global_timelines.get(ttid).map_err(ApiError::from)?;
+    let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
     let tli = tli
         .wal_residence_guard()
         .await
@@ -414,8 +393,7 @@ async fn timeline_backup_partial_reset(request: Request<Body>) -> Result<Respons
     );
     check_permission(&request, Some(ttid.tenant_id))?;
 
-    let global_timelines = get_global_timelines(&request);
-    let tli = global_timelines.get(ttid).map_err(ApiError::from)?;
+    let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
 
     let response = tli
         .backup_partial_reset()
@@ -437,8 +415,7 @@ async fn timeline_term_bump_handler(
 
     let request_data: TimelineTermBumpRequest = json_request(&mut request).await?;
 
-    let global_timelines = get_global_timelines(&request);
-    let tli = global_timelines.get(ttid).map_err(ApiError::from)?;
+    let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
     let response = tli
         .term_bump(request_data.term)
         .await
@@ -475,8 +452,7 @@ async fn record_safekeeper_info(mut request: Request<Body>) -> Result<Response<B
         standby_horizon: sk_info.standby_horizon.0,
     };
 
-    let global_timelines = get_global_timelines(&request);
-    let tli = global_timelines.get(ttid).map_err(ApiError::from)?;
+    let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
     tli.record_safekeeper_info(proto_sk_info)
         .await
         .map_err(ApiError::InternalServerError)?;
@@ -530,8 +506,6 @@ async fn dump_debug_handler(mut request: Request<Body>) -> Result<Response<Body>
     let dump_term_history = dump_term_history.unwrap_or(true);
     let dump_wal_last_modified = dump_wal_last_modified.unwrap_or(dump_all);
 
-    let global_timelines = get_global_timelines(&request);
-
     let args = debug_dump::Args {
         dump_all,
         dump_control_file,
@@ -543,7 +517,7 @@ async fn dump_debug_handler(mut request: Request<Body>) -> Result<Response<Body>
         timeline_id,
     };
 
-    let resp = debug_dump::build(args, global_timelines)
+    let resp = debug_dump::build(args)
         .await
         .map_err(ApiError::InternalServerError)?;
 
@@ -596,10 +570,7 @@ async fn dump_debug_handler(mut request: Request<Body>) -> Result<Response<Body>
 }
 
 /// Safekeeper http router.
-pub fn make_router(
-    conf: Arc<SafeKeeperConf>,
-    global_timelines: Arc<GlobalTimelines>,
-) -> RouterBuilder<hyper::Body, ApiError> {
+pub fn make_router(conf: SafeKeeperConf) -> RouterBuilder<hyper::Body, ApiError> {
     let mut router = endpoint::make_router();
     if conf.http_auth.is_some() {
         router = router.middleware(auth_middleware(|request| {
@@ -621,8 +592,7 @@ pub fn make_router(
     // located nearby (/safekeeper/src/http/openapi_spec.yaml).
     let auth = conf.http_auth.clone();
     router
-        .data(conf)
-        .data(global_timelines)
+        .data(Arc::new(conf))
         .data(auth)
         .get("/metrics", |r| request_span(r, prometheus_metrics_handler))
         .get("/profile/cpu", |r| request_span(r, profile_cpu_handler))

safekeeper/src/json_ctrl.rs

@@ -11,6 +11,7 @@ use postgres_backend::QueryError;
 use serde::{Deserialize, Serialize};
 use tokio::io::{AsyncRead, AsyncWrite};
 use tracing::*;
+use utils::id::TenantTimelineId;
 
 use crate::handler::SafekeeperPostgresHandler;
 use crate::safekeeper::{AcceptorProposerMessage, AppendResponse, ServerInfo};
@@ -20,6 +21,7 @@ use crate::safekeeper::{
 use crate::safekeeper::{Term, TermHistory, TermLsn};
 use crate::state::TimelinePersistentState;
 use crate::timeline::WalResidentTimeline;
+use crate::GlobalTimelines;
 use postgres_backend::PostgresBackend;
 use postgres_ffi::encode_logical_message;
 use postgres_ffi::WAL_SEGMENT_SIZE;
@@ -68,7 +70,7 @@ pub async fn handle_json_ctrl<IO: AsyncRead + AsyncWrite + Unpin>(
     info!("JSON_CTRL request: {append_request:?}");
 
     // need to init safekeeper state before AppendRequest
-    let tli = prepare_safekeeper(spg, append_request.pg_version).await?;
+    let tli = prepare_safekeeper(spg.ttid, append_request.pg_version).await?;
 
     // if send_proposer_elected is true, we need to update local history
     if append_request.send_proposer_elected {
@@ -97,22 +99,20 @@ pub async fn handle_json_ctrl<IO: AsyncRead + AsyncWrite + Unpin>(
 /// Prepare safekeeper to process append requests without crashes,
 /// by sending ProposerGreeting with default server.wal_seg_size.
 async fn prepare_safekeeper(
-    spg: &SafekeeperPostgresHandler,
+    ttid: TenantTimelineId,
     pg_version: u32,
 ) -> anyhow::Result<WalResidentTimeline> {
-    let tli = spg
-        .global_timelines
-        .create(
-            spg.ttid,
-            ServerInfo {
-                pg_version,
-                wal_seg_size: WAL_SEGMENT_SIZE as u32,
-                system_id: 0,
-            },
-            Lsn::INVALID,
-            Lsn::INVALID,
-        )
-        .await?;
+    let tli = GlobalTimelines::create(
+        ttid,
+        ServerInfo {
+            pg_version,
+            wal_seg_size: WAL_SEGMENT_SIZE as u32,
+            system_id: 0,
+        },
+        Lsn::INVALID,
+        Lsn::INVALID,
+    )
+    .await?;
 
     tli.wal_residence_guard().await
 }

safekeeper/src/metrics.rs

@@ -455,7 +455,6 @@ pub struct FullTimelineInfo {
 
 /// Collects metrics for all active timelines.
 pub struct TimelineCollector {
-    global_timelines: Arc<GlobalTimelines>,
     descs: Vec<Desc>,
     commit_lsn: GenericGaugeVec<AtomicU64>,
     backup_lsn: GenericGaugeVec<AtomicU64>,
@@ -479,8 +478,14 @@ pub struct TimelineCollector {
     active_timelines_count: IntGauge,
 }
 
+impl Default for TimelineCollector {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl TimelineCollector {
-    pub fn new(global_timelines: Arc<GlobalTimelines>) -> TimelineCollector {
+    pub fn new() -> TimelineCollector {
         let mut descs = Vec::new();
 
         let commit_lsn = GenericGaugeVec::new(
@@ -671,7 +676,6 @@ impl TimelineCollector {
         descs.extend(active_timelines_count.desc().into_iter().cloned());
 
         TimelineCollector {
-            global_timelines,
             descs,
             commit_lsn,
             backup_lsn,
@@ -724,18 +728,17 @@ impl Collector for TimelineCollector {
         self.written_wal_seconds.reset();
         self.flushed_wal_seconds.reset();
 
-        let timelines_count = self.global_timelines.get_all().len();
+        let timelines_count = GlobalTimelines::get_all().len();
         let mut active_timelines_count = 0;
 
         // Prometheus Collector is sync, and data is stored under async lock. To
         // bridge the gap with a crutch, collect data in spawned thread with
         // local tokio runtime.
-        let global_timelines = self.global_timelines.clone();
         let infos = std::thread::spawn(|| {
             let rt = tokio::runtime::Builder::new_current_thread()
                 .build()
                 .expect("failed to create rt");
-            rt.block_on(collect_timeline_metrics(global_timelines))
+            rt.block_on(collect_timeline_metrics())
         })
         .join()
         .expect("collect_timeline_metrics thread panicked");
@@ -854,9 +857,9 @@ impl Collector for TimelineCollector {
     }
 }
 
-async fn collect_timeline_metrics(global_timelines: Arc<GlobalTimelines>) -> Vec<FullTimelineInfo> {
+async fn collect_timeline_metrics() -> Vec<FullTimelineInfo> {
     let mut res = vec![];
-    let active_timelines = global_timelines.get_global_broker_active_set().get_all();
+    let active_timelines = GlobalTimelines::get_global_broker_active_set().get_all();
 
     for tli in active_timelines {
         if let Some(info) = tli.info_for_metrics().await {

safekeeper/src/pull_timeline.rs

@@ -409,9 +409,8 @@ pub struct DebugDumpResponse {
 pub async fn handle_request(
     request: Request,
     sk_auth_token: Option<SecretString>,
-    global_timelines: Arc<GlobalTimelines>,
 ) -> Result<Response> {
-    let existing_tli = global_timelines.get(TenantTimelineId::new(
+    let existing_tli = GlobalTimelines::get(TenantTimelineId::new(
         request.tenant_id,
         request.timeline_id,
     ));
@@ -454,14 +453,13 @@ pub async fn handle_request(
     assert!(status.tenant_id == request.tenant_id);
     assert!(status.timeline_id == request.timeline_id);
 
-    pull_timeline(status, safekeeper_host, sk_auth_token, global_timelines).await
+    pull_timeline(status, safekeeper_host, sk_auth_token).await
 }
 
 async fn pull_timeline(
     status: TimelineStatus,
     host: String,
     sk_auth_token: Option<SecretString>,
-    global_timelines: Arc<GlobalTimelines>,
 ) -> Result<Response> {
     let ttid = TenantTimelineId::new(status.tenant_id, status.timeline_id);
     info!(
@@ -474,7 +472,7 @@ async fn pull_timeline(
         status.acceptor_state.epoch
     );
 
-    let conf = &global_timelines.get_global_config();
+    let conf = &GlobalTimelines::get_global_config();
 
     let (_tmp_dir, tli_dir_path) = create_temp_timeline_dir(conf, ttid).await?;
 
@@ -533,9 +531,7 @@ async fn pull_timeline(
     assert!(status.commit_lsn <= status.flush_lsn);
 
     // Finally, load the timeline.
-    let _tli = global_timelines
-        .load_temp_timeline(ttid, &tli_dir_path, false)
-        .await?;
+    let _tli = GlobalTimelines::load_temp_timeline(ttid, &tli_dir_path, false).await?;
 
     Ok(Response {
         safekeeper_host: host,

safekeeper/src/receive_wal.rs

@@ -267,7 +267,6 @@ impl SafekeeperPostgresHandler {
             pgb_reader: &mut pgb_reader,
             peer_addr,
             acceptor_handle: &mut acceptor_handle,
-            global_timelines: self.global_timelines.clone(),
         };
 
         // Read first message and create timeline if needed.
@@ -332,7 +331,6 @@ struct NetworkReader<'a, IO> {
     // WalAcceptor is spawned when we learn server info from walproposer and
     // create timeline; handle is put here.
     acceptor_handle: &'a mut Option<JoinHandle<anyhow::Result<()>>>,
-    global_timelines: Arc<GlobalTimelines>,
 }
 
 impl<'a, IO: AsyncRead + AsyncWrite + Unpin> NetworkReader<'a, IO> {
@@ -352,11 +350,10 @@ impl<'a, IO: AsyncRead + AsyncWrite + Unpin> NetworkReader<'a, IO> {
                     system_id: greeting.system_id,
                     wal_seg_size: greeting.wal_seg_size,
                 };
-                let tli = self
-                    .global_timelines
-                    .create(self.ttid, server_info, Lsn::INVALID, Lsn::INVALID)
-                    .await
-                    .context("create timeline")?;
+                let tli =
+                    GlobalTimelines::create(self.ttid, server_info, Lsn::INVALID, Lsn::INVALID)
+                        .await
+                        .context("create timeline")?;
                 tli.wal_residence_guard().await?
             }
             _ => {

safekeeper/src/send_wal.rs

@@ -10,6 +10,7 @@ use crate::timeline::WalResidentTimeline;
 use crate::wal_reader_stream::WalReaderStreamBuilder;
 use crate::wal_service::ConnectionId;
 use crate::wal_storage::WalReader;
+use crate::GlobalTimelines;
 use anyhow::{bail, Context as AnyhowContext};
 use bytes::Bytes;
 use futures::future::Either;
@@ -399,10 +400,7 @@ impl SafekeeperPostgresHandler {
         start_pos: Lsn,
         term: Option<Term>,
     ) -> Result<(), QueryError> {
-        let tli = self
-            .global_timelines
-            .get(self.ttid)
-            .map_err(|e| QueryError::Other(e.into()))?;
+        let tli = GlobalTimelines::get(self.ttid).map_err(|e| QueryError::Other(e.into()))?;
         let residence_guard = tli.wal_residence_guard().await?;
 
         if let Err(end) = self

safekeeper/src/timeline.rs

@@ -44,8 +44,8 @@ use crate::wal_backup_partial::PartialRemoteSegment;
 
 use crate::metrics::{FullTimelineInfo, WalStorageMetrics, MISC_OPERATION_SECONDS};
 use crate::wal_storage::{Storage as wal_storage_iface, WalReader};
-use crate::SafeKeeperConf;
 use crate::{debug_dump, timeline_manager, wal_storage};
+use crate::{GlobalTimelines, SafeKeeperConf};
 
 /// Things safekeeper should know about timeline state on peers.
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -467,7 +467,6 @@ pub struct Timeline {
     walreceivers: Arc<WalReceivers>,
     timeline_dir: Utf8PathBuf,
     manager_ctl: ManagerCtl,
-    conf: Arc<SafeKeeperConf>,
 
     /// Hold this gate from code that depends on the Timeline's non-shut-down state.  While holding
     /// this gate, you must respect [`Timeline::cancel`]
@@ -490,7 +489,6 @@ impl Timeline {
         timeline_dir: &Utf8Path,
         remote_path: &RemotePath,
         shared_state: SharedState,
-        conf: Arc<SafeKeeperConf>,
     ) -> Arc<Self> {
         let (commit_lsn_watch_tx, commit_lsn_watch_rx) =
             watch::channel(shared_state.sk.state().commit_lsn);
@@ -518,7 +516,6 @@ impl Timeline {
             gate: Default::default(),
             cancel: CancellationToken::default(),
             manager_ctl: ManagerCtl::new(),
-            conf,
             broker_active: AtomicBool::new(false),
             wal_backup_active: AtomicBool::new(false),
             last_removed_segno: AtomicU64::new(0),
@@ -527,14 +524,11 @@ impl Timeline {
     }
 
     /// Load existing timeline from disk.
-    pub fn load_timeline(
-        conf: Arc<SafeKeeperConf>,
-        ttid: TenantTimelineId,
-    ) -> Result<Arc<Timeline>> {
+    pub fn load_timeline(conf: &SafeKeeperConf, ttid: TenantTimelineId) -> Result<Arc<Timeline>> {
         let _enter = info_span!("load_timeline", timeline = %ttid.timeline_id).entered();
 
-        let shared_state = SharedState::restore(conf.as_ref(), &ttid)?;
-        let timeline_dir = get_timeline_dir(conf.as_ref(), &ttid);
+        let shared_state = SharedState::restore(conf, &ttid)?;
+        let timeline_dir = get_timeline_dir(conf, &ttid);
         let remote_path = remote_timeline_path(&ttid)?;
 
         Ok(Timeline::new(
@@ -542,7 +536,6 @@ impl Timeline {
             &timeline_dir,
             &remote_path,
             shared_state,
-            conf,
         ))
     }
 
@@ -611,7 +604,8 @@ impl Timeline {
         // it is cancelled, so WAL storage won't be opened again.
         shared_state.sk.close_wal_store();
 
-        if !only_local && self.conf.is_wal_backup_enabled() {
+        let conf = GlobalTimelines::get_global_config();
+        if !only_local && conf.is_wal_backup_enabled() {
             // Note: we concurrently delete remote storage data from multiple
             // safekeepers. That's ok, s3 replies 200 if object doesn't exist and we
             // do some retries anyway.
@@ -957,7 +951,7 @@ impl WalResidentTimeline {
 
     pub async fn get_walreader(&self, start_lsn: Lsn) -> Result<WalReader> {
         let (_, persisted_state) = self.get_state().await;
-        let enable_remote_read = self.conf.is_wal_backup_enabled();
+        let enable_remote_read = GlobalTimelines::get_global_config().is_wal_backup_enabled();
 
         WalReader::new(
             &self.ttid,
@@ -1067,6 +1061,7 @@ impl ManagerTimeline {
 
     /// Try to switch state Offloaded->Present.
     pub(crate) async fn switch_to_present(&self) -> anyhow::Result<()> {
+        let conf = GlobalTimelines::get_global_config();
         let mut shared = self.write_shared_state().await;
 
         // trying to restore WAL storage
@@ -1074,7 +1069,7 @@ impl ManagerTimeline {
             &self.ttid,
             &self.timeline_dir,
             shared.sk.state(),
-            self.conf.no_sync,
+            conf.no_sync,
         )?;
 
         // updating control file
@@ -1101,7 +1096,7 @@ impl ManagerTimeline {
         // now we can switch shared.sk to Present, shouldn't fail
         let prev_sk = std::mem::replace(&mut shared.sk, StateSK::Empty);
         let cfile_state = prev_sk.take_state();
-        shared.sk = StateSK::Loaded(SafeKeeper::new(cfile_state, wal_store, self.conf.my_id)?);
+        shared.sk = StateSK::Loaded(SafeKeeper::new(cfile_state, wal_store, conf.my_id)?);
 
         Ok(())
     }

safekeeper/src/timelines_global_map.rs

@@ -13,6 +13,7 @@ use crate::{control_file, wal_storage, SafeKeeperConf};
 use anyhow::{bail, Context, Result};
 use camino::Utf8PathBuf;
 use camino_tempfile::Utf8TempDir;
+use once_cell::sync::Lazy;
 use serde::Serialize;
 use std::collections::HashMap;
 use std::str::FromStr;
@@ -41,16 +42,23 @@ struct GlobalTimelinesState {
     // this map is dropped on restart.
     tombstones: HashMap<TenantTimelineId, Instant>,
 
-    conf: Arc<SafeKeeperConf>,
+    conf: Option<SafeKeeperConf>,
     broker_active_set: Arc<TimelinesSet>,
     global_rate_limiter: RateLimiter,
 }
 
 impl GlobalTimelinesState {
+    /// Get configuration, which must be set once during init.
+    fn get_conf(&self) -> &SafeKeeperConf {
+        self.conf
+            .as_ref()
+            .expect("GlobalTimelinesState conf is not initialized")
+    }
+
     /// Get dependencies for a timeline constructor.
-    fn get_dependencies(&self) -> (Arc<SafeKeeperConf>, Arc<TimelinesSet>, RateLimiter) {
+    fn get_dependencies(&self) -> (SafeKeeperConf, Arc<TimelinesSet>, RateLimiter) {
         (
-            self.conf.clone(),
+            self.get_conf().clone(),
             self.broker_active_set.clone(),
             self.global_rate_limiter.clone(),
         )
@@ -74,39 +82,35 @@ impl GlobalTimelinesState {
     }
 }
 
-/// A struct used to manage access to the global timelines map.
-pub struct GlobalTimelines {
-    state: Mutex<GlobalTimelinesState>,
-}
+static TIMELINES_STATE: Lazy<Mutex<GlobalTimelinesState>> = Lazy::new(|| {
+    Mutex::new(GlobalTimelinesState {
+        timelines: HashMap::new(),
+        tombstones: HashMap::new(),
+        conf: None,
+        broker_active_set: Arc::new(TimelinesSet::default()),
+        global_rate_limiter: RateLimiter::new(1, 1),
+    })
+});
+
+/// A zero-sized struct used to manage access to the global timelines map.
+pub struct GlobalTimelines;
 
 impl GlobalTimelines {
-    /// Create a new instance of the global timelines map.
-    pub fn new(conf: Arc<SafeKeeperConf>) -> Self {
-        Self {
-            state: Mutex::new(GlobalTimelinesState {
-                timelines: HashMap::new(),
-                tombstones: HashMap::new(),
-                conf,
-                broker_active_set: Arc::new(TimelinesSet::default()),
-                global_rate_limiter: RateLimiter::new(1, 1),
-            }),
-        }
-    }
-
     /// Inject dependencies needed for the timeline constructors and load all timelines to memory.
-    pub async fn init(&self) -> Result<()> {
+    pub async fn init(conf: SafeKeeperConf) -> Result<()> {
         // clippy isn't smart enough to understand that drop(state) releases the
         // lock, so use explicit block
         let tenants_dir = {
-            let mut state = self.state.lock().unwrap();
+            let mut state = TIMELINES_STATE.lock().unwrap();
             state.global_rate_limiter = RateLimiter::new(
-                state.conf.partial_backup_concurrency,
+                conf.partial_backup_concurrency,
                 DEFAULT_EVICTION_CONCURRENCY,
             );
+            state.conf = Some(conf);
 
             // Iterate through all directories and load tenants for all directories
             // named as a valid tenant_id.
-            state.conf.workdir.clone()
+            state.get_conf().workdir.clone()
         };
         let mut tenant_count = 0;
         for tenants_dir_entry in std::fs::read_dir(&tenants_dir)
@@ -118,7 +122,7 @@ impl GlobalTimelines {
                         TenantId::from_str(tenants_dir_entry.file_name().to_str().unwrap_or(""))
                     {
                         tenant_count += 1;
-                        self.load_tenant_timelines(tenant_id).await?;
+                        GlobalTimelines::load_tenant_timelines(tenant_id).await?;
                     }
                 }
                 Err(e) => error!(
@@ -131,7 +135,7 @@ impl GlobalTimelines {
         info!(
             "found {} tenants directories, successfully loaded {} timelines",
             tenant_count,
-            self.state.lock().unwrap().timelines.len()
+            TIMELINES_STATE.lock().unwrap().timelines.len()
         );
         Ok(())
     }
@@ -139,13 +143,13 @@ impl GlobalTimelines {
     /// Loads all timelines for the given tenant to memory. Returns fs::read_dir
     /// errors if any.
     ///
-    /// It is async, but self.state lock is sync and there is no important
+    /// It is async, but TIMELINES_STATE lock is sync and there is no important
     /// reason to make it async (it is always held for a short while), so we
     /// just lock and unlock it for each timeline -- this function is called
     /// during init when nothing else is running, so this is fine.
-    async fn load_tenant_timelines(&self, tenant_id: TenantId) -> Result<()> {
+    async fn load_tenant_timelines(tenant_id: TenantId) -> Result<()> {
         let (conf, broker_active_set, partial_backup_rate_limiter) = {
-            let state = self.state.lock().unwrap();
+            let state = TIMELINES_STATE.lock().unwrap();
             state.get_dependencies()
         };
 
@@ -159,10 +163,10 @@ impl GlobalTimelines {
                         TimelineId::from_str(timeline_dir_entry.file_name().to_str().unwrap_or(""))
                     {
                         let ttid = TenantTimelineId::new(tenant_id, timeline_id);
-                        match Timeline::load_timeline(conf.clone(), ttid) {
+                        match Timeline::load_timeline(&conf, ttid) {
                             Ok(tli) => {
                                 let mut shared_state = tli.write_shared_state().await;
-                                self.state
+                                TIMELINES_STATE
                                     .lock()
                                     .unwrap()
                                     .timelines
@@ -196,30 +200,29 @@ impl GlobalTimelines {
     }
 
     /// Get the number of timelines in the map.
-    pub fn timelines_count(&self) -> usize {
-        self.state.lock().unwrap().timelines.len()
+    pub fn timelines_count() -> usize {
+        TIMELINES_STATE.lock().unwrap().timelines.len()
     }
 
     /// Get the global safekeeper config.
-    pub fn get_global_config(&self) -> Arc<SafeKeeperConf> {
-        self.state.lock().unwrap().conf.clone()
+    pub fn get_global_config() -> SafeKeeperConf {
+        TIMELINES_STATE.lock().unwrap().get_conf().clone()
     }
 
-    pub fn get_global_broker_active_set(&self) -> Arc<TimelinesSet> {
-        self.state.lock().unwrap().broker_active_set.clone()
+    pub fn get_global_broker_active_set() -> Arc<TimelinesSet> {
+        TIMELINES_STATE.lock().unwrap().broker_active_set.clone()
     }
 
     /// Create a new timeline with the given id. If the timeline already exists, returns
     /// an existing timeline.
     pub(crate) async fn create(
-        &self,
         ttid: TenantTimelineId,
         server_info: ServerInfo,
         commit_lsn: Lsn,
         local_start_lsn: Lsn,
     ) -> Result<Arc<Timeline>> {
         let (conf, _, _) = {
-            let state = self.state.lock().unwrap();
+            let state = TIMELINES_STATE.lock().unwrap();
             if let Ok(timeline) = state.get(&ttid) {
                 // Timeline already exists, return it.
                 return Ok(timeline);
@@ -242,7 +245,7 @@ impl GlobalTimelines {
         let state =
             TimelinePersistentState::new(&ttid, server_info, vec![], commit_lsn, local_start_lsn)?;
         control_file::FileStorage::create_new(&tmp_dir_path, state, conf.no_sync).await?;
-        let timeline = self.load_temp_timeline(ttid, &tmp_dir_path, true).await?;
+        let timeline = GlobalTimelines::load_temp_timeline(ttid, &tmp_dir_path, true).await?;
         Ok(timeline)
     }
 
@@ -258,14 +261,13 @@ impl GlobalTimelines {
     /// 2) move the directory and load the timeline
     /// 3) take lock again and insert the timeline into the global map.
     pub async fn load_temp_timeline(
-        &self,
         ttid: TenantTimelineId,
         tmp_path: &Utf8PathBuf,
         check_tombstone: bool,
     ) -> Result<Arc<Timeline>> {
         // Check for existence and mark that we're creating it.
         let (conf, broker_active_set, partial_backup_rate_limiter) = {
-            let mut state = self.state.lock().unwrap();
+            let mut state = TIMELINES_STATE.lock().unwrap();
             match state.timelines.get(&ttid) {
                 Some(GlobalMapTimeline::CreationInProgress) => {
                     bail!(TimelineError::CreationInProgress(ttid));
@@ -293,10 +295,10 @@ impl GlobalTimelines {
         };
 
         // Do the actual move and reflect the result in the map.
-        match GlobalTimelines::install_temp_timeline(ttid, tmp_path, conf.clone()).await {
+        match GlobalTimelines::install_temp_timeline(ttid, tmp_path, &conf).await {
             Ok(timeline) => {
                 let mut timeline_shared_state = timeline.write_shared_state().await;
-                let mut state = self.state.lock().unwrap();
+                let mut state = TIMELINES_STATE.lock().unwrap();
                 assert!(matches!(
                     state.timelines.get(&ttid),
                     Some(GlobalMapTimeline::CreationInProgress)
@@ -317,7 +319,7 @@ impl GlobalTimelines {
             }
             Err(e) => {
                 // Init failed, remove the marker from the map
-                let mut state = self.state.lock().unwrap();
+                let mut state = TIMELINES_STATE.lock().unwrap();
                 assert!(matches!(
                     state.timelines.get(&ttid),
                     Some(GlobalMapTimeline::CreationInProgress)
@@ -332,10 +334,10 @@ impl GlobalTimelines {
     async fn install_temp_timeline(
         ttid: TenantTimelineId,
         tmp_path: &Utf8PathBuf,
-        conf: Arc<SafeKeeperConf>,
+        conf: &SafeKeeperConf,
     ) -> Result<Arc<Timeline>> {
-        let tenant_path = get_tenant_dir(conf.as_ref(), &ttid.tenant_id);
-        let timeline_path = get_timeline_dir(conf.as_ref(), &ttid);
+        let tenant_path = get_tenant_dir(conf, &ttid.tenant_id);
+        let timeline_path = get_timeline_dir(conf, &ttid);
 
         // We must have already checked that timeline doesn't exist in the map,
         // but there might be existing datadir: if timeline is corrupted it is
@@ -380,9 +382,9 @@ impl GlobalTimelines {
     /// Get a timeline from the global map. If it's not present, it doesn't exist on disk,
     /// or was corrupted and couldn't be loaded on startup. Returned timeline is always valid,
     /// i.e. loaded in memory and not cancelled.
-    pub(crate) fn get(&self, ttid: TenantTimelineId) -> Result<Arc<Timeline>, TimelineError> {
+    pub(crate) fn get(ttid: TenantTimelineId) -> Result<Arc<Timeline>, TimelineError> {
         let tli_res = {
-            let state = self.state.lock().unwrap();
+            let state = TIMELINES_STATE.lock().unwrap();
             state.get(&ttid)
         };
         match tli_res {
@@ -397,8 +399,8 @@ impl GlobalTimelines {
     }
 
     /// Returns all timelines. This is used for background timeline processes.
-    pub fn get_all(&self) -> Vec<Arc<Timeline>> {
-        let global_lock = self.state.lock().unwrap();
+    pub fn get_all() -> Vec<Arc<Timeline>> {
+        let global_lock = TIMELINES_STATE.lock().unwrap();
         global_lock
             .timelines
             .values()
@@ -417,8 +419,8 @@ impl GlobalTimelines {
 
     /// Returns all timelines belonging to a given tenant. Used for deleting all timelines of a tenant,
     /// and that's why it can return cancelled timelines, to retry deleting them.
-    fn get_all_for_tenant(&self, tenant_id: TenantId) -> Vec<Arc<Timeline>> {
-        let global_lock = self.state.lock().unwrap();
+    fn get_all_for_tenant(tenant_id: TenantId) -> Vec<Arc<Timeline>> {
+        let global_lock = TIMELINES_STATE.lock().unwrap();
         global_lock
             .timelines
             .values()
@@ -433,12 +435,11 @@ impl GlobalTimelines {
     /// Cancels timeline, then deletes the corresponding data directory.
     /// If only_local, doesn't remove WAL segments in remote storage.
     pub(crate) async fn delete(
-        &self,
         ttid: &TenantTimelineId,
         only_local: bool,
     ) -> Result<TimelineDeleteForceResult> {
         let tli_res = {
-            let state = self.state.lock().unwrap();
+            let state = TIMELINES_STATE.lock().unwrap();
 
             if state.tombstones.contains_key(ttid) {
                 // Presence of a tombstone guarantees that a previous deletion has completed and there is no work to do.
@@ -471,7 +472,7 @@ impl GlobalTimelines {
             }
             Err(_) => {
                 // Timeline is not memory, but it may still exist on disk in broken state.
-                let dir_path = get_timeline_dir(self.state.lock().unwrap().conf.as_ref(), ttid);
+                let dir_path = get_timeline_dir(TIMELINES_STATE.lock().unwrap().get_conf(), ttid);
                 let dir_existed = delete_dir(dir_path)?;
 
                 Ok(TimelineDeleteForceResult {
@@ -484,7 +485,7 @@ impl GlobalTimelines {
         // Finalize deletion, by dropping Timeline objects and storing smaller tombstones.  The tombstones
         // are used to prevent still-running computes from re-creating the same timeline when they send data,
         // and to speed up repeated deletion calls by avoiding re-listing objects.
-        self.state.lock().unwrap().delete(*ttid);
+        TIMELINES_STATE.lock().unwrap().delete(*ttid);
 
         result
     }
@@ -496,18 +497,17 @@ impl GlobalTimelines {
     ///
     /// If only_local, doesn't remove WAL segments in remote storage.
     pub async fn delete_force_all_for_tenant(
-        &self,
         tenant_id: &TenantId,
         only_local: bool,
     ) -> Result<HashMap<TenantTimelineId, TimelineDeleteForceResult>> {
         info!("deleting all timelines for tenant {}", tenant_id);
-        let to_delete = self.get_all_for_tenant(*tenant_id);
+        let to_delete = Self::get_all_for_tenant(*tenant_id);
 
         let mut err = None;
 
         let mut deleted = HashMap::new();
         for tli in &to_delete {
-            match self.delete(&tli.ttid, only_local).await {
+            match Self::delete(&tli.ttid, only_local).await {
                 Ok(result) => {
                     deleted.insert(tli.ttid, result);
                 }
@@ -529,15 +529,15 @@ impl GlobalTimelines {
         // so the directory may be not empty. In this case timelines will have bad state
         // and timeline background jobs can panic.
         delete_dir(get_tenant_dir(
-            self.state.lock().unwrap().conf.as_ref(),
+            TIMELINES_STATE.lock().unwrap().get_conf(),
             tenant_id,
         ))?;
 
         Ok(deleted)
     }
 
-    pub fn housekeeping(&self, tombstone_ttl: &Duration) {
-        let mut state = self.state.lock().unwrap();
+    pub fn housekeeping(tombstone_ttl: &Duration) {
+        let mut state = TIMELINES_STATE.lock().unwrap();
 
         // We keep tombstones long enough to have a good chance of preventing rogue computes from re-creating deleted
         // timelines.  If a compute kept running for longer than this TTL (or across a safekeeper restart) then they

safekeeper/src/wal_service.rs

@@ -4,7 +4,6 @@
 //!
 use anyhow::{Context, Result};
 use postgres_backend::QueryError;
-use std::sync::Arc;
 use std::time::Duration;
 use tokio::net::TcpStream;
 use tokio_io_timeout::TimeoutReader;
@@ -12,9 +11,9 @@ use tokio_util::sync::CancellationToken;
 use tracing::*;
 use utils::{auth::Scope, measured_stream::MeasuredStream};
 
+use crate::handler::SafekeeperPostgresHandler;
 use crate::metrics::TrafficMetrics;
 use crate::SafeKeeperConf;
-use crate::{handler::SafekeeperPostgresHandler, GlobalTimelines};
 use postgres_backend::{AuthType, PostgresBackend};
 
 /// Accept incoming TCP connections and spawn them into a background thread.
@@ -23,10 +22,9 @@ use postgres_backend::{AuthType, PostgresBackend};
 /// to any tenant are allowed) or Tenant (only tokens giving access to specific
 /// tenant are allowed). Doesn't matter if auth is disabled in conf.
 pub async fn task_main(
-    conf: Arc<SafeKeeperConf>,
+    conf: SafeKeeperConf,
     pg_listener: std::net::TcpListener,
     allowed_auth_scope: Scope,
-    global_timelines: Arc<GlobalTimelines>,
 ) -> anyhow::Result<()> {
     // Tokio's from_std won't do this for us, per its comment.
     pg_listener.set_nonblocking(true)?;
@@ -39,10 +37,10 @@ pub async fn task_main(
         debug!("accepted connection from {}", peer_addr);
         let conf = conf.clone();
         let conn_id = issue_connection_id(&mut connection_count);
-        let global_timelines = global_timelines.clone();
+
         tokio::spawn(
             async move {
-                if let Err(err) = handle_socket(socket, conf, conn_id, allowed_auth_scope, global_timelines).await {
+                if let Err(err) = handle_socket(socket, conf, conn_id, allowed_auth_scope).await {
                     error!("connection handler exited: {}", err);
                 }
             }
@@ -55,10 +53,9 @@ pub async fn task_main(
 ///
 async fn handle_socket(
     socket: TcpStream,
-    conf: Arc<SafeKeeperConf>,
+    conf: SafeKeeperConf,
     conn_id: ConnectionId,
     allowed_auth_scope: Scope,
-    global_timelines: Arc<GlobalTimelines>,
 ) -> Result<(), QueryError> {
     socket.set_nodelay(true)?;
     let peer_addr = socket.peer_addr()?;
@@ -99,13 +96,8 @@ async fn handle_socket(
         Some(_) => AuthType::NeonJWT,
     };
     let auth_pair = auth_key.map(|key| (allowed_auth_scope, key));
-    let mut conn_handler = SafekeeperPostgresHandler::new(
-        conf,
-        conn_id,
-        Some(traffic_metrics.clone()),
-        auth_pair,
-        global_timelines,
-    );
+    let mut conn_handler =
+        SafekeeperPostgresHandler::new(conf, conn_id, Some(traffic_metrics.clone()), auth_pair);
     let pgbackend = PostgresBackend::new_from_io(socket, peer_addr, auth_type, None)?;
     // libpq protocol between safekeeper and walproposer / pageserver
     // We don't use shutdown.

storage_controller/src/background_node_operations.rs

@@ -3,7 +3,7 @@ use std::{borrow::Cow, fmt::Debug, fmt::Display};
 use tokio_util::sync::CancellationToken;
 use utils::id::NodeId;
 
-pub(crate) const MAX_RECONCILES_PER_OPERATION: usize = 64;
+pub(crate) const MAX_RECONCILES_PER_OPERATION: usize = 32;
 
 #[derive(Copy, Clone)]
 pub(crate) struct Drain {

storage_controller/src/http.rs

@@ -18,9 +18,8 @@ use pageserver_api::controller_api::{
     ShardsPreferredAzsRequest, TenantCreateRequest,
 };
 use pageserver_api::models::{
-    TenantConfigPatchRequest, TenantConfigRequest, TenantLocationConfigRequest,
-    TenantShardSplitRequest, TenantTimeTravelRequest, TimelineArchivalConfigRequest,
-    TimelineCreateRequest,
+    TenantConfigRequest, TenantLocationConfigRequest, TenantShardSplitRequest,
+    TenantTimeTravelRequest, TimelineArchivalConfigRequest, TimelineCreateRequest,
 };
 use pageserver_api::shard::TenantShardId;
 use pageserver_client::{mgmt_api, BlockUnblock};
@@ -209,27 +208,6 @@ async fn handle_tenant_location_config(
     )
 }
 
-async fn handle_tenant_config_patch(
-    service: Arc<Service>,
-    req: Request<Body>,
-) -> Result<Response<Body>, ApiError> {
-    check_permissions(&req, Scope::PageServerApi)?;
-
-    let mut req = match maybe_forward(req).await {
-        ForwardOutcome::Forwarded(res) => {
-            return res;
-        }
-        ForwardOutcome::NotForwarded(req) => req,
-    };
-
-    let config_req = json_request::<TenantConfigPatchRequest>(&mut req).await?;
-
-    json_response(
-        StatusCode::OK,
-        service.tenant_config_patch(config_req).await?,
-    )
-}
-
 async fn handle_tenant_config_set(
     service: Arc<Service>,
     req: Request<Body>,
@@ -879,21 +857,6 @@ async fn handle_cancel_node_fill(req: Request<Body>) -> Result<Response<Body>, A
     json_response(StatusCode::ACCEPTED, ())
 }
 
-async fn handle_safekeeper_list(req: Request<Body>) -> Result<Response<Body>, ApiError> {
-    check_permissions(&req, Scope::Infra)?;
-
-    let req = match maybe_forward(req).await {
-        ForwardOutcome::Forwarded(res) => {
-            return res;
-        }
-        ForwardOutcome::NotForwarded(req) => req,
-    };
-
-    let state = get_state(&req);
-    let safekeepers = state.service.safekeepers_list().await?;
-    json_response(StatusCode::OK, safekeepers)
-}
-
 async fn handle_metadata_health_update(req: Request<Body>) -> Result<Response<Body>, ApiError> {
     check_permissions(&req, Scope::Scrubber)?;
 
@@ -1218,7 +1181,7 @@ impl From<ReconcileError> for ApiError {
 ///
 /// Not used by anything except manual testing.
 async fn handle_get_safekeeper(req: Request<Body>) -> Result<Response<Body>, ApiError> {
-    check_permissions(&req, Scope::Infra)?;
+    check_permissions(&req, Scope::Admin)?;
 
     let id = parse_request_param::<i64>(&req, "id")?;
 
@@ -1236,7 +1199,7 @@ async fn handle_get_safekeeper(req: Request<Body>) -> Result<Response<Body>, Api
     match res {
         Ok(b) => json_response(StatusCode::OK, b),
         Err(crate::persistence::DatabaseError::Query(diesel::result::Error::NotFound)) => {
-            Err(ApiError::NotFound("unknown instance id".into()))
+            Err(ApiError::NotFound("unknown instance_id".into()))
         }
         Err(other) => Err(other.into()),
     }
@@ -1832,21 +1795,6 @@ pub fn make_router(
                 RequestName("control_v1_metadata_health_list_outdated"),
             )
         })
-        // Safekeepers
-        .get("/control/v1/safekeeper", |r| {
-            named_request_span(
-                r,
-                handle_safekeeper_list,
-                RequestName("control_v1_safekeeper_list"),
-            )
-        })
-        .get("/control/v1/safekeeper/:id", |r| {
-            named_request_span(r, handle_get_safekeeper, RequestName("v1_safekeeper"))
-        })
-        .post("/control/v1/safekeeper/:id", |r| {
-            // id is in the body
-            named_request_span(r, handle_upsert_safekeeper, RequestName("v1_safekeeper"))
-        })
         // Tenant Shard operations
         .put("/control/v1/tenant/:tenant_shard_id/migrate", |r| {
             tenant_service_handler(
@@ -1899,6 +1847,13 @@ pub fn make_router(
         .put("/control/v1/step_down", |r| {
             named_request_span(r, handle_step_down, RequestName("control_v1_step_down"))
         })
+        .get("/control/v1/safekeeper/:id", |r| {
+            named_request_span(r, handle_get_safekeeper, RequestName("v1_safekeeper"))
+        })
+        .post("/control/v1/safekeeper/:id", |r| {
+            // id is in the body
+            named_request_span(r, handle_upsert_safekeeper, RequestName("v1_safekeeper"))
+        })
         // Tenant operations
         // The ^/v1/ endpoints act as a "Virtual Pageserver", enabling shard-naive clients to call into
         // this service to manage tenants that actually consist of many tenant shards, as if they are a single entity.
@@ -1908,13 +1863,6 @@ pub fn make_router(
         .delete("/v1/tenant/:tenant_id", |r| {
             tenant_service_handler(r, handle_tenant_delete, RequestName("v1_tenant"))
         })
-        .patch("/v1/tenant/config", |r| {
-            tenant_service_handler(
-                r,
-                handle_tenant_config_patch,
-                RequestName("v1_tenant_config"),
-            )
-        })
         .put("/v1/tenant/config", |r| {
             tenant_service_handler(r, handle_tenant_config_set, RequestName("v1_tenant_config"))
         })

storage_controller/src/persistence.rs

@@ -104,7 +104,6 @@ pub(crate) enum DatabaseOperation {
     ListMetadataHealth,
     ListMetadataHealthUnhealthy,
     ListMetadataHealthOutdated,
-    ListSafekeepers,
     GetLeader,
     UpdateLeader,
     SetPreferredAzs,
@@ -637,13 +636,6 @@ impl Persistence {
                     .into_boxed(),
             };
 
-            // Clear generation_pageserver if we are moving into a state where we won't have
-            // any attached pageservers.
-            let input_generation_pageserver = match input_placement_policy {
-                None | Some(PlacementPolicy::Attached(_)) => None,
-                Some(PlacementPolicy::Detached | PlacementPolicy::Secondary) => Some(None),
-            };
-
             #[derive(AsChangeset)]
             #[diesel(table_name = crate::schema::tenant_shards)]
             struct ShardUpdate {
@@ -651,7 +643,6 @@ impl Persistence {
                 placement_policy: Option<String>,
                 config: Option<String>,
                 scheduling_policy: Option<String>,
-                generation_pageserver: Option<Option<i64>>,
             }
 
             let update = ShardUpdate {
@@ -664,7 +655,6 @@ impl Persistence {
                     .map(|c| serde_json::to_string(&c).unwrap()),
                 scheduling_policy: input_scheduling_policy
                     .map(|p| serde_json::to_string(&p).unwrap()),
-                generation_pageserver: input_generation_pageserver,
             };
 
             query.set(update).execute(conn)?;
@@ -1012,22 +1002,6 @@ impl Persistence {
         Ok(())
     }
 
-    /// At startup, populate the list of nodes which our shards may be placed on
-    pub(crate) async fn list_safekeepers(&self) -> DatabaseResult<Vec<SafekeeperPersistence>> {
-        let safekeepers: Vec<SafekeeperPersistence> = self
-            .with_measured_conn(
-                DatabaseOperation::ListNodes,
-                move |conn| -> DatabaseResult<_> {
-                    Ok(crate::schema::safekeepers::table.load::<SafekeeperPersistence>(conn)?)
-                },
-            )
-            .await?;
-
-        tracing::info!("list_safekeepers: loaded {} nodes", safekeepers.len());
-
-        Ok(safekeepers)
-    }
-
     pub(crate) async fn safekeeper_get(
         &self,
         id: i64,

storage_controller/src/schema.rs

@@ -29,19 +29,6 @@ diesel::table! {
     }
 }
 
-diesel::table! {
-    safekeepers (id) {
-        id -> Int8,
-        region_id -> Text,
-        version -> Int8,
-        host -> Text,
-        port -> Int4,
-        active -> Bool,
-        http_port -> Int4,
-        availability_zone_id -> Text,
-    }
-}
-
 diesel::table! {
     tenant_shards (tenant_id, shard_number, shard_count) {
         tenant_id -> Varchar,
@@ -58,10 +45,18 @@ diesel::table! {
     }
 }
 
-diesel::allow_tables_to_appear_in_same_query!(
-    controllers,
-    metadata_health,
-    nodes,
-    safekeepers,
-    tenant_shards,
-);
+diesel::allow_tables_to_appear_in_same_query!(controllers, metadata_health, nodes, tenant_shards,);
+
+diesel::table! {
+    safekeepers {
+        id -> Int8,
+        region_id -> Text,
+        version -> Int8,
+        instance_id -> Text,
+        host -> Text,
+        port -> Int4,
+        active -> Bool,
+        http_port -> Int4,
+        availability_zone_id -> Text,
+    }
+}

storage_controller/src/service.rs

@@ -52,8 +52,8 @@ use pageserver_api::{
         TenantPolicyRequest, TenantShardMigrateRequest, TenantShardMigrateResponse,
     },
     models::{
-        SecondaryProgress, TenantConfigPatchRequest, TenantConfigRequest,
-        TimelineArchivalConfigRequest, TopTenantShardsRequest,
+        SecondaryProgress, TenantConfigRequest, TimelineArchivalConfigRequest,
+        TopTenantShardsRequest,
     },
 };
 use reqwest::StatusCode;
@@ -100,8 +100,6 @@ use crate::{
 
 use context_iterator::TenantShardContextIterator;
 
-const WAITER_FILL_DRAIN_POLL_TIMEOUT: Duration = Duration::from_millis(500);
-
 // For operations that should be quick, like attaching a new tenant
 const SHORT_RECONCILE_TIMEOUT: Duration = Duration::from_secs(5);
 
@@ -141,7 +139,6 @@ enum TenantOperations {
     Create,
     LocationConfig,
     ConfigSet,
-    ConfigPatch,
     TimeTravelRemoteStorage,
     Delete,
     UpdatePolicy,
@@ -516,9 +513,6 @@ struct ShardUpdate {
 
     /// If this is None, generation is not updated.
     generation: Option<Generation>,
-
-    /// If this is None, scheduling policy is not updated.
-    scheduling_policy: Option<ShardSchedulingPolicy>,
 }
 
 enum StopReconciliationsReason {
@@ -2382,23 +2376,6 @@ impl Service {
             }
         };
 
-        // Ordinarily we do not update scheduling policy, but when making major changes
-        // like detaching or demoting to secondary-only, we need to force the scheduling
-        // mode to Active, or the caller's expected outcome (detach it) will not happen.
-        let scheduling_policy = match req.config.mode {
-            LocationConfigMode::Detached | LocationConfigMode::Secondary => {
-                // Special case: when making major changes like detaching or demoting to secondary-only,
-                // we need to force the scheduling mode to Active, or nothing will happen.
-                Some(ShardSchedulingPolicy::Active)
-            }
-            LocationConfigMode::AttachedMulti
-            | LocationConfigMode::AttachedSingle
-            | LocationConfigMode::AttachedStale => {
-                // While attached, continue to respect whatever the existing scheduling mode is.
-                None
-            }
-        };
-
         let mut create = true;
         for (shard_id, shard) in tenants.range_mut(TenantShardId::tenant_range(tenant_id)) {
             // Saw an existing shard: this is not a creation
@@ -2424,7 +2401,6 @@ impl Service {
                 placement_policy: placement_policy.clone(),
                 tenant_config: req.config.tenant_conf.clone(),
                 generation: set_generation,
-                scheduling_policy,
             });
         }
 
@@ -2521,7 +2497,6 @@ impl Service {
                     placement_policy,
                     tenant_config,
                     generation,
-                    scheduling_policy,
                 } in &updates
                 {
                     self.persistence
@@ -2530,7 +2505,7 @@ impl Service {
                             Some(placement_policy.clone()),
                             Some(tenant_config.clone()),
                             *generation,
-                            *scheduling_policy,
+                            None,
                         )
                         .await?;
                 }
@@ -2546,7 +2521,6 @@ impl Service {
                         placement_policy,
                         tenant_config,
                         generation: update_generation,
-                        scheduling_policy,
                     } in updates
                     {
                         let Some(shard) = tenants.get_mut(&tenant_shard_id) else {
@@ -2565,10 +2539,6 @@ impl Service {
                             shard.generation = Some(generation);
                         }
 
-                        if let Some(scheduling_policy) = scheduling_policy {
-                            shard.set_scheduling_policy(scheduling_policy);
-                        }
-
                         shard.schedule(scheduler, &mut schedule_context)?;
 
                         let maybe_waiter = self.maybe_reconcile_shard(shard, nodes);
@@ -2605,55 +2575,6 @@ impl Service {
         Ok(result)
     }
 
-    pub(crate) async fn tenant_config_patch(
-        &self,
-        req: TenantConfigPatchRequest,
-    ) -> Result<(), ApiError> {
-        let _tenant_lock = trace_exclusive_lock(
-            &self.tenant_op_locks,
-            req.tenant_id,
-            TenantOperations::ConfigPatch,
-        )
-        .await;
-
-        let tenant_id = req.tenant_id;
-        let patch = req.config;
-
-        let base = {
-            let locked = self.inner.read().unwrap();
-            let shards = locked
-                .tenants
-                .range(TenantShardId::tenant_range(req.tenant_id));
-
-            let mut configs = shards.map(|(_sid, shard)| &shard.config).peekable();
-
-            let first = match configs.peek() {
-                Some(first) => (*first).clone(),
-                None => {
-                    return Err(ApiError::NotFound(
-                        anyhow::anyhow!("Tenant {} not found", req.tenant_id).into(),
-                    ));
-                }
-            };
-
-            if !configs.all_equal() {
-                tracing::error!("Tenant configs for {} are mismatched. ", req.tenant_id);
-                // This can't happen because we atomically update the database records
-                // of all shards to the new value in [`Self::set_tenant_config_and_reconcile`].
-                return Err(ApiError::InternalServerError(anyhow::anyhow!(
-                    "Tenant configs for {} are mismatched",
-                    req.tenant_id
-                )));
-            }
-
-            first
-        };
-
-        let updated_config = base.apply_patch(patch);
-        self.set_tenant_config_and_reconcile(tenant_id, updated_config)
-            .await
-    }
-
     pub(crate) async fn tenant_config_set(&self, req: TenantConfigRequest) -> Result<(), ApiError> {
         // We require an exclusive lock, because we are updating persistent and in-memory state
         let _tenant_lock = trace_exclusive_lock(
@@ -2663,32 +2584,12 @@ impl Service {
         )
         .await;
 
-        let tenant_exists = {
-            let locked = self.inner.read().unwrap();
-            let mut r = locked
-                .tenants
-                .range(TenantShardId::tenant_range(req.tenant_id));
-            r.next().is_some()
-        };
+        let tenant_id = req.tenant_id;
+        let config = req.config;
 
-        if !tenant_exists {
-            return Err(ApiError::NotFound(
-                anyhow::anyhow!("Tenant {} not found", req.tenant_id).into(),
-            ));
-        }
-
-        self.set_tenant_config_and_reconcile(req.tenant_id, req.config)
-            .await
-    }
-
-    async fn set_tenant_config_and_reconcile(
-        &self,
-        tenant_id: TenantId,
-        config: TenantConfig,
-    ) -> Result<(), ApiError> {
         self.persistence
             .update_tenant_shard(
-                TenantFilter::Tenant(tenant_id),
+                TenantFilter::Tenant(req.tenant_id),
                 None,
                 Some(config.clone()),
                 None,
@@ -3091,17 +2992,9 @@ impl Service {
 
         let TenantPolicyRequest {
             placement,
-            mut scheduling,
+            scheduling,
         } = req;
 
-        if let Some(PlacementPolicy::Detached | PlacementPolicy::Secondary) = placement {
-            // When someone configures a tenant to detach, we force the scheduling policy to enable
-            // this to take effect.
-            if scheduling.is_none() {
-                scheduling = Some(ShardSchedulingPolicy::Active);
-            }
-        }
-
         self.persistence
             .update_tenant_shard(
                 TenantFilter::Tenant(tenant_id),
@@ -6800,7 +6693,7 @@ impl Service {
             }
 
             waiters = self
-                .await_waiters_remainder(waiters, WAITER_FILL_DRAIN_POLL_TIMEOUT)
+                .await_waiters_remainder(waiters, SHORT_RECONCILE_TIMEOUT)
                 .await;
 
             failpoint_support::sleep_millis_async!("sleepy-drain-loop", &cancel);
@@ -7053,7 +6946,7 @@ impl Service {
             }
 
             waiters = self
-                .await_waiters_remainder(waiters, WAITER_FILL_DRAIN_POLL_TIMEOUT)
+                .await_waiters_remainder(waiters, SHORT_RECONCILE_TIMEOUT)
                 .await;
         }
 
@@ -7185,12 +7078,6 @@ impl Service {
         global_observed
     }
 
-    pub(crate) async fn safekeepers_list(
-        &self,
-    ) -> Result<Vec<crate::persistence::SafekeeperPersistence>, DatabaseError> {
-        self.persistence.list_safekeepers().await
-    }
-
     pub(crate) async fn get_safekeeper(
         &self,
         id: i64,

storage_scrubber/src/checks.rs

@@ -533,9 +533,8 @@ async fn list_timeline_blobs_impl(
 }
 
 pub(crate) struct RemoteTenantManifestInfo {
-    pub(crate) generation: Generation,
-    pub(crate) manifest: TenantManifest,
-    pub(crate) listing_object: ListingObject,
+    pub(crate) latest_generation: Option<Generation>,
+    pub(crate) manifests: Vec<(Generation, ListingObject)>,
 }
 
 pub(crate) enum ListTenantManifestResult {
@@ -544,10 +543,7 @@ pub(crate) enum ListTenantManifestResult {
         #[allow(dead_code)]
         unknown_keys: Vec<ListingObject>,
     },
-    NoErrors {
-        latest_generation: Option<RemoteTenantManifestInfo>,
-        manifests: Vec<(Generation, ListingObject)>,
-    },
+    NoErrors(RemoteTenantManifestInfo),
 }
 
 /// Lists the tenant manifests in remote storage and parses the latest one, returning a [`ListTenantManifestResult`] object.
@@ -596,6 +592,14 @@ pub(crate) async fn list_tenant_manifests(
         unknown_keys.push(obj);
     }
 
+    if manifests.is_empty() {
+        tracing::debug!("No manifest for timeline.");
+
+        return Ok(ListTenantManifestResult::WithErrors {
+            errors,
+            unknown_keys,
+        });
+    }
     if !unknown_keys.is_empty() {
         errors.push(((*prefix_str).to_owned(), "unknown keys listed".to_string()));
 
@@ -605,15 +609,6 @@ pub(crate) async fn list_tenant_manifests(
         });
     }
 
-    if manifests.is_empty() {
-        tracing::debug!("No manifest for timeline.");
-
-        return Ok(ListTenantManifestResult::NoErrors {
-            latest_generation: None,
-            manifests,
-        });
-    }
-
     // Find the manifest with the highest generation
     let (latest_generation, latest_listing_object) = manifests
         .iter()
@@ -621,8 +616,6 @@ pub(crate) async fn list_tenant_manifests(
         .map(|(g, obj)| (*g, obj.clone()))
         .unwrap();
 
-    manifests.retain(|(gen, _obj)| gen != &latest_generation);
-
     let manifest_bytes =
         match download_object_with_retries(remote_client, &latest_listing_object.key).await {
             Ok(bytes) => bytes,
@@ -641,15 +634,13 @@ pub(crate) async fn list_tenant_manifests(
         };
 
     match TenantManifest::from_json_bytes(&manifest_bytes) {
-        Ok(manifest) => {
-            return Ok(ListTenantManifestResult::NoErrors {
-                latest_generation: Some(RemoteTenantManifestInfo {
-                    generation: latest_generation,
-                    manifest,
-                    listing_object: latest_listing_object,
-                }),
-                manifests,
-            });
+        Ok(_manifest) => {
+            return Ok(ListTenantManifestResult::NoErrors(
+                RemoteTenantManifestInfo {
+                    latest_generation: Some(latest_generation),
+                    manifests,
+                },
+            ));
         }
         Err(parse_error) => errors.push((
             latest_listing_object.key.get_path().as_str().to_owned(),

storage_scrubber/src/garbage.rs

@@ -459,10 +459,12 @@ pub async fn get_timeline_objects(
     Ok(list.keys)
 }
 
+const MAX_KEYS_PER_DELETE: usize = 1000;
+
 /// Drain a buffer of keys into DeleteObjects requests
 ///
 /// If `drain` is true, drains keys completely; otherwise stops when <
-/// `max_keys_per_delete`` keys are left.
+/// MAX_KEYS_PER_DELETE keys are left.
 /// `num_deleted` returns number of deleted keys.
 async fn do_delete(
     remote_client: &GenericRemoteStorage,
@@ -472,10 +474,9 @@ async fn do_delete(
     progress_tracker: &mut DeletionProgressTracker,
 ) -> anyhow::Result<()> {
     let cancel = CancellationToken::new();
-    let max_keys_per_delete = remote_client.max_keys_per_delete();
-    while (!keys.is_empty() && drain) || (keys.len() >= max_keys_per_delete) {
+    while (!keys.is_empty() && drain) || (keys.len() >= MAX_KEYS_PER_DELETE) {
         let request_keys =
-            keys.split_off(keys.len() - (std::cmp::min(max_keys_per_delete, keys.len())));
+            keys.split_off(keys.len() - (std::cmp::min(MAX_KEYS_PER_DELETE, keys.len())));
 
         let request_keys: Vec<RemotePath> = request_keys.into_iter().map(|o| o.key).collect();
 
@@ -616,7 +617,7 @@ pub async fn purge_garbage(
         }
 
         objects_to_delete.append(&mut object_list);
-        if objects_to_delete.len() >= remote_client.max_keys_per_delete() {
+        if objects_to_delete.len() >= MAX_KEYS_PER_DELETE {
             do_delete(
                 &remote_client,
                 &mut objects_to_delete,

storage_scrubber/src/main.rs

@@ -86,8 +86,6 @@ enum Command {
         /// For safekeeper node_kind only, json list of timelines and their lsn info
         #[arg(long, default_value = None)]
         timeline_lsns: Option<String>,
-        #[arg(long, default_value_t = false)]
-        verbose: bool,
     },
     TenantSnapshot {
         #[arg(long = "tenant-id")]
@@ -168,7 +166,6 @@ async fn main() -> anyhow::Result<()> {
             dump_db_connstr,
             dump_db_table,
             timeline_lsns,
-            verbose,
         } => {
             if let NodeKind::Safekeeper = node_kind {
                 let db_or_list = match (timeline_lsns, dump_db_connstr) {
@@ -206,7 +203,6 @@ async fn main() -> anyhow::Result<()> {
                     tenant_ids,
                     json,
                     post_to_storcon,
-                    verbose,
                     cli.exit_code,
                 )
                 .await
@@ -317,7 +313,6 @@ pub async fn run_cron_job(
         Vec::new(),
         true,
         post_to_storcon,
-        false, // default to non-verbose mode
         exit_code,
     )
     .await?;
@@ -367,13 +362,12 @@ pub async fn scan_pageserver_metadata_cmd(
     tenant_shard_ids: Vec<TenantShardId>,
     json: bool,
     post_to_storcon: bool,
-    verbose: bool,
     exit_code: bool,
 ) -> anyhow::Result<()> {
     if controller_client.is_none() && post_to_storcon {
         return Err(anyhow!("Posting pageserver scan health status to storage controller requires `--controller-api` and `--controller-jwt` to run"));
     }
-    match scan_pageserver_metadata(bucket_config.clone(), tenant_shard_ids, verbose).await {
+    match scan_pageserver_metadata(bucket_config.clone(), tenant_shard_ids).await {
         Err(e) => {
             tracing::error!("Failed: {e}");
             Err(e)

storage_scrubber/src/pageserver_physical_gc.rs

@@ -4,13 +4,11 @@ use std::time::Duration;
 
 use crate::checks::{
     list_tenant_manifests, list_timeline_blobs, BlobDataParseResult, ListTenantManifestResult,
-    RemoteTenantManifestInfo,
 };
 use crate::metadata_stream::{stream_tenant_timelines, stream_tenants};
 use crate::{init_remote, BucketConfig, NodeKind, RootTarget, TenantShardTimelineId, MAX_RETRIES};
 use futures_util::{StreamExt, TryStreamExt};
 use pageserver::tenant::remote_timeline_client::index::LayerFileMetadata;
-use pageserver::tenant::remote_timeline_client::manifest::OffloadedTimelineManifest;
 use pageserver::tenant::remote_timeline_client::{
     parse_remote_index_path, parse_remote_tenant_manifest_path, remote_layer_path,
 };
@@ -529,7 +527,7 @@ async fn gc_tenant_manifests(
     target: &RootTarget,
     mode: GcMode,
     tenant_shard_id: TenantShardId,
-) -> anyhow::Result<(GcSummary, Option<RemoteTenantManifestInfo>)> {
+) -> anyhow::Result<GcSummary> {
     let mut gc_summary = GcSummary::default();
     match list_tenant_manifests(remote_client, tenant_shard_id, target).await? {
         ListTenantManifestResult::WithErrors {
@@ -539,35 +537,33 @@ async fn gc_tenant_manifests(
             for (_key, error) in errors {
                 tracing::warn!(%tenant_shard_id, "list_tenant_manifests: {error}");
             }
-            Ok((gc_summary, None))
         }
-        ListTenantManifestResult::NoErrors {
-            latest_generation,
-            mut manifests,
-        } => {
-            let Some(latest_generation) = latest_generation else {
-                return Ok((gc_summary, None));
+        ListTenantManifestResult::NoErrors(mut manifest_info) => {
+            let Some(latest_gen) = manifest_info.latest_generation else {
+                return Ok(gc_summary);
             };
-            manifests.sort_by_key(|(generation, _obj)| *generation);
+            manifest_info
+                .manifests
+                .sort_by_key(|(generation, _obj)| *generation);
             // skip the two latest generations (they don't neccessarily have to be 1 apart from each other)
-            let candidates = manifests.iter().rev().skip(2);
+            let candidates = manifest_info.manifests.iter().rev().skip(2);
             for (_generation, key) in candidates {
                 maybe_delete_tenant_manifest(
                     remote_client,
                     &min_age,
-                    latest_generation.generation,
+                    latest_gen,
                     key,
                     mode,
                     &mut gc_summary,
                 )
                 .instrument(
-                    info_span!("maybe_delete_tenant_manifest", %tenant_shard_id, ?latest_generation.generation, %key.key),
+                    info_span!("maybe_delete_tenant_manifest", %tenant_shard_id, ?latest_gen, %key.key),
                 )
                 .await;
             }
-            Ok((gc_summary, Some(latest_generation)))
         }
     }
+    Ok(gc_summary)
 }
 
 async fn gc_timeline(
@@ -577,7 +573,6 @@ async fn gc_timeline(
     mode: GcMode,
     ttid: TenantShardTimelineId,
     accumulator: &Arc<std::sync::Mutex<TenantRefAccumulator>>,
-    tenant_manifest_info: Arc<Option<RemoteTenantManifestInfo>>,
 ) -> anyhow::Result<GcSummary> {
     let mut summary = GcSummary::default();
     let data = list_timeline_blobs(remote_client, ttid, target).await?;
@@ -602,60 +597,6 @@ async fn gc_timeline(
         }
     };
 
-    if let Some(tenant_manifest_info) = &*tenant_manifest_info {
-        // TODO: this is O(n^2) in the number of offloaded timelines. Do a hashmap lookup instead.
-        let maybe_offloaded = tenant_manifest_info
-            .manifest
-            .offloaded_timelines
-            .iter()
-            .find(|offloaded_timeline| offloaded_timeline.timeline_id == ttid.timeline_id);
-        if let Some(offloaded) = maybe_offloaded {
-            let warnings = validate_index_part_with_offloaded(index_part, offloaded);
-            let warn = if warnings.is_empty() {
-                false
-            } else {
-                // Verify that the manifest hasn't changed. If it has, a potential racing change could have been cause for our troubles.
-                match list_tenant_manifests(remote_client, ttid.tenant_shard_id, target).await? {
-                    ListTenantManifestResult::WithErrors {
-                        errors,
-                        unknown_keys: _,
-                    } => {
-                        for (_key, error) in errors {
-                            tracing::warn!(%ttid, "list_tenant_manifests in gc_timeline: {error}");
-                        }
-                        true
-                    }
-                    ListTenantManifestResult::NoErrors {
-                        latest_generation,
-                        manifests: _,
-                    } => {
-                        if let Some(new_latest_gen) = latest_generation {
-                            let manifest_changed = (
-                                new_latest_gen.generation,
-                                new_latest_gen.listing_object.last_modified,
-                            ) == (
-                                tenant_manifest_info.generation,
-                                tenant_manifest_info.listing_object.last_modified,
-                            );
-                            if manifest_changed {
-                                tracing::debug!(%ttid, "tenant manifest changed since it was loaded, suppressing {} warnings", warnings.len());
-                            }
-                            manifest_changed
-                        } else {
-                            // The latest generation is gone. This timeline is in the progress of being deleted?
-                            false
-                        }
-                    }
-                }
-            };
-            if warn {
-                for warning in warnings {
-                    tracing::warn!(%ttid, "{}", warning);
-                }
-            }
-        }
-    }
-
     accumulator.lock().unwrap().update(ttid, index_part);
 
     for key in candidates {
@@ -667,35 +608,6 @@ async fn gc_timeline(
     Ok(summary)
 }
 
-fn validate_index_part_with_offloaded(
-    index_part: &IndexPart,
-    offloaded: &OffloadedTimelineManifest,
-) -> Vec<String> {
-    let mut warnings = Vec::new();
-    if let Some(archived_at_index_part) = index_part.archived_at {
-        if archived_at_index_part
-            .signed_duration_since(offloaded.archived_at)
-            .num_seconds()
-            != 0
-        {
-            warnings.push(format!(
-                "index-part archived_at={} differs from manifest archived_at={}",
-                archived_at_index_part, offloaded.archived_at
-            ));
-        }
-    } else {
-        warnings.push("Timeline offloaded in manifest but not archived in index-part".to_string());
-    }
-    if index_part.metadata.ancestor_timeline() != offloaded.ancestor_timeline_id {
-        warnings.push(format!(
-            "index-part anestor={:?} differs from manifest ancestor={:?}",
-            index_part.metadata.ancestor_timeline(),
-            offloaded.ancestor_timeline_id
-        ));
-    }
-    warnings
-}
-
 /// Physical garbage collection: removing unused S3 objects.
 ///
 /// This is distinct from the garbage collection done inside the pageserver, which operates at a higher level
@@ -738,38 +650,29 @@ pub async fn pageserver_physical_gc(
         let target_ref = &target;
         let remote_client_ref = &remote_client;
         async move {
-            let gc_manifest_result = gc_tenant_manifests(
+            let summaries_from_manifests = match gc_tenant_manifests(
                 remote_client_ref,
                 min_age,
                 target_ref,
                 mode,
                 tenant_shard_id,
             )
-            .await;
-            let (summary_from_manifest, tenant_manifest_opt) = match gc_manifest_result {
-                Ok((gc_summary, tenant_manifest)) => (gc_summary, tenant_manifest),
+            .await
+            {
+                Ok(gc_summary) => vec![Ok(GcSummaryOrContent::<TenantShardTimelineId>::GcSummary(
+                    gc_summary,
+                ))],
                 Err(e) => {
                     tracing::warn!(%tenant_shard_id, "Error in gc_tenant_manifests: {e}");
-                    (GcSummary::default(), None)
+                    Vec::new()
                 }
             };
-            let tenant_manifest_arc = Arc::new(tenant_manifest_opt);
-            let summary_from_manifest = Ok(GcSummaryOrContent::<(_, _)>::GcSummary(
-                summary_from_manifest,
-            ));
             stream_tenant_timelines(remote_client_ref, target_ref, tenant_shard_id)
                 .await
                 .map(|stream| {
                     stream
-                        .zip(futures::stream::iter(std::iter::repeat(
-                            tenant_manifest_arc,
-                        )))
-                        .map(|(ttid_res, tenant_manifest_arc)| {
-                            ttid_res.map(move |ttid| {
-                                GcSummaryOrContent::Content((ttid, tenant_manifest_arc))
-                            })
-                        })
-                        .chain(futures::stream::iter([summary_from_manifest].into_iter()))
+                        .map_ok(GcSummaryOrContent::Content)
+                        .chain(futures::stream::iter(summaries_from_manifests.into_iter()))
                 })
         }
     });
@@ -781,17 +684,14 @@ pub async fn pageserver_physical_gc(
     // Drain futures for per-shard GC, populating accumulator as a side effect
     {
         let timelines = timelines.map_ok(|summary_or_ttid| match summary_or_ttid {
-            GcSummaryOrContent::Content((ttid, tenant_manifest_arc)) => {
-                futures::future::Either::Left(gc_timeline(
-                    &remote_client,
-                    &min_age,
-                    &target,
-                    mode,
-                    ttid,
-                    &accumulator,
-                    tenant_manifest_arc,
-                ))
-            }
+            GcSummaryOrContent::Content(ttid) => futures::future::Either::Left(gc_timeline(
+                &remote_client,
+                &min_age,
+                &target,
+                mode,
+                ttid,
+                &accumulator,
+            )),
             GcSummaryOrContent::GcSummary(gc_summary) => {
                 futures::future::Either::Right(futures::future::ok(gc_summary))
             }

storage_scrubber/src/scan_pageserver_metadata.rs

@@ -21,12 +21,8 @@ pub struct MetadataSummary {
     tenant_count: usize,
     timeline_count: usize,
     timeline_shard_count: usize,
-    /// Tenant-shard timeline (key) mapping to errors. The key has to be a string because it will be serialized to a JSON.
-    /// The key is generated using `TenantShardTimelineId::to_string()`.
-    with_errors: HashMap<String, Vec<String>>,
-    /// Tenant-shard timeline (key) mapping to warnings. The key has to be a string because it will be serialized to a JSON.
-    /// The key is generated using `TenantShardTimelineId::to_string()`.
-    with_warnings: HashMap<String, Vec<String>>,
+    with_errors: HashSet<TenantShardTimelineId>,
+    with_warnings: HashSet<TenantShardTimelineId>,
     with_orphans: HashSet<TenantShardTimelineId>,
     indices_by_version: HashMap<usize, usize>,
 
@@ -56,12 +52,7 @@ impl MetadataSummary {
         }
     }
 
-    fn update_analysis(
-        &mut self,
-        id: &TenantShardTimelineId,
-        analysis: &TimelineAnalysis,
-        verbose: bool,
-    ) {
+    fn update_analysis(&mut self, id: &TenantShardTimelineId, analysis: &TimelineAnalysis) {
         if analysis.is_healthy() {
             self.healthy_tenant_shards.insert(id.tenant_shard_id);
         } else {
@@ -70,17 +61,11 @@ impl MetadataSummary {
         }
 
         if !analysis.errors.is_empty() {
-            let entry = self.with_errors.entry(id.to_string()).or_default();
-            if verbose {
-                entry.extend(analysis.errors.iter().cloned());
-            }
+            self.with_errors.insert(*id);
         }
 
         if !analysis.warnings.is_empty() {
-            let entry = self.with_warnings.entry(id.to_string()).or_default();
-            if verbose {
-                entry.extend(analysis.warnings.iter().cloned());
-            }
+            self.with_warnings.insert(*id);
         }
     }
 
@@ -135,7 +120,6 @@ Index versions: {version_summary}
 pub async fn scan_pageserver_metadata(
     bucket_config: BucketConfig,
     tenant_ids: Vec<TenantShardId>,
-    verbose: bool,
 ) -> anyhow::Result<MetadataSummary> {
     let (remote_client, target) = init_remote(bucket_config, NodeKind::Pageserver).await?;
 
@@ -180,7 +164,6 @@ pub async fn scan_pageserver_metadata(
         mut tenant_objects: TenantObjectListing,
         timelines: Vec<(TenantShardTimelineId, RemoteTimelineBlobData)>,
         highest_shard_count: ShardCount,
-        verbose: bool,
     ) {
         summary.tenant_count += 1;
 
@@ -220,7 +203,7 @@ pub async fn scan_pageserver_metadata(
                         Some(data),
                     )
                     .await;
-                    summary.update_analysis(&ttid, &analysis, verbose);
+                    summary.update_analysis(&ttid, &analysis);
 
                     timeline_ids.insert(ttid.timeline_id);
                 } else {
@@ -288,6 +271,10 @@ pub async fn scan_pageserver_metadata(
         summary.update_data(&data);
 
         match tenant_id {
+            None => {
+                tenant_id = Some(ttid.tenant_shard_id.tenant_id);
+                highest_shard_count = highest_shard_count.max(ttid.tenant_shard_id.shard_count);
+            }
             Some(prev_tenant_id) => {
                 if prev_tenant_id != ttid.tenant_shard_id.tenant_id {
                     // New tenant: analyze this tenant's timelines, clear accumulated tenant_timeline_results
@@ -300,7 +287,6 @@ pub async fn scan_pageserver_metadata(
                         tenant_objects,
                         timelines,
                         highest_shard_count,
-                        verbose,
                     )
                     .instrument(info_span!("analyze-tenant", tenant = %prev_tenant_id))
                     .await;
@@ -310,10 +296,6 @@ pub async fn scan_pageserver_metadata(
                     highest_shard_count = highest_shard_count.max(ttid.tenant_shard_id.shard_count);
                 }
             }
-            None => {
-                tenant_id = Some(ttid.tenant_shard_id.tenant_id);
-                highest_shard_count = highest_shard_count.max(ttid.tenant_shard_id.shard_count);
-            }
         }
 
         match &data.blob_data {
@@ -344,7 +326,6 @@ pub async fn scan_pageserver_metadata(
             tenant_objects,
             tenant_timeline_results,
             highest_shard_count,
-            verbose,
         )
         .instrument(info_span!("analyze-tenant", tenant = %tenant_id))
         .await;

test_runner/cloud_regress/README.md

@@ -1,21 +0,0 @@
-# How to run the `pg_regress` tests on a cloud Neon instance.
-
-* Create a Neon project on staging.
-* Grant the superuser privileges to the DB user.
-* (Optional) create a branch for testing
-* Configure the endpoint by updating the control-plane database with the following settings:
-  * `Timeone`: `America/Los_Angeles`
-  * `DateStyle`: `Postgres,MDY`
-  * `compute_query_id`: `off`
-* Checkout the actual `Neon` sources
-* Patch the sql and expected files for the specific PostgreSQL version, e.g. for v17:
-```bash
-$ cd vendor/postgres-v17
-$ patch -p1 <../../compute/patches/cloud_regress_pg17.patch
-```
-* Set the environment variable `BENCHMARK_CONNSTR` to the connection URI of your project.
-* Set the environment variable `PG_VERSION` to the version of your project.
-* Run 
-```bash
-$ pytest -m remote_cluster -k cloud_regress
-```

test_runner/cloud_regress/test_cloud_regress.py

@@ -5,15 +5,68 @@ Run the regression tests on the cloud instance of Neon
 from __future__ import annotations
 
 from pathlib import Path
+from typing import Any
 
+import psycopg2
 import pytest
+from fixtures.log_helper import log
 from fixtures.neon_fixtures import RemotePostgres
 from fixtures.pg_version import PgVersion
 
 
+@pytest.fixture
+def setup(remote_pg: RemotePostgres):
+    """
+    Setup and teardown of the tests
+    """
+    with psycopg2.connect(remote_pg.connstr()) as conn:
+        with conn.cursor() as cur:
+            log.info("Creating the extension")
+            cur.execute("CREATE EXTENSION IF NOT EXISTS regress_so")
+            conn.commit()
+            # TODO: Migrate to branches and remove this code
+            log.info("Looking for subscriptions in the regress database")
+            cur.execute(
+                "SELECT subname FROM pg_catalog.pg_subscription WHERE "
+                "subdbid = (SELECT oid FROM pg_catalog.pg_database WHERE datname='regression');"
+            )
+            if cur.rowcount > 0:
+                with psycopg2.connect(
+                    dbname="regression",
+                    host=remote_pg.default_options["host"],
+                    user=remote_pg.default_options["user"],
+                    password=remote_pg.default_options["password"],
+                ) as regress_conn:
+                    with regress_conn.cursor() as regress_cur:
+                        for sub in cur:
+                            regress_cur.execute(f"ALTER SUBSCRIPTION {sub[0]} DISABLE")
+                            regress_cur.execute(
+                                f"ALTER SUBSCRIPTION {sub[0]} SET (slot_name = NONE)"
+                            )
+                            regress_cur.execute(f"DROP SUBSCRIPTION {sub[0]}")
+                        regress_conn.commit()
+
+    yield
+    # TODO: Migrate to branches and remove this code
+    log.info("Looking for extra roles...")
+    with psycopg2.connect(remote_pg.connstr()) as conn:
+        with conn.cursor() as cur:
+            cur.execute(
+                "SELECT rolname FROM pg_catalog.pg_roles WHERE oid > 16384 AND rolname <> 'neondb_owner'"
+            )
+            roles: list[Any] = []
+            for role in cur:
+                log.info("Role found: %s", role[0])
+                roles.append(role[0])
+            for role in roles:
+                cur.execute(f"DROP ROLE {role}")
+            conn.commit()
+
+
 @pytest.mark.timeout(7200)
 @pytest.mark.remote_cluster
 def test_cloud_regress(
+    setup,
     remote_pg: RemotePostgres,
     pg_version: PgVersion,
     pg_distrib_dir: Path,

test_runner/fixtures/httpserver.py

@@ -7,25 +7,24 @@ from pytest_httpserver import HTTPServer
 
 if TYPE_CHECKING:
     from collections.abc import Iterator
-    from ssl import SSLContext
 
     from fixtures.port_distributor import PortDistributor
 
-    ListenAddress = tuple[str, int]
+# TODO: mypy fails with:
+#  Module "fixtures.neon_fixtures" does not explicitly export attribute "PortDistributor"  [attr-defined]
+# from fixtures.neon_fixtures import PortDistributor
 
 # compared to the fixtures from pytest_httpserver with same names, these are
 # always function scoped, so you can check and stop the server in tests.
 
 
 @pytest.fixture(scope="function")
-def httpserver_ssl_context() -> Iterator[SSLContext | None]:
-    yield None
+def httpserver_ssl_context():
+    return None
 
 
 @pytest.fixture(scope="function")
-def make_httpserver(
-    httpserver_listen_address: ListenAddress, httpserver_ssl_context: SSLContext | None
-) -> Iterator[HTTPServer]:
+def make_httpserver(httpserver_listen_address, httpserver_ssl_context) -> Iterator[HTTPServer]:
     host, port = httpserver_listen_address
     if not host:
         host = HTTPServer.DEFAULT_LISTEN_HOST
@@ -48,6 +47,6 @@ def httpserver(make_httpserver: HTTPServer) -> Iterator[HTTPServer]:
 
 
 @pytest.fixture(scope="function")
-def httpserver_listen_address(port_distributor: PortDistributor) -> ListenAddress:
+def httpserver_listen_address(port_distributor: PortDistributor) -> tuple[str, int]:
     port = port_distributor.get_port()
     return ("localhost", port)

test_runner/fixtures/metrics.py

@@ -178,7 +178,6 @@ PAGESERVER_PER_TENANT_METRICS: tuple[str, ...] = (
     counter("pageserver_timeline_wal_records_received"),
     counter("pageserver_page_service_pagestream_flush_in_progress_micros"),
     *histogram("pageserver_page_service_batch_size"),
-    *histogram("pageserver_page_service_pagestream_batch_wait_time_seconds"),
     *PAGESERVER_PER_TENANT_REMOTE_TIMELINE_CLIENT_METRICS,
     # "pageserver_directory_entries_count", -- only used if above a certain threshold
     # "pageserver_broken_tenants_count" -- used only for broken

test_runner/fixtures/neon_fixtures.py

@@ -2329,16 +2329,6 @@ class NeonStorageController(MetricsGetter, LogUtils):
                 return None
             raise e
 
-    def get_safekeepers(self) -> list[dict[str, Any]]:
-        response = self.request(
-            "GET",
-            f"{self.api}/control/v1/safekeeper",
-            headers=self.headers(TokenScope.ADMIN),
-        )
-        json = response.json()
-        assert isinstance(json, list)
-        return json
-
     def set_preferred_azs(self, preferred_azs: dict[TenantShardId, str]) -> list[TenantShardId]:
         response = self.request(
             "PUT",
@@ -4566,7 +4556,6 @@ class StorageScrubber:
     def __init__(self, env: NeonEnv, log_dir: Path):
         self.env = env
         self.log_dir = log_dir
-        self.allowed_errors: list[str] = []
 
     def scrubber_cli(
         self, args: list[str], timeout, extra_env: dict[str, str] | None = None
@@ -4644,70 +4633,19 @@ class StorageScrubber:
         if timeline_lsns is not None:
             args.append("--timeline-lsns")
             args.append(json.dumps(timeline_lsns))
-        if node_kind == NodeKind.PAGESERVER:
-            args.append("--verbose")
         stdout = self.scrubber_cli(args, timeout=30, extra_env=extra_env)
 
         try:
             summary = json.loads(stdout)
-            healthy = self._check_run_healthy(summary)
+            # summary does not contain "with_warnings" if node_kind is the safekeeper
+            no_warnings = "with_warnings" not in summary or not summary["with_warnings"]
+            healthy = not summary["with_errors"] and no_warnings
             return healthy, summary
         except:
             log.error("Failed to decode JSON output from `scan-metadata`.  Dumping stdout:")
             log.error(stdout)
             raise
 
-    def _check_line_allowed(self, line: str) -> bool:
-        for a in self.allowed_errors:
-            try:
-                if re.match(a, line):
-                    return True
-            except re.error:
-                log.error(f"Invalid regex: '{a}'")
-                raise
-        return False
-
-    def _check_line_list_allowed(self, lines: list[str]) -> bool:
-        for line in lines:
-            if not self._check_line_allowed(line):
-                return False
-        return True
-
-    def _check_run_healthy(self, summary: dict[str, Any]) -> bool:
-        # summary does not contain "with_warnings" if node_kind is the safekeeper
-        healthy = True
-        with_warnings = summary.get("with_warnings", None)
-        if with_warnings is not None:
-            if isinstance(with_warnings, list):
-                if len(with_warnings) > 0:
-                    # safekeeper scan_metadata output is a list of tenants
-                    healthy = False
-            else:
-                for _, warnings in with_warnings.items():
-                    assert (
-                        len(warnings) > 0
-                    ), "with_warnings value should not be empty, running without verbose mode?"
-                    if not self._check_line_list_allowed(warnings):
-                        healthy = False
-                        break
-        if not healthy:
-            return healthy
-        with_errors = summary.get("with_errors", None)
-        if with_errors is not None:
-            if isinstance(with_errors, list):
-                if len(with_errors) > 0:
-                    # safekeeper scan_metadata output is a list of tenants
-                    healthy = False
-            else:
-                for _, errors in with_errors.items():
-                    assert (
-                        len(errors) > 0
-                    ), "with_errors value should not be empty, running without verbose mode?"
-                    if not self._check_line_list_allowed(errors):
-                        healthy = False
-                        break
-        return healthy
-
     def tenant_snapshot(self, tenant_id: TenantId, output_path: Path):
         stdout = self.scrubber_cli(
             ["tenant-snapshot", "--tenant-id", str(tenant_id), "--output-path", str(output_path)],

test_runner/fixtures/pageserver/http.py

@@ -488,20 +488,7 @@ class PageserverHttpClient(requests.Session, MetricsGetter):
         )
         self.verbose_error(res)
 
-    def patch_tenant_config(self, tenant_id: TenantId | TenantShardId, updates: dict[str, Any]):
-        """
-        Only use this via storage_controller.pageserver_api().
-
-        See `set_tenant_config` for more information.
-        """
-        assert "tenant_id" not in updates.keys()
-        res = self.patch(
-            f"http://localhost:{self.port}/v1/tenant/config",
-            json={**updates, "tenant_id": str(tenant_id)},
-        )
-        self.verbose_error(res)
-
-    def update_tenant_config(
+    def patch_tenant_config_client_side(
         self,
         tenant_id: TenantId,
         inserts: dict[str, Any] | None = None,
@@ -512,13 +499,13 @@ class PageserverHttpClient(requests.Session, MetricsGetter):
 
         See `set_tenant_config` for more information.
         """
-        if inserts is None:
-            inserts = {}
-        if removes is None:
-            removes = []
-
-        patch = inserts | {remove: None for remove in removes}
-        self.patch_tenant_config(tenant_id, patch)
+        current = self.tenant_config(tenant_id).tenant_specific_overrides
+        if inserts is not None:
+            current.update(inserts)
+        if removes is not None:
+            for key in removes:
+                del current[key]
+        self.set_tenant_config(tenant_id, current)
 
     def tenant_size(self, tenant_id: TenantId | TenantShardId) -> int:
         return self.tenant_size_and_modelinputs(tenant_id)[0]

test_runner/regress/test_compaction.py

@@ -121,6 +121,9 @@ page_cache_size=10
     assert vectored_average < 8
 
 
+@pytest.mark.skip(
+    "This is being fixed and tracked in https://github.com/neondatabase/neon/issues/9114"
+)
 @skip_in_debug_build("only run with release build")
 def test_pageserver_gc_compaction_smoke(neon_env_builder: NeonEnvBuilder):
     SMOKE_CONF = {
@@ -153,20 +156,20 @@ def test_pageserver_gc_compaction_smoke(neon_env_builder: NeonEnvBuilder):
         if i % 10 == 0:
             log.info(f"Running churn round {i}/{churn_rounds} ...")
 
-            # Run gc-compaction every 10 rounds to ensure the test doesn't take too long time.
-            ps_http.timeline_compact(
-                tenant_id,
-                timeline_id,
-                enhanced_gc_bottom_most_compaction=True,
-                body={
-                    "scheduled": True,
-                    "sub_compaction": True,
-                    "compact_range": {
-                        "start": "000000000000000000000000000000000000",
-                        "end": "030000000000000000000000000000000000",
-                    },
+        ps_http.timeline_compact(
+            tenant_id,
+            timeline_id,
+            enhanced_gc_bottom_most_compaction=True,
+            body={
+                "scheduled": True,
+                "sub_compaction": True,
+                "compact_range": {
+                    "start": "000000000000000000000000000000000000",
+                    # skip the SLRU range for now -- it races with get-lsn-by-timestamp, TODO: fix this
+                    "end": "010000000000000000000000000000000000",
                 },
-            )
+            },
+        )
 
         workload.churn_rows(row_count, env.pageserver.id)
 
@@ -178,10 +181,6 @@ def test_pageserver_gc_compaction_smoke(neon_env_builder: NeonEnvBuilder):
     log.info("Validating at workload end ...")
     workload.validate(env.pageserver.id)
 
-    # Run a legacy compaction+gc to ensure gc-compaction can coexist with legacy compaction.
-    ps_http.timeline_checkpoint(tenant_id, timeline_id, wait_until_uploaded=True)
-    ps_http.timeline_gc(tenant_id, timeline_id, None)
-
 
 # Stripe sizes in number of pages.
 TINY_STRIPES = 16

test_runner/regress/test_ddl_forwarding.py

@@ -15,8 +15,6 @@ from werkzeug.wrappers.response import Response
 if TYPE_CHECKING:
     from typing import Any, Self
 
-    from fixtures.httpserver import ListenAddress
-
 
 def handle_db(dbs, roles, operation):
     if operation["op"] == "set":
@@ -122,7 +120,7 @@ class DdlForwardingContext:
 
 @pytest.fixture(scope="function")
 def ddl(
-    httpserver: HTTPServer, vanilla_pg: VanillaPostgres, httpserver_listen_address: ListenAddress
+    httpserver: HTTPServer, vanilla_pg: VanillaPostgres, httpserver_listen_address: tuple[str, int]
 ):
     (host, port) = httpserver_listen_address
     with DdlForwardingContext(httpserver, vanilla_pg, host, port) as ddl:

test_runner/regress/test_disk_usage_eviction.py

@@ -460,10 +460,10 @@ def test_pageserver_respects_overridden_resident_size(
     assert (
         du_by_timeline[large_tenant] > min_resident_size
     ), "ensure the larger tenant will get a haircut"
-    env.neon_env.storage_controller.pageserver_api().update_tenant_config(
+    env.neon_env.storage_controller.pageserver_api().patch_tenant_config_client_side(
         small_tenant[0], {"min_resident_size_override": min_resident_size}
     )
-    env.neon_env.storage_controller.pageserver_api().update_tenant_config(
+    env.neon_env.storage_controller.pageserver_api().patch_tenant_config_client_side(
         large_tenant[0], {"min_resident_size_override": min_resident_size}
     )
 

test_runner/regress/test_download_extensions.py

@@ -20,8 +20,6 @@ from werkzeug.wrappers.response import Response
 if TYPE_CHECKING:
     from typing import Any
 
-    from fixtures.httpserver import ListenAddress
-
 
 # use neon_env_builder_local fixture to override the default neon_env_builder fixture
 # and use a test-specific pg_install instead of shared one
@@ -49,8 +47,8 @@ def neon_env_builder_local(
 def test_remote_extensions(
     httpserver: HTTPServer,
     neon_env_builder_local: NeonEnvBuilder,
-    httpserver_listen_address: ListenAddress,
-    pg_version: PgVersion,
+    httpserver_listen_address,
+    pg_version,
 ):
     # setup mock http server
     # that expects request for anon.tar.zst

test_runner/regress/test_ingestion_layer_size.py

@@ -74,7 +74,7 @@ def test_ingesting_large_batches_of_images(neon_env_builder: NeonEnvBuilder):
     print_layer_size_histogram(post_ingest)
 
     # since all we have are L0s, we should be getting nice L1s and images out of them now
-    env.storage_controller.pageserver_api().update_tenant_config(
+    env.storage_controller.pageserver_api().patch_tenant_config_client_side(
         env.initial_tenant,
         {
             "compaction_threshold": 1,

test_runner/regress/test_installed_extensions.py

@@ -30,7 +30,7 @@ def test_installed_extensions(neon_simple_env: NeonEnv):
     info("Extensions: %s", res["extensions"])
     # 'plpgsql' is a default extension that is always installed.
     assert any(
-        ext["extname"] == "plpgsql" and ext["version"] == "1.0" for ext in res["extensions"]
+        ext["extname"] == "plpgsql" and ext["versions"] == ["1.0"] for ext in res["extensions"]
     ), "The 'plpgsql' extension is missing"
 
     # check that the neon_test_utils extension is not installed
@@ -63,7 +63,7 @@ def test_installed_extensions(neon_simple_env: NeonEnv):
     # and has the expected version
     assert any(
         ext["extname"] == "neon_test_utils"
-        and ext["version"] == neon_test_utils_version
+        and ext["versions"] == [neon_test_utils_version]
         and ext["n_databases"] == 1
         for ext in res["extensions"]
     )
@@ -75,8 +75,9 @@ def test_installed_extensions(neon_simple_env: NeonEnv):
     # check that the neon extension is installed and has expected versions
     for ext in res["extensions"]:
         if ext["extname"] == "neon":
-            assert ext["version"] in ["1.1", "1.2"]
-            assert ext["n_databases"] == 1
+            assert ext["n_databases"] == 2
+            ext["versions"].sort()
+            assert ext["versions"] == ["1.1", "1.2"]
 
     with pg_conn.cursor() as cur:
         cur.execute("ALTER EXTENSION neon UPDATE TO '1.3'")
@@ -89,8 +90,9 @@ def test_installed_extensions(neon_simple_env: NeonEnv):
     # check that the neon_test_utils extension is updated
     for ext in res["extensions"]:
         if ext["extname"] == "neon":
-            assert ext["version"] in ["1.2", "1.3"]
-            assert ext["n_databases"] == 1
+            assert ext["n_databases"] == 2
+            ext["versions"].sort()
+            assert ext["versions"] == ["1.2", "1.3"]
 
     # check that /metrics endpoint is available
     # ensure that we see the metric before and after restart
@@ -98,15 +100,13 @@ def test_installed_extensions(neon_simple_env: NeonEnv):
     info("Metrics: %s", res)
     m = parse_metrics(res)
     neon_m = m.query_all(
-        "compute_installed_extensions",
-        {"extension_name": "neon", "version": "1.2", "owned_by_superuser": "1"},
+        "compute_installed_extensions", {"extension_name": "neon", "version": "1.2"}
     )
     assert len(neon_m) == 1
     for sample in neon_m:
-        assert sample.value == 1
+        assert sample.value == 2
     neon_m = m.query_all(
-        "compute_installed_extensions",
-        {"extension_name": "neon", "version": "1.3", "owned_by_superuser": "1"},
+        "compute_installed_extensions", {"extension_name": "neon", "version": "1.3"}
     )
     assert len(neon_m) == 1
     for sample in neon_m:
@@ -138,16 +138,14 @@ def test_installed_extensions(neon_simple_env: NeonEnv):
         info("After restart metrics: %s", res)
         m = parse_metrics(res)
         neon_m = m.query_all(
-            "compute_installed_extensions",
-            {"extension_name": "neon", "version": "1.2", "owned_by_superuser": "1"},
+            "compute_installed_extensions", {"extension_name": "neon", "version": "1.2"}
         )
         assert len(neon_m) == 1
         for sample in neon_m:
             assert sample.value == 1
 
         neon_m = m.query_all(
-            "compute_installed_extensions",
-            {"extension_name": "neon", "version": "1.3", "owned_by_superuser": "1"},
+            "compute_installed_extensions", {"extension_name": "neon", "version": "1.3"}
         )
         assert len(neon_m) == 1
         for sample in neon_m:

test_runner/regress/test_layers_from_future.py

@@ -132,7 +132,7 @@ def test_issue_5878(neon_env_builder: NeonEnvBuilder, attach_mode: str):
     ), "sanity check for what above loop is supposed to do"
 
     # create the image layer from the future
-    env.storage_controller.pageserver_api().update_tenant_config(
+    env.storage_controller.pageserver_api().patch_tenant_config_client_side(
         tenant_id, {"image_creation_threshold": image_creation_threshold}, None
     )
     assert ps_http.tenant_config(tenant_id).effective_config["image_creation_threshold"] == 1

test_runner/regress/test_nbtree_pagesplit_cycleid.py

@@ -1,124 +0,0 @@
-import threading
-import time
-
-from fixtures.neon_fixtures import NeonEnv
-
-BTREE_NUM_CYCLEID_PAGES = """
-    WITH raw_pages AS (
-        SELECT blkno, get_raw_page_at_lsn('t_uidx', 'main', blkno, NULL, NULL) page
-        FROM generate_series(1, pg_relation_size('t_uidx'::regclass) / 8192) blkno
-    ),
-    parsed_pages AS (
-        /* cycle ID is the last 2 bytes of the btree page */
-        SELECT blkno, SUBSTRING(page FROM 8191 FOR 2) as cycle_id
-        FROM raw_pages
-    )
-    SELECT count(*),
-           encode(cycle_id, 'hex')
-     FROM parsed_pages
-    WHERE encode(cycle_id, 'hex') != '0000'
-    GROUP BY encode(cycle_id, 'hex');
-    """
-
-
-def test_nbtree_pagesplit_cycleid(neon_simple_env: NeonEnv):
-    env = neon_simple_env
-    endpoint = env.endpoints.create_start("main")
-
-    ses1 = endpoint.connect().cursor()
-    ses1.execute("ALTER SYSTEM SET autovacuum = off;")
-    ses1.execute("ALTER SYSTEM SET enable_seqscan = off;")
-    ses1.execute("ALTER SYSTEM SET full_page_writes = off;")
-    ses1.execute("SELECT pg_reload_conf();")
-    ses1.execute("CREATE EXTENSION neon_test_utils;")
-    # prepare a large index
-    ses1.execute("CREATE TABLE t(id integer GENERATED ALWAYS AS IDENTITY, txt text);")
-    ses1.execute("CREATE UNIQUE INDEX t_uidx ON t(id);")
-    ses1.execute("INSERT INTO t (txt) SELECT i::text FROM generate_series(1, 2035) i;")
-
-    ses1.execute("SELECT neon_xlogflush();")
-    ses1.execute(BTREE_NUM_CYCLEID_PAGES)
-    pages = ses1.fetchall()
-    assert (
-        len(pages) == 0
-    ), f"0 back splits with cycle ID expected, real {len(pages)} first {pages[0]}"
-    # Delete enough tuples to clear the first index page.
-    # (there are up to 407 rows per 8KiB page; 406 for non-rightmost leafs.
-    ses1.execute("DELETE FROM t WHERE id <= 406;")
-    # Make sure the page is cleaned up
-    ses1.execute("VACUUM (FREEZE, INDEX_CLEANUP ON) t;")
-
-    # Do another delete-then-indexcleanup cycle, to move the pages from
-    # "dead" to "reusable"
-    ses1.execute("DELETE FROM t WHERE id <= 446;")
-    ses1.execute("VACUUM (FREEZE, INDEX_CLEANUP ON) t;")
-
-    # Make sure the vacuum we're about to trigger in s3 has cleanup work to do
-    ses1.execute("DELETE FROM t WHERE id <= 610;")
-
-    # Flush wal, for checking purposes
-    ses1.execute("SELECT neon_xlogflush();")
-    ses1.execute(BTREE_NUM_CYCLEID_PAGES)
-    pages = ses1.fetchall()
-    assert len(pages) == 0, f"No back splits with cycle ID expected, got batches of {pages} instead"
-
-    ses2 = endpoint.connect().cursor()
-    ses3 = endpoint.connect().cursor()
-
-    # Session 2 pins a btree page, which prevents vacuum from processing that
-    # page, thus allowing us to reliably split pages while a concurrent vacuum
-    # is running.
-    ses2.execute("BEGIN;")
-    ses2.execute(
-        "DECLARE foo NO SCROLL CURSOR FOR SELECT row_number() over () FROM t ORDER BY id ASC"
-    )
-    ses2.execute("FETCH FROM foo;")  # pins the leaf page with id 611
-    wait_evt = threading.Event()
-
-    # Session 3 runs the VACUUM command. Note that this will block, and
-    # therefore must run on another thread.
-    # We rely on this running quickly enough to hit the pinned page from
-    # session 2 by the time we start other work again in session 1, but
-    # technically there is a race where the thread (and/or PostgreSQL process)
-    # don't get to that pinned page with vacuum until >2s after evt.set() was
-    # called, and session 1 thus might already have split pages.
-    def vacuum_freeze_t(ses3, evt: threading.Event):
-        # Begin parallel vacuum that should hit the index
-        evt.set()
-        # this'll hang until s2 fetches enough new data from its cursor.
-        # this is technically a race with the time.sleep(2) below, but if this
-        # command doesn't hit
-        ses3.execute("VACUUM (FREEZE, INDEX_CLEANUP on, DISABLE_PAGE_SKIPPING on) t;")
-
-    ses3t = threading.Thread(target=vacuum_freeze_t, args=(ses3, wait_evt))
-    ses3t.start()
-    wait_evt.wait()
-    # Make extra sure we got the thread started and vacuum is stuck, by waiting
-    # some time even after wait_evt got set. This isn't truly reliable (it is
-    # possible
-    time.sleep(2)
-
-    # Insert 2 pages worth of new data.
-    # This should reuse the one empty page, plus another page at the end of
-    # the index relation; with split ordering
-    #    old_blk -> blkno=1 -> old_blk + 1.
-    # As this is run while vacuum in session 3 is happening, these splits
-    # should receive cycle IDs where applicable.
-    ses1.execute("INSERT INTO t (txt) SELECT i::text FROM generate_series(1, 812) i;")
-    # unpin the btree page, allowing s3's vacuum to complete
-    ses2.execute("FETCH ALL FROM foo;")
-    ses2.execute("ROLLBACK;")
-    # flush WAL to make sure PS is up-to-date
-    ses1.execute("SELECT neon_xlogflush();")
-    # check that our expectations are correct
-    ses1.execute(BTREE_NUM_CYCLEID_PAGES)
-    pages = ses1.fetchall()
-    assert (
-        len(pages) == 1 and pages[0][0] == 3
-    ), f"3 page splits with cycle ID expected; actual {pages}"
-
-    # final cleanup
-    ses3t.join()
-    ses1.close()
-    ses2.close()
-    ses3.close()

test_runner/regress/test_pageserver_crash_consistency.py

@@ -46,7 +46,7 @@ def test_local_only_layers_after_crash(neon_env_builder: NeonEnvBuilder, pg_bin:
     for sk in env.safekeepers:
         sk.stop()
 
-    env.storage_controller.pageserver_api().update_tenant_config(
+    env.storage_controller.pageserver_api().patch_tenant_config_client_side(
         tenant_id, {"compaction_threshold": 3}
     )
     # hit the exit failpoint

test_runner/regress/test_pageserver_metric_collection.py

@@ -27,8 +27,6 @@ from werkzeug.wrappers.response import Response
 if TYPE_CHECKING:
     from typing import Any
 
-    from fixtures.httpserver import ListenAddress
-
 
 # TODO: collect all of the env setup *AFTER* removal of RemoteStorageKind.NOOP
 
@@ -36,7 +34,7 @@ if TYPE_CHECKING:
 def test_metric_collection(
     httpserver: HTTPServer,
     neon_env_builder: NeonEnvBuilder,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
 ):
     (host, port) = httpserver_listen_address
     metric_collection_endpoint = f"http://{host}:{port}/billing/api/v1/usage_events"
@@ -197,7 +195,7 @@ def test_metric_collection(
 def test_metric_collection_cleans_up_tempfile(
     httpserver: HTTPServer,
     neon_env_builder: NeonEnvBuilder,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
 ):
     (host, port) = httpserver_listen_address
     metric_collection_endpoint = f"http://{host}:{port}/billing/api/v1/usage_events"

test_runner/regress/test_proxy_metric_collection.py

@@ -2,7 +2,6 @@ from __future__ import annotations
 
 from collections.abc import Iterator
 from pathlib import Path
-from typing import TYPE_CHECKING
 
 import pytest
 from fixtures.log_helper import log
@@ -16,9 +15,6 @@ from pytest_httpserver import HTTPServer
 from werkzeug.wrappers.request import Request
 from werkzeug.wrappers.response import Response
 
-if TYPE_CHECKING:
-    from fixtures.httpserver import ListenAddress
-
 
 def proxy_metrics_handler(request: Request) -> Response:
     if request.json is None:
@@ -42,7 +38,7 @@ def proxy_metrics_handler(request: Request) -> Response:
 def proxy_with_metric_collector(
     port_distributor: PortDistributor,
     neon_binpath: Path,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
     test_output_dir: Path,
 ) -> Iterator[NeonProxy]:
     """Neon proxy that routes through link auth and has metric collection enabled."""

test_runner/regress/test_sharding.py

@@ -3,7 +3,7 @@ from __future__ import annotations
 import os
 import time
 from collections import defaultdict
-from typing import TYPE_CHECKING, Any
+from typing import Any
 
 import pytest
 import requests
@@ -27,9 +27,6 @@ from typing_extensions import override
 from werkzeug.wrappers.request import Request
 from werkzeug.wrappers.response import Response
 
-if TYPE_CHECKING:
-    from fixtures.httpserver import ListenAddress
-
 
 def test_sharding_smoke(
     neon_env_builder: NeonEnvBuilder,
@@ -762,7 +759,7 @@ def test_sharding_split_smoke(
 def test_sharding_split_stripe_size(
     neon_env_builder: NeonEnvBuilder,
     httpserver: HTTPServer,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
     initial_stripe_size: int,
 ):
     """

test_runner/regress/test_storage_controller.py

@@ -58,8 +58,6 @@ from werkzeug.wrappers.response import Response
 if TYPE_CHECKING:
     from typing import Any
 
-    from fixtures.httpserver import ListenAddress
-
 
 def get_node_shard_counts(env: NeonEnv, tenant_ids):
     counts: defaultdict[int, int] = defaultdict(int)
@@ -565,7 +563,7 @@ def test_storage_controller_onboard_detached(neon_env_builder: NeonEnvBuilder):
 def test_storage_controller_compute_hook(
     httpserver: HTTPServer,
     neon_env_builder: NeonEnvBuilder,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
 ):
     """
     Test that the sharding service calls out to the configured HTTP endpoint on attachment changes
@@ -683,7 +681,7 @@ NOTIFY_FAILURE_LOGS = [
 def test_storage_controller_stuck_compute_hook(
     httpserver: HTTPServer,
     neon_env_builder: NeonEnvBuilder,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
 ):
     """
     Test the migration process's behavior when the compute hook does not enable it to proceed
@@ -820,7 +818,7 @@ def test_storage_controller_stuck_compute_hook(
 def test_storage_controller_compute_hook_revert(
     httpserver: HTTPServer,
     neon_env_builder: NeonEnvBuilder,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
 ):
     """
     'revert' in the sense of a migration which gets reversed shortly after, as may happen during
@@ -1770,7 +1768,7 @@ def test_storcon_cli(neon_env_builder: NeonEnvBuilder):
     # Modify a tenant's config
     storcon_cli(
         [
-            "patch-tenant-config",
+            "tenant-config",
             "--tenant-id",
             str(env.initial_tenant),
             "--config",
@@ -2138,7 +2136,7 @@ def test_background_operation_cancellation(neon_env_builder: NeonEnvBuilder):
     env.start()
 
     tenant_count = 10
-    shard_count_per_tenant = 16
+    shard_count_per_tenant = 8
     tenant_ids = []
 
     for _ in range(0, tenant_count):
@@ -2405,7 +2403,7 @@ def test_storage_controller_step_down(neon_env_builder: NeonEnvBuilder):
 
     # Make a change to the tenant config to trigger a slow reconcile
     virtual_ps_http = PageserverHttpClient(env.storage_controller_port, lambda: True)
-    virtual_ps_http.update_tenant_config(tid, {"compaction_threshold": 5}, None)
+    virtual_ps_http.patch_tenant_config_client_side(tid, {"compaction_threshold": 5}, None)
     env.storage_controller.allowed_errors.extend(
         [
             ".*Accepted configuration update but reconciliation failed.*",
@@ -2955,8 +2953,6 @@ def test_safekeeper_deployment_time_update(neon_env_builder: NeonEnvBuilder):
 
     assert target.get_safekeeper(fake_id) is None
 
-    assert len(target.get_safekeepers()) == 0
-
     body = {
         "active": True,
         "id": fake_id,
@@ -2974,7 +2970,6 @@ def test_safekeeper_deployment_time_update(neon_env_builder: NeonEnvBuilder):
 
     inserted = target.get_safekeeper(fake_id)
     assert inserted is not None
-    assert target.get_safekeepers() == [inserted]
     assert eq_safekeeper_records(body, inserted)
 
     # error out if pk is changed (unexpected)
@@ -2986,7 +2981,6 @@ def test_safekeeper_deployment_time_update(neon_env_builder: NeonEnvBuilder):
     assert exc.value.status_code == 400
 
     inserted_again = target.get_safekeeper(fake_id)
-    assert target.get_safekeepers() == [inserted_again]
     assert inserted_again is not None
     assert eq_safekeeper_records(inserted, inserted_again)
 
@@ -2995,7 +2989,6 @@ def test_safekeeper_deployment_time_update(neon_env_builder: NeonEnvBuilder):
     body["version"] += 1
     target.on_safekeeper_deploy(fake_id, body)
     inserted_now = target.get_safekeeper(fake_id)
-    assert target.get_safekeepers() == [inserted_now]
     assert inserted_now is not None
 
     assert eq_safekeeper_records(body, inserted_now)
@@ -3237,55 +3230,3 @@ def test_multi_attached_timeline_creation(neon_env_builder: NeonEnvBuilder, migr
         # Always disable 'pause' failpoints, even on failure, to avoid hanging in shutdown
         env.storage_controller.configure_failpoints((migration_failpoint.value, "off"))
         raise
-
-
-@run_only_on_default_postgres("Postgres version makes no difference here")
-def test_storage_controller_detached_stopped(
-    neon_env_builder: NeonEnvBuilder,
-):
-    """
-    Test that detaching a tenant while it has scheduling policy set to Paused or Stop works
-    """
-
-    remote_storage_kind = s3_storage()
-    neon_env_builder.enable_pageserver_remote_storage(remote_storage_kind)
-
-    neon_env_builder.num_pageservers = 1
-
-    env = neon_env_builder.init_configs()
-    env.start()
-    virtual_ps_http = PageserverHttpClient(env.storage_controller_port, lambda: True)
-
-    tenant_id = TenantId.generate()
-    env.storage_controller.tenant_create(
-        tenant_id,
-        shard_count=1,
-    )
-
-    assert len(env.pageserver.http_client().tenant_list_locations()["tenant_shards"]) == 1
-
-    # Disable scheduling: ordinarily this would prevent the tenant's configuration being
-    # reconciled to pageservers, but this should be overridden when detaching.
-    env.storage_controller.allowed_errors.append(".*Scheduling is disabled by policy.*")
-    env.storage_controller.tenant_policy_update(
-        tenant_id,
-        {"scheduling": "Stop"},
-    )
-
-    env.storage_controller.consistency_check()
-
-    # Detach the tenant
-    virtual_ps_http.tenant_location_conf(
-        tenant_id,
-        {
-            "mode": "Detached",
-            "secondary_conf": None,
-            "tenant_conf": {},
-            "generation": None,
-        },
-    )
-
-    env.storage_controller.consistency_check()
-
-    # Confirm the detach happened
-    assert env.pageserver.http_client().tenant_list_locations()["tenant_shards"] == []

test_runner/regress/test_storage_scrubber.py

@@ -572,10 +572,4 @@ def test_scrubber_scan_pageserver_metadata(
     unhealthy = env.storage_controller.metadata_health_list_unhealthy()["unhealthy_tenant_shards"]
     assert len(unhealthy) == 1 and unhealthy[0] == str(tenant_shard_id)
 
-    healthy, _ = env.storage_scrubber.scan_metadata()
-    assert not healthy
-    env.storage_scrubber.allowed_errors.append(".*not present in remote storage.*")
-    healthy, _ = env.storage_scrubber.scan_metadata()
-    assert healthy
-
-    neon_env_builder.disable_scrub_on_exit()  # We already ran scrubber, no need to do an extra run
+    neon_env_builder.disable_scrub_on_exit()

test_runner/regress/test_tenant_conf.py

@@ -3,14 +3,13 @@ from __future__ import annotations
 import json
 from typing import TYPE_CHECKING
 
-import pytest
 from fixtures.common_types import Lsn
 from fixtures.neon_fixtures import (
     NeonEnvBuilder,
 )
 from fixtures.pageserver.utils import assert_tenant_state, wait_for_upload
 from fixtures.remote_storage import LocalFsStorage, RemoteStorageKind
-from fixtures.utils import run_only_on_default_postgres, wait_until
+from fixtures.utils import wait_until
 from fixtures.workload import Workload
 
 if TYPE_CHECKING:
@@ -331,83 +330,3 @@ def test_live_reconfig_get_evictions_low_residence_duration_metric_threshold(
     metric = get_metric()
     assert int(metric.labels["low_threshold_secs"]) == 24 * 60 * 60, "label resets to default"
     assert int(metric.value) == 0, "value resets to default"
-
-
-@run_only_on_default_postgres("Test does not start a compute")
-@pytest.mark.parametrize("ps_managed_by", ["storcon", "cplane"])
-def test_tenant_config_patch(neon_env_builder: NeonEnvBuilder, ps_managed_by: str):
-    """
-    Test tenant config patching (i.e. additive updates)
-
-    The flow is different for storage controller and cplane managed pageserver.
-    1. Storcon managed: /v1/tenant/config request lands on storcon, which generates
-    location_config calls containing the update to the pageserver
-    2. Cplane managed: /v1/tenant/config is called directly on the pageserver
-    """
-
-    def assert_tenant_conf_semantically_equal(lhs, rhs):
-        """
-        Storcon returns None for fields that are not set while the pageserver does not.
-        Compare two tenant's config overrides semantically, by dropping the None values.
-        """
-        lhs = {k: v for k, v in lhs.items() if v is not None}
-        rhs = {k: v for k, v in rhs.items() if v is not None}
-
-        assert lhs == rhs
-
-    env = neon_env_builder.init_start()
-
-    if ps_managed_by == "storcon":
-        api = env.storage_controller.pageserver_api()
-    elif ps_managed_by == "cplane":
-        # Disallow storcon from sending location_configs to the pageserver.
-        # These would overwrite the manually set tenant configs.
-        env.storage_controller.reconcile_until_idle()
-        env.storage_controller.tenant_policy_update(env.initial_tenant, {"scheduling": "Stop"})
-        env.storage_controller.allowed_errors.append(".*Scheduling is disabled by policy Stop.*")
-
-        api = env.pageserver.http_client()
-    else:
-        raise Exception(f"Unexpected value of ps_managed_by param: {ps_managed_by}")
-
-    crnt_tenant_conf = api.tenant_config(env.initial_tenant).tenant_specific_overrides
-
-    patch: dict[str, Any | None] = {
-        "gc_period": "3h",
-        "wal_receiver_protocol_override": {
-            "type": "interpreted",
-            "args": {"format": "bincode", "compression": {"zstd": {"level": 1}}},
-        },
-    }
-    api.patch_tenant_config(env.initial_tenant, patch)
-    tenant_conf_after_patch = api.tenant_config(env.initial_tenant).tenant_specific_overrides
-    if ps_managed_by == "storcon":
-        # Check that the config was propagated to the PS.
-        overrides_on_ps = (
-            env.pageserver.http_client().tenant_config(env.initial_tenant).tenant_specific_overrides
-        )
-        assert_tenant_conf_semantically_equal(overrides_on_ps, tenant_conf_after_patch)
-    assert_tenant_conf_semantically_equal(tenant_conf_after_patch, crnt_tenant_conf | patch)
-    crnt_tenant_conf = tenant_conf_after_patch
-
-    patch = {"gc_period": "5h", "wal_receiver_protocol_override": None}
-    api.patch_tenant_config(env.initial_tenant, patch)
-    tenant_conf_after_patch = api.tenant_config(env.initial_tenant).tenant_specific_overrides
-    if ps_managed_by == "storcon":
-        overrides_on_ps = (
-            env.pageserver.http_client().tenant_config(env.initial_tenant).tenant_specific_overrides
-        )
-        assert_tenant_conf_semantically_equal(overrides_on_ps, tenant_conf_after_patch)
-    assert_tenant_conf_semantically_equal(tenant_conf_after_patch, crnt_tenant_conf | patch)
-    crnt_tenant_conf = tenant_conf_after_patch
-
-    put = {"pitr_interval": "1m 1s"}
-    api.set_tenant_config(env.initial_tenant, put)
-    tenant_conf_after_put = api.tenant_config(env.initial_tenant).tenant_specific_overrides
-    if ps_managed_by == "storcon":
-        overrides_on_ps = (
-            env.pageserver.http_client().tenant_config(env.initial_tenant).tenant_specific_overrides
-        )
-        assert_tenant_conf_semantically_equal(overrides_on_ps, tenant_conf_after_put)
-    assert_tenant_conf_semantically_equal(tenant_conf_after_put, put)
-    crnt_tenant_conf = tenant_conf_after_put

test_runner/regress/test_threshold_based_eviction.py

@@ -2,7 +2,6 @@ from __future__ import annotations
 
 import time
 from dataclasses import dataclass
-from typing import TYPE_CHECKING
 
 from fixtures.log_helper import log
 from fixtures.neon_fixtures import (
@@ -14,15 +13,12 @@ from fixtures.pageserver.http import LayerMapInfo
 from fixtures.remote_storage import RemoteStorageKind
 from pytest_httpserver import HTTPServer
 
-if TYPE_CHECKING:
-    from fixtures.httpserver import ListenAddress
-
 # NB: basic config change tests are in test_tenant_conf.py
 
 
 def test_threshold_based_eviction(
     httpserver: HTTPServer,
-    httpserver_listen_address: ListenAddress,
+    httpserver_listen_address,
     pg_bin: PgBin,
     neon_env_builder: NeonEnvBuilder,
 ):
@@ -85,7 +81,7 @@ def test_threshold_based_eviction(
 
     # create a bunch of L1s, only the least of which will need to be resident
     compaction_threshold = 3  # create L1 layers quickly
-    vps_http.update_tenant_config(
+    vps_http.patch_tenant_config_client_side(
         tenant_id,
         inserts={
             # Disable gc and compaction to avoid on-demand downloads from their side.

test_runner/regress/test_timeline_detach_ancestor.py

@@ -514,7 +514,7 @@ def test_compaction_induced_by_detaches_in_history(
 
         assert len(delta_layers(branch_timeline_id)) == 5
 
-        env.storage_controller.pageserver_api().update_tenant_config(
+        env.storage_controller.pageserver_api().patch_tenant_config_client_side(
             env.initial_tenant, {"compaction_threshold": 5}, None
         )
 

test_runner/regress/test_vm_bits.py

@@ -3,9 +3,8 @@ from __future__ import annotations
 import time
 from contextlib import closing
 
-import pytest
 from fixtures.log_helper import log
-from fixtures.neon_fixtures import NeonEnv, NeonEnvBuilder, PgBin, fork_at_current_lsn
+from fixtures.neon_fixtures import NeonEnv, NeonEnvBuilder, fork_at_current_lsn
 from fixtures.utils import query_scalar
 
 
@@ -293,77 +292,3 @@ def test_vm_bit_clear_on_heap_lock_blackbox(neon_env_builder: NeonEnvBuilder):
     tup = cur.fetchall()
     log.info(f"tuple = {tup}")
     cur.execute("commit transaction")
-
-
-@pytest.mark.timeout(600)  # slow in debug builds
-def test_check_visibility_map(neon_env_builder: NeonEnvBuilder, pg_bin: PgBin):
-    """
-    Runs pgbench across a few databases on a sharded tenant, then performs a visibility map
-    consistency check. Regression test for https://github.com/neondatabase/neon/issues/9914.
-    """
-
-    # Use a large number of shards with small stripe sizes, to ensure the visibility
-    # map will end up on non-zero shards.
-    SHARD_COUNT = 8
-    STRIPE_SIZE = 32  # in 8KB pages
-    PGBENCH_RUNS = 4
-
-    env = neon_env_builder.init_start(
-        initial_tenant_shard_count=SHARD_COUNT, initial_tenant_shard_stripe_size=STRIPE_SIZE
-    )
-    endpoint = env.endpoints.create_start(
-        "main",
-        config_lines=[
-            "shared_buffers = 64MB",
-        ],
-    )
-
-    # Run pgbench in 4 different databases, to exercise different shards.
-    dbnames = [f"pgbench{i}" for i in range(PGBENCH_RUNS)]
-    for i, dbname in enumerate(dbnames):
-        log.info(f"pgbench run {i+1}/{PGBENCH_RUNS}")
-        endpoint.safe_psql(f"create database {dbname}")
-        connstr = endpoint.connstr(dbname=dbname)
-        # pgbench -i will automatically vacuum the tables. This creates the visibility map.
-        pg_bin.run(["pgbench", "-i", "-s", "10", connstr])
-        # Freeze the tuples to set the initial frozen bit.
-        endpoint.safe_psql("vacuum freeze", dbname=dbname)
-        # Run pgbench.
-        pg_bin.run(["pgbench", "-c", "32", "-j", "8", "-T", "10", connstr])
-
-    # Restart the endpoint to flush the compute page cache. We want to make sure we read VM pages
-    # from storage, not cache.
-    endpoint.stop()
-    endpoint.start()
-
-    # Check that the visibility map matches the heap contents for pg_accounts (the main table).
-    for dbname in dbnames:
-        log.info(f"Checking visibility map for {dbname}")
-        with endpoint.cursor(dbname=dbname) as cur:
-            cur.execute("create extension pg_visibility")
-
-            cur.execute("select count(*) from pg_check_visible('pgbench_accounts')")
-            row = cur.fetchone()
-            assert row is not None
-            assert row[0] == 0, f"{row[0]} inconsistent VM pages (visible)"
-
-            cur.execute("select count(*) from pg_check_frozen('pgbench_accounts')")
-            row = cur.fetchone()
-            assert row is not None
-            assert row[0] == 0, f"{row[0]} inconsistent VM pages (frozen)"
-
-    # Vacuum and freeze the tables, and check that the visibility map is still accurate.
-    for dbname in dbnames:
-        log.info(f"Vacuuming and checking visibility map for {dbname}")
-        with endpoint.cursor(dbname=dbname) as cur:
-            cur.execute("vacuum freeze")
-
-            cur.execute("select count(*) from pg_check_visible('pgbench_accounts')")
-            row = cur.fetchone()
-            assert row is not None
-            assert row[0] == 0, f"{row[0]} inconsistent VM pages (visible)"
-
-            cur.execute("select count(*) from pg_check_frozen('pgbench_accounts')")
-            row = cur.fetchone()
-            assert row is not None
-            assert row[0] == 0, f"{row[0]} inconsistent VM pages (frozen)"