Fix chunked_buffer clippy

Still lots of work to do here. Also a lot of copying going on that
should be avoided.

.circleci/config.yml

@@ -243,6 +243,9 @@ jobs:
       - run:
           name: Run pytest
           working_directory: test_runner
+          # pytest doesn't output test logs in real time, so CI job may fail
+          # with `Too long with no output` error, if a test is running for a long time.
+          no_output_timeout: 20m
           environment:
             - ZENITH_BIN: /tmp/zenith/bin
             - POSTGRES_DISTRIB_DIR: /tmp/zenith/pg_install
@@ -267,7 +270,7 @@ jobs:
             # -n4 uses four processes to run tests via pytest-xdist
             # -s is not used to prevent pytest from capturing output, because tests are running
             # in parallel and logs are mixed between different tests
-            pipenv run pytest --junitxml=$TEST_OUTPUT/junit.xml --tb=short --verbose -rA $TEST_SELECTION $EXTRA_PARAMS
+            pipenv run pytest --junitxml=$TEST_OUTPUT/junit.xml --tb=short --verbose -m "not remote_cluster" -rA $TEST_SELECTION $EXTRA_PARAMS
       - run:
           # CircleCI artifacts are preserved one file at a time, so skipping
           # this step isn't a good idea. If you want to extract the
@@ -299,7 +302,7 @@ jobs:
           name: Build and push Docker image
           command: |
             echo $DOCKER_PWD | docker login -u $DOCKER_LOGIN --password-stdin
-            docker build -t zenithdb/zenith:latest . && docker push zenithdb/zenith:latest
+            docker build --build-arg GIT_VERSION=$CIRCLE_SHA1 -t zenithdb/zenith:latest . && docker push zenithdb/zenith:latest
 
   # Trigger a new remote CI job
   remote-ci-trigger:

.github/workflows/benchmarking.yml

@@ -0,0 +1,158 @@
+name: benchmarking
+
+on:
+  # uncomment to run on push for debugging your PR
+  # push:
+  #   branches: [ mybranch ]
+  schedule:
+    # * is a special character in YAML so you have to quote this string
+    #          ┌───────────── minute (0 - 59)
+    #          │ ┌───────────── hour (0 - 23)
+    #          │ │ ┌───────────── day of the month (1 - 31)
+    #          │ │ │ ┌───────────── month (1 - 12 or JAN-DEC)
+    #          │ │ │ │ ┌───────────── day of the week (0 - 6 or SUN-SAT)
+    - cron:  '36 7 * * *' # run once a day, timezone is utc
+
+env:
+  BASE_URL: "https://console.zenith.tech"
+
+jobs:
+  bench:
+    # this workflow runs on self hosteed runner
+    # it's environment is quite different from usual guthub runner
+    # probably the most important difference is that it doesnt start from clean workspace each time
+    # e g if you install system packages they are not cleaned up since you install them directly in host machine
+    # not a container or something
+    # See documentation for more info: https://docs.github.com/en/actions/hosting-your-own-runners/about-self-hosted-runners
+    runs-on: [self-hosted, zenith-benchmarker]
+
+    steps:
+    - name: Checkout zenith repo
+      uses: actions/checkout@v2
+
+    - name: Checkout zenith-perf-data repo
+      uses: actions/checkout@v2
+      with:
+        repository: zenithdb/zenith-perf-data
+        token: ${{ secrets.VIP_VAP_ACCESS_TOKEN }}
+        ref: testing # TODO replace with master once everything is ready
+        path: zenith-perf-data
+
+    # actions/setup-python@v2 is not working correctly on self-hosted runners
+    # see https://github.com/actions/setup-python/issues/162
+    # and probably https://github.com/actions/setup-python/issues/162#issuecomment-865387976 in particular
+    # so the simplest solution to me is to use already installed system python and spin virtualenvs for job runs.
+    # there is Python 3.7.10 already installed on the machine so use it to install pipenv and then use pipenv's virtuealenvs
+    - name: Install pipenv & deps
+      run: |
+        python3 -m pip install --upgrade pipenv wheel
+        # since pip/pipenv caches are reused there shouldn't be any troubles with install every time
+        pipenv install
+
+    - name: Show versions
+      run: |
+        echo Python
+        python3 --version
+        pipenv run python3 --version
+        echo Pipenv
+        pipenv --version
+        echo Pgbench
+        pgbench --version
+
+    # FIXME cluster setup is skipped due to various changes in console API
+    # for now pre created cluster is used. When API gain some stability
+    # after massive changes dynamic cluster setup will be revived.
+    # So use pre created cluster. It needs to be started manually, but stop is automatic after 5 minutes of inactivity
+    - name: Setup cluster
+      env:
+        BENCHMARK_CONSOLE_USER_PASSWORD: "${{ secrets.BENCHMARK_CONSOLE_USER_PASSWORD }}"
+        BENCHMARK_CONSOLE_ACCESS_TOKEN: "${{ secrets.BENCHMARK_CONSOLE_ACCESS_TOKEN }}"
+        # USERNAME: "benchmark"
+      shell: bash
+      run: |
+        set -e
+        # echo "Creating cluster"
+
+        # CLUSTER=$(curl -s --fail --show-error $BASE_URL/api/v1/clusters.json \
+        #     -H 'Content-Type: application/json; charset=utf-8' \
+        #     -H "Authorization: Bearer $BENCHMARK_CONSOLE_ACCESS_TOKEN" \
+        #     --data-binary @- << EOF
+        # {
+        #     "cluster": {
+        #         "name": "default_cluster",
+        #         "region_id": "2",
+        #         "instance_type_id": 7,
+        #         "settings": {}
+        #     },
+        #     "database": {"name": "benchmark"},
+        #     "role": {"name": "$USERNAME", "password": "$BENCHMARK_CONSOLE_USER_PASSWORD"}
+        # }
+        # EOF
+        # )
+
+        # echo "Created cluster"
+
+        echo "Starting cluster"
+        CLUSTER_ID=285
+        CLUSTER=$(curl -s --fail --show-error -X POST $BASE_URL/api/v1/clusters/$CLUSTER_ID/start \
+            -H "Authorization: Bearer $BENCHMARK_CONSOLE_ACCESS_TOKEN")
+        echo $CLUSTER | python -m json.tool
+
+        echo "Waiting for cluster to become ready"
+        sleep 10
+
+        # # note that jq is installed on host system
+        # CLUSTER_ID=$(echo $CLUSTER| jq ".id")
+        echo "CLUSTER_ID=$CLUSTER_ID" >> $GITHUB_ENV
+        # echo "Constructing connstr"
+        # CLUSTER=$(curl -s --fail --show-error -X GET $BASE_URL/api/v1/clusters/$CLUSTER_ID.json \
+        #     -H "Authorization: Bearer $BENCHMARK_CONSOLE_ACCESS_TOKEN")
+
+        # echo $CLUSTER | python -m json.tool
+        # CONNSTR=$(echo $CLUSTER | jq -r ".| \"postgresql://$USERNAME:$BENCHMARK_CONSOLE_USER_PASSWORD@\(.public_ip_address):\(.public_pg_port)/benchmark\"")
+        # echo "BENCHMARK_CONNSTR=$CONNSTR" >> $GITHUB_ENV
+
+    - name: Run benchmark
+      # pgbench is installed system wide from official repo
+      # https://download.postgresql.org/pub/repos/yum/13/redhat/rhel-7-x86_64/
+      # via
+      # sudo tee /etc/yum.repos.d/pgdg.repo<<EOF
+      # [pgdg13]
+      # name=PostgreSQL 13 for RHEL/CentOS 7 - x86_64
+      # baseurl=https://download.postgresql.org/pub/repos/yum/13/redhat/rhel-7-x86_64/
+      # enabled=1
+      # gpgcheck=0
+      # EOF
+      # sudo yum makecache
+      # sudo yum install postgresql13-contrib
+      # actual binaries are located in /usr/pgsql-13/bin/
+      env:
+        PG_BIN: "/usr/pgsql-13/bin/"
+        TEST_PG_BENCH_TRANSACTIONS_MATRIX: "5000,10000,20000"
+        TEST_PG_BENCH_SCALES_MATRIX: "10,15"
+        PLATFORM: "zenith-staging"
+        BENCHMARK_CONSOLE_ACCESS_TOKEN: "${{ secrets.BENCHMARK_CONSOLE_ACCESS_TOKEN }}"
+        BENCHMARK_CONNSTR: "${{ secrets.BENCHMARK_STAGING_CONNSTR }}"
+        REMOTE_ENV: "1" # indicate to test harness that we do not have zenith binaries locally
+      run: |
+        mkdir -p zenith-perf-data/data/staging
+        pipenv run pytest test_runner/performance/ -v -m "remote_cluster" --skip-interfering-proc-check --out-dir zenith-perf-data/data/staging
+
+    - name: Submit result
+      env:
+        VIP_VAP_ACCESS_TOKEN: "${{ secrets.VIP_VAP_ACCESS_TOKEN }}"
+      run: |
+        cd zenith-perf-data
+        git add data
+        git commit --author="vipvap <vipvap@zenith.tech>" -m "add performance test result for $GITHUB_SHA zenith revision"
+        git push https://$VIP_VAP_ACCESS_TOKEN@github.com/zenithdb/zenith-perf-data.git main
+
+    # FIXME see comment above Setup cluster job
+    # change to delete cluster after switching to creating a cluster for every run
+    - name: Stop cluster
+      if: ${{ always() }}
+      env:
+        BENCHMARK_CONSOLE_ACCESS_TOKEN: "${{ secrets.BENCHMARK_CONSOLE_ACCESS_TOKEN }}"
+      run: |
+        curl -s --fail --show-error -X POST $BASE_URL/api/v1/clusters/$CLUSTER_ID/stop \
+            -H "Authorization: Bearer $BENCHMARK_CONSOLE_ACCESS_TOKEN"

Cargo.lock

@@ -654,6 +654,28 @@ dependencies = [
  "wasi",
 ]
 
+[[package]]
+name = "git-version"
+version = "0.3.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f6b0decc02f4636b9ccad390dcbe77b722a77efedfa393caf8379a51d5c61899"
+dependencies = [
+ "git-version-macro",
+ "proc-macro-hack",
+]
+
+[[package]]
+name = "git-version-macro"
+version = "0.3.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "fe69f1cbdb6e28af2bac214e943b99ce8a0a06b447d15d3e61161b0423139f3f"
+dependencies = [
+ "proc-macro-hack",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "glob"
 version = "0.3.0"
@@ -1195,6 +1217,7 @@ dependencies = [
  "hyper",
  "lazy_static",
  "log",
+ "nix",
  "postgres",
  "postgres-protocol",
  "postgres-types",
@@ -2573,6 +2596,7 @@ dependencies = [
  "bincode",
  "byteorder",
  "bytes",
+ "git-version",
  "hex",
  "hex-literal",
  "hyper",

Dockerfile

@@ -21,11 +21,15 @@ RUN rm -rf postgres_install/build
 # net time waste in a lot of cases. Copying Cargo.lock with empty lib.rs should do the work.
 #
 FROM zenithdb/build:buster AS build
+
+ARG GIT_VERSION
+RUN if [ -z "$GIT_VERSION" ]; then echo "GIT_VERSION is reqired, use build_arg to pass it"; exit 1; fi
+
 WORKDIR /zenith
 COPY --from=pg-build /zenith/tmp_install/include/postgresql/server tmp_install/include/postgresql/server
 
 COPY . .
-RUN cargo build --release
+RUN GIT_VERSION=$GIT_VERSION cargo build --release
 
 #
 # Copy binaries to resulting image.

pageserver/Cargo.toml

@@ -38,6 +38,7 @@ const_format = "0.2.21"
 tracing = "0.1.27"
 signal-hook = {version = "0.3.10", features = ["extended-siginfo"] }
 url = "2"
+nix = "0.23"
 
 postgres_ffi = { path = "../postgres_ffi" }
 zenith_metrics = { path = "../zenith_metrics" }

pageserver/README.md

@@ -129,7 +129,7 @@ There are the following implementations present:
 * local filesystem — to use in tests mainly
 * AWS S3           - to use in production
 
-Implementation details are covered in the [storage readme](./src/relish_storage/README.md) and corresponding Rust file docs.
+Implementation details are covered in the [backup readme](./src/remote_storage/README.md) and corresponding Rust file docs.
 
 The backup service is disabled by default and can be enabled to interact with a single remote storage.
 
@@ -140,18 +140,18 @@ CLI examples:
 For Amazon AWS S3, a key id and secret access key could be located in `~/.aws/credentials` if awscli was ever configured to work with the desired bucket, on the AWS Settings page for a certain user. Also note, that the bucket names does not contain any protocols when used on AWS.
 For local S3 installations, refer to the their documentation for name format and credentials.
 
-Similar to other pageserver settings, toml config file can be used to configure either of the storages as backup backup targets.
+Similar to other pageserver settings, toml config file can be used to configure either of the storages as backup targets.
 Required sections are:
 
 ```toml
-[relish_storage]
+[remote_storage]
 local_path = '/Users/someonetoignore/Downloads/tmp_dir/'
 ```
 
 or
 
 ```toml
-[relish_storage]
+[remote_storage]
 bucket_name = 'some-sample-bucket'
 bucket_region = 'eu-north-1'
 access_key_id = 'SOMEKEYAAAAASADSAH*#'

pageserver/src/bin/dump_layerfile.rs

@@ -5,10 +5,12 @@ use anyhow::Result;
 use clap::{App, Arg};
 use pageserver::layered_repository::dump_layerfile_from_path;
 use std::path::PathBuf;
+use zenith_utils::GIT_VERSION;
 
 fn main() -> Result<()> {
     let arg_matches = App::new("Zenith dump_layerfile utility")
         .about("Dump contents of one layer file, for debugging")
+        .version(GIT_VERSION)
         .arg(
             Arg::with_name("path")
                 .help("Path to file to dump")

pageserver/src/bin/pageserver.rs

@@ -10,7 +10,7 @@ use std::{
     thread,
 };
 use tracing::*;
-use zenith_utils::{auth::JwtAuth, logging, postgres_backend::AuthType, tcp_listener};
+use zenith_utils::{auth::JwtAuth, logging, postgres_backend::AuthType, tcp_listener, GIT_VERSION};
 
 use anyhow::{bail, ensure, Context, Result};
 use signal_hook::consts::signal::*;
@@ -26,8 +26,8 @@ use clap::{App, Arg, ArgMatches};
 use daemonize::Daemonize;
 
 use pageserver::{
-    branches, defaults::*, http, page_service, relish_storage, tenant_mgr, PageServerConf,
-    RelishStorageConfig, RelishStorageKind, S3Config, LOG_FILE_NAME,
+    branches, defaults::*, http, page_service, remote_storage, tenant_mgr, PageServerConf,
+    RemoteStorageConfig, RemoteStorageKind, S3Config, LOG_FILE_NAME,
 };
 use zenith_utils::http::endpoint;
 use zenith_utils::postgres_backend;
@@ -43,25 +43,26 @@ struct CfgFileParams {
     checkpoint_period: Option<String>,
     gc_horizon: Option<String>,
     gc_period: Option<String>,
+    open_mem_limit: Option<String>,
     pg_distrib_dir: Option<String>,
     auth_validation_public_key_path: Option<String>,
     auth_type: Option<String>,
-    relish_storage_max_concurrent_sync: Option<String>,
+    remote_storage_max_concurrent_sync: Option<String>,
     /////////////////////////////////
     //// Don't put `Option<String>` and other "simple" values below.
     ////
-    /// `Option<RelishStorage>` is a <a href='https://toml.io/en/v1.0.0#table'>table</a> in TOML.
+    /// `Option<RemoteStorage>` is a <a href='https://toml.io/en/v1.0.0#table'>table</a> in TOML.
     /// Values in TOML cannot be defined after tables (other tables can),
     /// and [`toml`] crate serializes all fields in the order of their appearance.
     ////////////////////////////////
-    relish_storage: Option<RelishStorage>,
+    remote_storage: Option<RemoteStorage>,
 }
 
 #[derive(Serialize, Deserialize, PartialEq, Eq, Clone)]
 // Without this attribute, enums with values won't be serialized by the `toml` library (but can be deserialized nonetheless!).
 // See https://github.com/alexcrichton/toml-rs/blob/6c162e6562c3e432bf04c82a3d1d789d80761a86/examples/enum_external.rs for the examples
 #[serde(untagged)]
-enum RelishStorage {
+enum RemoteStorage {
     Local {
         local_path: String,
     },
@@ -82,12 +83,12 @@ impl CfgFileParams {
             arg_matches.value_of(arg_name).map(str::to_owned)
         };
 
-        let relish_storage = if let Some(local_path) = get_arg("relish-storage-local-path") {
-            Some(RelishStorage::Local { local_path })
+        let remote_storage = if let Some(local_path) = get_arg("relish-storage-local-path") {
+            Some(RemoteStorage::Local { local_path })
         } else if let Some((bucket_name, bucket_region)) =
             get_arg("relish-storage-s3-bucket").zip(get_arg("relish-storage-region"))
         {
-            Some(RelishStorage::AwsS3 {
+            Some(RemoteStorage::AwsS3 {
                 bucket_name,
                 bucket_region,
                 access_key_id: get_arg("relish-storage-access-key"),
@@ -104,11 +105,12 @@ impl CfgFileParams {
             checkpoint_period: get_arg("checkpoint_period"),
             gc_horizon: get_arg("gc_horizon"),
             gc_period: get_arg("gc_period"),
+            open_mem_limit: get_arg("open_mem_limit"),
             pg_distrib_dir: get_arg("postgres-distrib"),
             auth_validation_public_key_path: get_arg("auth-validation-public-key-path"),
             auth_type: get_arg("auth-type"),
-            relish_storage,
-            relish_storage_max_concurrent_sync: get_arg("relish-storage-max-concurrent-sync"),
+            remote_storage,
+            remote_storage_max_concurrent_sync: get_arg("relish-storage-max-concurrent-sync"),
         }
     }
 
@@ -122,15 +124,16 @@ impl CfgFileParams {
             checkpoint_period: self.checkpoint_period.or(other.checkpoint_period),
             gc_horizon: self.gc_horizon.or(other.gc_horizon),
             gc_period: self.gc_period.or(other.gc_period),
+            open_mem_limit: self.open_mem_limit.or(other.open_mem_limit),
             pg_distrib_dir: self.pg_distrib_dir.or(other.pg_distrib_dir),
             auth_validation_public_key_path: self
                 .auth_validation_public_key_path
                 .or(other.auth_validation_public_key_path),
             auth_type: self.auth_type.or(other.auth_type),
-            relish_storage: self.relish_storage.or(other.relish_storage),
-            relish_storage_max_concurrent_sync: self
-                .relish_storage_max_concurrent_sync
-                .or(other.relish_storage_max_concurrent_sync),
+            remote_storage: self.remote_storage.or(other.remote_storage),
+            remote_storage_max_concurrent_sync: self
+                .remote_storage_max_concurrent_sync
+                .or(other.remote_storage_max_concurrent_sync),
         }
     }
 
@@ -166,6 +169,11 @@ impl CfgFileParams {
             None => DEFAULT_GC_PERIOD,
         };
 
+        let open_mem_limit: usize = match self.open_mem_limit.as_ref() {
+            Some(open_mem_limit_str) => open_mem_limit_str.parse()?,
+            None => DEFAULT_OPEN_MEM_LIMIT,
+        };
+
         let pg_distrib_dir = match self.pg_distrib_dir.as_ref() {
             Some(pg_distrib_dir_str) => PathBuf::from(pg_distrib_dir_str),
             None => env::current_dir()?.join("tmp_install"),
@@ -199,30 +207,28 @@ impl CfgFileParams {
             );
         }
 
-        let max_concurrent_sync = match self.relish_storage_max_concurrent_sync.as_deref() {
-            Some(relish_storage_max_concurrent_sync) => {
-                relish_storage_max_concurrent_sync.parse()?
-            }
-            None => DEFAULT_RELISH_STORAGE_MAX_CONCURRENT_SYNC_LIMITS,
+        let max_concurrent_sync = match self.remote_storage_max_concurrent_sync.as_deref() {
+            Some(number_str) => number_str.parse()?,
+            None => DEFAULT_REMOTE_STORAGE_MAX_CONCURRENT_SYNC_LIMITS,
         };
-        let relish_storage_config = self.relish_storage.as_ref().map(|storage_params| {
+        let remote_storage_config = self.remote_storage.as_ref().map(|storage_params| {
             let storage = match storage_params.clone() {
-                RelishStorage::Local { local_path } => {
-                    RelishStorageKind::LocalFs(PathBuf::from(local_path))
+                RemoteStorage::Local { local_path } => {
+                    RemoteStorageKind::LocalFs(PathBuf::from(local_path))
                 }
-                RelishStorage::AwsS3 {
+                RemoteStorage::AwsS3 {
                     bucket_name,
                     bucket_region,
                     access_key_id,
                     secret_access_key,
-                } => RelishStorageKind::AwsS3(S3Config {
+                } => RemoteStorageKind::AwsS3(S3Config {
                     bucket_name,
                     bucket_region,
                     access_key_id,
                     secret_access_key,
                 }),
             };
-            RelishStorageConfig {
+            RemoteStorageConfig {
                 max_concurrent_sync,
                 storage,
             }
@@ -237,6 +243,7 @@ impl CfgFileParams {
             checkpoint_period,
             gc_horizon,
             gc_period,
+            open_mem_limit,
 
             superuser: String::from(DEFAULT_SUPERUSER),
 
@@ -246,7 +253,7 @@ impl CfgFileParams {
 
             auth_validation_public_key_path,
             auth_type,
-            relish_storage_config,
+            remote_storage_config,
         })
     }
 }
@@ -255,6 +262,7 @@ fn main() -> Result<()> {
     zenith_metrics::set_common_metrics_prefix("pageserver");
     let arg_matches = App::new("Zenith page server")
         .about("Materializes WAL stream to pages and serves them to the postgres")
+        .version(GIT_VERSION)
         .arg(
             Arg::with_name("listen-pg")
                 .short("l")
@@ -307,6 +315,12 @@ fn main() -> Result<()> {
                 .takes_value(true)
                 .help("Interval between garbage collector iterations"),
         )
+        .arg(
+            Arg::with_name("open_mem_limit")
+                .long("open_mem_limit")
+                .takes_value(true)
+                .help("Amount of memory reserved for buffering incoming WAL"),
+        )
         .arg(
             Arg::with_name("workdir")
                 .short("D")
@@ -461,6 +475,8 @@ fn start_pageserver(conf: &'static PageServerConf) -> Result<()> {
     // Initialize logger
     let log_file = logging::init(LOG_FILE_NAME, conf.daemonize)?;
 
+    info!("version: {}", GIT_VERSION);
+
     let term_now = Arc::new(AtomicBool::new(false));
     for sig in TERM_SIGNALS {
         // When terminated by a second term signal, exit with exit code 1.
@@ -511,7 +527,7 @@ fn start_pageserver(conf: &'static PageServerConf) -> Result<()> {
     // don't spawn threads before daemonizing
     let mut join_handles = Vec::new();
 
-    if let Some(handle) = relish_storage::run_storage_sync_thread(conf)? {
+    if let Some(handle) = remote_storage::run_storage_sync_thread(conf)? {
         join_handles.push(handle);
     }
     // Initialize tenant manager.
@@ -601,16 +617,17 @@ mod tests {
             checkpoint_period: Some("checkpoint_period_VALUE".to_string()),
             gc_horizon: Some("gc_horizon_VALUE".to_string()),
             gc_period: Some("gc_period_VALUE".to_string()),
+            open_mem_limit: Some("open_mem_limit_VALUE".to_string()),
             pg_distrib_dir: Some("pg_distrib_dir_VALUE".to_string()),
             auth_validation_public_key_path: Some(
                 "auth_validation_public_key_path_VALUE".to_string(),
             ),
             auth_type: Some("auth_type_VALUE".to_string()),
-            relish_storage: Some(RelishStorage::Local {
-                local_path: "relish_storage_local_VALUE".to_string(),
+            remote_storage: Some(RemoteStorage::Local {
+                local_path: "remote_storage_local_VALUE".to_string(),
             }),
-            relish_storage_max_concurrent_sync: Some(
-                "relish_storage_max_concurrent_sync_VALUE".to_string(),
+            remote_storage_max_concurrent_sync: Some(
+                "remote_storage_max_concurrent_sync_VALUE".to_string(),
             ),
         };
 
@@ -624,13 +641,14 @@ checkpoint_distance = 'checkpoint_distance_VALUE'
 checkpoint_period = 'checkpoint_period_VALUE'
 gc_horizon = 'gc_horizon_VALUE'
 gc_period = 'gc_period_VALUE'
+open_mem_limit = 'open_mem_limit_VALUE'
 pg_distrib_dir = 'pg_distrib_dir_VALUE'
 auth_validation_public_key_path = 'auth_validation_public_key_path_VALUE'
 auth_type = 'auth_type_VALUE'
-relish_storage_max_concurrent_sync = 'relish_storage_max_concurrent_sync_VALUE'
+remote_storage_max_concurrent_sync = 'remote_storage_max_concurrent_sync_VALUE'
 
-[relish_storage]
-local_path = 'relish_storage_local_VALUE'
+[remote_storage]
+local_path = 'remote_storage_local_VALUE'
 "#,
             toml_pretty_string
         );
@@ -658,19 +676,20 @@ local_path = 'relish_storage_local_VALUE'
             checkpoint_period: Some("checkpoint_period_VALUE".to_string()),
             gc_horizon: Some("gc_horizon_VALUE".to_string()),
             gc_period: Some("gc_period_VALUE".to_string()),
+            open_mem_limit: Some("open_mem_limit_VALUE".to_string()),
             pg_distrib_dir: Some("pg_distrib_dir_VALUE".to_string()),
             auth_validation_public_key_path: Some(
                 "auth_validation_public_key_path_VALUE".to_string(),
             ),
             auth_type: Some("auth_type_VALUE".to_string()),
-            relish_storage: Some(RelishStorage::AwsS3 {
+            remote_storage: Some(RemoteStorage::AwsS3 {
                 bucket_name: "bucket_name_VALUE".to_string(),
                 bucket_region: "bucket_region_VALUE".to_string(),
                 access_key_id: Some("access_key_id_VALUE".to_string()),
                 secret_access_key: Some("secret_access_key_VALUE".to_string()),
             }),
-            relish_storage_max_concurrent_sync: Some(
-                "relish_storage_max_concurrent_sync_VALUE".to_string(),
+            remote_storage_max_concurrent_sync: Some(
+                "remote_storage_max_concurrent_sync_VALUE".to_string(),
             ),
         };
 
@@ -684,12 +703,13 @@ checkpoint_distance = 'checkpoint_distance_VALUE'
 checkpoint_period = 'checkpoint_period_VALUE'
 gc_horizon = 'gc_horizon_VALUE'
 gc_period = 'gc_period_VALUE'
+open_mem_limit = 'open_mem_limit_VALUE'
 pg_distrib_dir = 'pg_distrib_dir_VALUE'
 auth_validation_public_key_path = 'auth_validation_public_key_path_VALUE'
 auth_type = 'auth_type_VALUE'
-relish_storage_max_concurrent_sync = 'relish_storage_max_concurrent_sync_VALUE'
+remote_storage_max_concurrent_sync = 'remote_storage_max_concurrent_sync_VALUE'
 
-[relish_storage]
+[remote_storage]
 bucket_name = 'bucket_name_VALUE'
 bucket_region = 'bucket_region_VALUE'
 "#,
@@ -702,7 +722,7 @@ bucket_region = 'bucket_region_VALUE'
             .expect("Failed to deserialize the prettified serialization result of the config");
 
         let mut expected_params = params;
-        expected_params.relish_storage = Some(RelishStorage::AwsS3 {
+        expected_params.remote_storage = Some(RemoteStorage::AwsS3 {
             bucket_name: "bucket_name_VALUE".to_string(),
             bucket_region: "bucket_region_VALUE".to_string(),
             access_key_id: None,

pageserver/src/layered_repository.rs

@@ -32,7 +32,7 @@ use std::time::{Duration, Instant};
 
 use self::metadata::{metadata_path, TimelineMetadata};
 use crate::relish::*;
-use crate::relish_storage::schedule_timeline_upload;
+use crate::remote_storage::schedule_timeline_upload;
 use crate::repository::{GcResult, Repository, Timeline, TimelineWriter, WALRecord};
 use crate::tenant_mgr;
 use crate::walreceiver;
@@ -53,6 +53,7 @@ use zenith_utils::seqwait::SeqWait;
 mod blob;
 mod delta_layer;
 mod filename;
+mod global_layer_map;
 mod image_layer;
 mod inmemory_layer;
 mod interval_tree;
@@ -64,6 +65,7 @@ mod storage_layer;
 use delta_layer::DeltaLayer;
 use image_layer::ImageLayer;
 
+use global_layer_map::{LayerId, GLOBAL_LAYER_MAP};
 use inmemory_layer::InMemoryLayer;
 use layer_map::LayerMap;
 use storage_layer::{
@@ -798,6 +800,10 @@ impl Timeline for LayeredTimeline {
             _write_guard: self.write_lock.lock().unwrap(),
         })
     }
+
+    fn upgrade_to_layered_timeline(&self) -> &crate::layered_repository::LayeredTimeline {
+        self
+    }
 }
 
 impl LayeredTimeline {
@@ -1139,9 +1145,10 @@ impl LayeredTimeline {
         // a lot of memory and/or aren't receiving much updates anymore.
         let mut disk_consistent_lsn = last_record_lsn;
 
-        let mut created_historics = false;
         let mut layer_uploads = Vec::new();
-        while let Some((oldest_layer, oldest_generation)) = layers.peek_oldest_open() {
+        while let Some((oldest_layer_id, oldest_layer, oldest_generation)) =
+            layers.peek_oldest_open()
+        {
             let oldest_pending_lsn = oldest_layer.get_oldest_pending_lsn();
 
             // Does this layer need freezing?
@@ -1166,42 +1173,14 @@ impl LayeredTimeline {
                 break;
             }
 
-            // Mark the layer as no longer accepting writes and record the end_lsn.
-            // This happens in-place, no new layers are created now.
-            // We call `get_last_record_lsn` again, which may be different from the
-            // original load, as we may have released the write lock since then.
-            oldest_layer.freeze(self.get_last_record_lsn());
-
-            // The layer is no longer open, update the layer map to reflect this.
-            // We will replace it with on-disk historics below.
-            layers.pop_oldest_open();
-            layers.insert_historic(oldest_layer.clone());
-
-            // Write the now-frozen layer to disk. That could take a while, so release the lock while do it
             drop(layers);
             drop(write_guard);
 
-            let new_historics = oldest_layer.write_to_disk(self)?;
+            let mut this_layer_uploads = self.evict_layer(oldest_layer_id)?;
+            layer_uploads.append(&mut this_layer_uploads);
 
             write_guard = self.write_lock.lock().unwrap();
             layers = self.layers.lock().unwrap();
-
-            if !new_historics.is_empty() {
-                created_historics = true;
-            }
-
-            // Finally, replace the frozen in-memory layer with the new on-disk layers
-            layers.remove_historic(oldest_layer);
-
-            // Add the historics to the LayerMap
-            for delta_layer in new_historics.delta_layers {
-                layer_uploads.push(delta_layer.path());
-                layers.insert_historic(Arc::new(delta_layer));
-            }
-            for image_layer in new_historics.image_layers {
-                layer_uploads.push(image_layer.path());
-                layers.insert_historic(Arc::new(image_layer));
-            }
         }
 
         // Call unload() on all frozen layers, to release memory.
@@ -1214,7 +1193,7 @@ impl LayeredTimeline {
         drop(layers);
         drop(write_guard);
 
-        if created_historics {
+        if !layer_uploads.is_empty() {
             // We must fsync the timeline dir to ensure the directory entries for
             // new layer files are durable
             let timeline_dir =
@@ -1267,6 +1246,55 @@ impl LayeredTimeline {
         Ok(())
     }
 
+    fn evict_layer(&self, layer_id: LayerId) -> Result<Vec<PathBuf>> {
+        // Mark the layer as no longer accepting writes and record the end_lsn.
+        // This happens in-place, no new layers are created now.
+        // We call `get_last_record_lsn` again, which may be different from the
+        // original load, as we may have released the write lock since then.
+
+        let mut write_guard = self.write_lock.lock().unwrap();
+        let mut layers = self.layers.lock().unwrap();
+
+        let mut layer_uploads = Vec::new();
+
+        let global_layer_map = GLOBAL_LAYER_MAP.read().unwrap();
+        if let Some(oldest_layer) = global_layer_map.get(&layer_id) {
+            drop(global_layer_map);
+            oldest_layer.freeze(self.get_last_record_lsn());
+
+            // The layer is no longer open, update the layer map to reflect this.
+            // We will replace it with on-disk historics below.
+            layers.remove_open(layer_id);
+            layers.insert_historic(oldest_layer.clone());
+
+            // Write the now-frozen layer to disk. That could take a while, so release the lock while do it
+            drop(layers);
+            drop(write_guard);
+
+            let new_historics = oldest_layer.write_to_disk(self)?;
+
+            write_guard = self.write_lock.lock().unwrap();
+            layers = self.layers.lock().unwrap();
+
+            // Finally, replace the frozen in-memory layer with the new on-disk layers
+            layers.remove_historic(oldest_layer);
+
+            // Add the historics to the LayerMap
+            for delta_layer in new_historics.delta_layers {
+                layer_uploads.push(delta_layer.path());
+                layers.insert_historic(Arc::new(delta_layer));
+            }
+            for image_layer in new_historics.image_layers {
+                layer_uploads.push(image_layer.path());
+                layers.insert_historic(Arc::new(image_layer));
+            }
+        }
+        drop(layers);
+        drop(write_guard);
+
+        Ok(layer_uploads)
+    }
+
     ///
     /// Garbage collect layer files on a timeline that are no longer needed.
     ///
@@ -1506,6 +1534,15 @@ impl LayeredTimeline {
                 PageReconstructResult::Continue(cont_lsn) => {
                     // Fetch base image / more WAL from the returned predecessor layer
                     if let Some((cont_layer, cont_lsn)) = self.get_layer_for_read(seg, cont_lsn)? {
+                        if cont_lsn == curr_lsn {
+                            // We landed on the same layer again. Shouldn't happen, but if it does,
+                            // don't get stuck in an infinite loop.
+                            bail!(
+                                "could not find predecessor layer of segment {} at {}",
+                                seg.rel,
+                                cont_lsn
+                            );
+                        }
                         layer_arc = cont_layer;
                         layer_ref = &*layer_arc;
                         curr_lsn = cont_lsn;
@@ -1564,7 +1601,7 @@ impl LayeredTimeline {
                     rel,
                     request_lsn
                 );
-                Ok(img.clone())
+                Ok(Bytes::from(img.image().to_vec()))
             } else {
                 // FIXME: this ought to be an error?
                 warn!("Page {} blk {} at {} not found", rel, blknum, request_lsn);
@@ -1575,7 +1612,7 @@ impl LayeredTimeline {
             //
             // If we don't have a base image, then the oldest WAL record better initialize
             // the page
-            if data.page_img.is_none() && !data.records.first().unwrap().1.will_init {
+            if data.page_img.is_none() && !data.records.first().unwrap().1.will_init() {
                 // FIXME: this ought to be an error?
                 warn!(
                     "Base image for page {}/{} at {} not found, but got {} WAL records",
@@ -1595,7 +1632,7 @@ impl LayeredTimeline {
                     rel,
                     blknum,
                     request_lsn,
-                    data.page_img.clone(),
+                    data.page_img.map(|page| Bytes::from(page.image().to_vec())), // FIXME
                     data.records,
                 )?;
 
@@ -1834,3 +1871,32 @@ fn rename_to_backup(path: PathBuf) -> anyhow::Result<()> {
         path
     ))
 }
+
+//----- Global layer management
+
+/// Check if too much memory is being used by open layers. If so, evict
+pub fn evict_layer_if_needed(conf: &PageServerConf) -> Result<()> {
+    // Keep evicting layers until we are below the memory threshold.
+    let mut global_layer_map = GLOBAL_LAYER_MAP.read().unwrap();
+    while let Some((layer_id, layer)) = global_layer_map.find_victim_if_needed(conf.open_mem_limit)
+    {
+        drop(global_layer_map);
+        let tenantid = layer.get_tenant_id();
+        let timelineid = layer.get_timeline_id();
+
+        let _entered =
+            info_span!("global evict", timeline = %timelineid, tenant = %tenantid).entered();
+        info!("evicting {}", layer.filename().display());
+        drop(layer);
+
+        let timeline = tenant_mgr::get_timeline_for_tenant(tenantid, timelineid)?;
+
+        timeline
+            .upgrade_to_layered_timeline()
+            .evict_layer(layer_id)?;
+
+        global_layer_map = GLOBAL_LAYER_MAP.read().unwrap();
+    }
+
+    Ok(())
+}

pageserver/src/layered_repository/blob.rs

@@ -10,10 +10,10 @@ pub struct BlobRange {
     size: usize,
 }
 
-pub fn read_blob(reader: &BoundedReader<&'_ File>, range: &BlobRange) -> Result<Vec<u8>> {
+pub fn read_blob(reader: &BoundedReader<&'_ File>, range: &BlobRange) -> Result<Box<[u8]>> {
     let mut buf = vec![0u8; range.size];
     reader.read_exact_at(&mut buf, range.offset)?;
-    Ok(buf)
+    Ok(buf.into_boxed_slice())
 }
 
 pub struct BlobWriter<W> {

pageserver/src/layered_repository/delta_layer.rs

@@ -39,6 +39,7 @@
 //!
 use crate::layered_repository::blob::BlobWriter;
 use crate::layered_repository::filename::{DeltaFileName, PathOrConf};
+use crate::layered_repository::page_versions::PageVersions;
 use crate::layered_repository::storage_layer::{
     Layer, PageReconstructData, PageReconstructResult, PageVersion, SegmentTag,
 };
@@ -46,6 +47,7 @@ use crate::waldecoder;
 use crate::PageServerConf;
 use crate::{ZTenantId, ZTimelineId};
 use anyhow::{bail, ensure, Result};
+use bytes::Bytes;
 use log::*;
 use serde::{Deserialize, Serialize};
 use zenith_utils::vec_map::VecMap;
@@ -148,6 +150,10 @@ pub struct DeltaLayerInner {
 }
 
 impl Layer for DeltaLayer {
+    fn get_tenant_id(&self) -> ZTenantId {
+        self.tenantid
+    }
+
     fn get_timeline_id(&self) -> ZTimelineId {
         self.timelineid
     }
@@ -199,24 +205,25 @@ impl Layer for DeltaLayer {
                 .iter()
                 .rev();
             for ((_blknum, pv_lsn), blob_range) in iter {
-                let pv = PageVersion::des(&read_blob(&page_version_reader, blob_range)?)?;
+                let pv_bytes = read_blob(&page_version_reader, blob_range)?;
+                let pv = PageVersion::from_bytes(pv_bytes);
 
-                if let Some(img) = pv.page_image {
-                    // Found a page image, return it
-                    reconstruct_data.page_img = Some(img);
-                    need_image = false;
-                    break;
-                } else if let Some(rec) = pv.record {
-                    let will_init = rec.will_init;
-                    reconstruct_data.records.push((*pv_lsn, rec));
-                    if will_init {
-                        // This WAL record initializes the page, so no need to go further back
+                match pv {
+                    PageVersion::Page(img) => {
+                        // Found a page image, return it
+                        reconstruct_data.page_img = Some(img);
                         need_image = false;
                         break;
                     }
-                } else {
-                    // No base image, and no WAL record. Huh?
-                    bail!("no page image or WAL record for requested page");
+                    PageVersion::Wal(rec) => {
+                        let will_init = rec.will_init();
+                        reconstruct_data.records.push((*pv_lsn, rec));
+                        if will_init {
+                            // This WAL record initializes the page, so no need to go further back
+                            need_image = false;
+                            break;
+                        }
+                    }
                 }
             }
 
@@ -226,7 +233,7 @@ impl Layer for DeltaLayer {
         // If an older page image is needed to reconstruct the page, let the
         // caller know.
         if need_image {
-            Ok(PageReconstructResult::Continue(self.start_lsn))
+            Ok(PageReconstructResult::Continue(Lsn(self.start_lsn.0 - 1)))
         } else {
             Ok(PageReconstructResult::Complete)
         }
@@ -305,21 +312,25 @@ impl Layer for DeltaLayer {
             let mut desc = String::new();
 
             let buf = read_blob(&chapter, blob_range)?;
-            let pv = PageVersion::des(&buf)?;
+            let pv = PageVersion::from_bytes(buf);
 
-            if let Some(img) = pv.page_image.as_ref() {
-                write!(&mut desc, " img {} bytes", img.len())?;
-            }
-            if let Some(rec) = pv.record.as_ref() {
-                let wal_desc = waldecoder::describe_wal_record(&rec.rec);
-                write!(
-                    &mut desc,
-                    " rec {} bytes will_init: {} {}",
-                    rec.rec.len(),
-                    rec.will_init,
-                    wal_desc
-                )?;
+            match pv {
+                PageVersion::Page(page) => {
+                    write!(&mut desc, " img {} bytes", page.image().len())?;
+                }
+                PageVersion::Wal(rec) => {
+                    let wal_desc =
+                        waldecoder::describe_wal_record(&Bytes::from(rec.rec().to_vec()));
+                    write!(
+                        &mut desc,
+                        " rec {} bytes will_init: {} {}",
+                        rec.rec().len(),
+                        rec.will_init(),
+                        wal_desc
+                    )?;
+                }
             }
+
             println!("  blk {} at {}: {}", blk, lsn, desc);
         }
 
@@ -343,14 +354,14 @@ impl DeltaLayer {
     }
 
     /// Create a new delta file, using the given page versions and relsizes.
-    /// The page versions are passed by an iterator; the iterator must return
-    /// page versions in blknum+lsn order.
+    /// The page versions are passed in a PageVersions struct. If 'cutoff' is
+    /// given, only page versions with LSN < cutoff are included.
     ///
-    /// This is used to write the in-memory layer to disk. The in-memory layer uses the same
-    /// data structure with two btreemaps as we do, so passing the btreemaps is currently
-    /// expedient.
+    /// This is used to write the in-memory layer to disk. The page_versions and
+    /// relsizes are thus passed in the same format as they are in the in-memory
+    /// layer, as that's expedient.
     #[allow(clippy::too_many_arguments)]
-    pub fn create<'a>(
+    pub fn create(
         conf: &'static PageServerConf,
         timelineid: ZTimelineId,
         tenantid: ZTenantId,
@@ -358,7 +369,8 @@ impl DeltaLayer {
         start_lsn: Lsn,
         end_lsn: Lsn,
         dropped: bool,
-        page_versions: impl Iterator<Item = (u32, Lsn, &'a PageVersion)>,
+        page_versions: &PageVersions,
+        cutoff: Option<Lsn>,
         relsizes: VecMap<Lsn, u32>,
     ) -> Result<DeltaLayer> {
         if seg.rel.is_blocky() {
@@ -392,9 +404,10 @@ impl DeltaLayer {
 
         let mut page_version_writer = BlobWriter::new(book, PAGE_VERSIONS_CHAPTER);
 
-        for (blknum, lsn, page_version) in page_versions {
-            let buf = PageVersion::ser(page_version)?;
-            let blob_range = page_version_writer.write_blob(&buf)?;
+        let page_versions_iter = page_versions.ordered_page_version_iter(cutoff);
+        for (blknum, lsn, token) in page_versions_iter {
+            let blob_range = page_version_writer
+                .write_blob(page_versions.get_page_version_bytes(token).as_slice())?;
 
             inner
                 .page_version_metas

pageserver/src/layered_repository/global_layer_map.rs

@@ -0,0 +1,209 @@
+//!
+//! Global registry of open layers.
+//!
+//! Whenever a new in-memory layer is created to hold incoming WAL, it is registered
+//! in [`GLOBAL_LAYER_MAP`], so that we can keep track of the total number of
+//! in-memory layers in the system, and know when we need to evict some to release
+//! memory.
+//!
+//! Each layer is assigned a unique ID when it's registered in the global registry.
+//! The ID can be used to relocate the layer later, without having to hold locks.
+//!
+
+use std::sync::atomic::{AtomicU8, AtomicUsize, Ordering};
+use std::sync::{Arc, RwLock};
+
+use super::inmemory_layer::InMemoryLayer;
+
+use lazy_static::lazy_static;
+
+const MAX_USAGE_COUNT: u8 = 5;
+
+lazy_static! {
+    pub static ref GLOBAL_LAYER_MAP: RwLock<OpenLayers> = RwLock::new(OpenLayers::default());
+}
+
+///
+/// How much memory is being used by all the open layers? This is used to trigger
+/// freezing and evicting an open layer to disk.
+///
+/// This is only a rough approximation, it leaves out a lot of things like malloc()
+/// overhead. But as long there is enough "slop" and it's not set too close to the RAM
+/// size on the system, it's good enough.
+pub static GLOBAL_OPEN_MEM_USAGE: AtomicUsize = AtomicUsize::new(0);
+
+// TODO these types can probably be smaller
+#[derive(PartialEq, Eq, Clone, Copy)]
+pub struct LayerId {
+    index: usize,
+    tag: u64, // to avoid ABA problem
+}
+
+enum SlotData {
+    Occupied(Arc<InMemoryLayer>),
+    /// Vacant slots form a linked list, the value is the index
+    /// of the next vacant slot in the list.
+    Vacant(Option<usize>),
+}
+
+struct Slot {
+    tag: u64,
+    data: SlotData,
+    usage_count: AtomicU8, // for clock algorithm
+}
+
+#[derive(Default)]
+pub struct OpenLayers {
+    slots: Vec<Slot>,
+    num_occupied: usize,
+    next_victim: AtomicUsize,
+
+    // Head of free-slot list.
+    next_empty_slot_idx: Option<usize>,
+}
+
+impl OpenLayers {
+    pub fn insert(&mut self, layer: Arc<InMemoryLayer>) -> LayerId {
+        let slot_idx = match self.next_empty_slot_idx {
+            Some(slot_idx) => slot_idx,
+            None => {
+                let idx = self.slots.len();
+                self.slots.push(Slot {
+                    tag: 0,
+                    data: SlotData::Vacant(None),
+                    usage_count: AtomicU8::new(0),
+                });
+                idx
+            }
+        };
+        let slots_len = self.slots.len();
+
+        let slot = &mut self.slots[slot_idx];
+
+        match slot.data {
+            SlotData::Occupied(_) => {
+                panic!("an occupied slot was in the free list");
+            }
+            SlotData::Vacant(next_empty_slot_idx) => {
+                self.next_empty_slot_idx = next_empty_slot_idx;
+            }
+        }
+
+        slot.data = SlotData::Occupied(layer);
+        slot.usage_count.store(1, Ordering::Relaxed);
+
+        self.num_occupied += 1;
+        assert!(self.num_occupied <= slots_len);
+
+        LayerId {
+            index: slot_idx,
+            tag: slot.tag,
+        }
+    }
+
+    pub fn get(&self, layer_id: &LayerId) -> Option<Arc<InMemoryLayer>> {
+        let slot = self.slots.get(layer_id.index)?; // TODO should out of bounds indexes just panic?
+        if slot.tag != layer_id.tag {
+            return None;
+        }
+
+        if let SlotData::Occupied(layer) = &slot.data {
+            let _ = slot.usage_count.fetch_update(
+                Ordering::Relaxed,
+                Ordering::Relaxed,
+                |old_usage_count| {
+                    if old_usage_count < MAX_USAGE_COUNT {
+                        Some(old_usage_count + 1)
+                    } else {
+                        None
+                    }
+                },
+            );
+            Some(Arc::clone(layer))
+        } else {
+            None
+        }
+    }
+
+    /// Find a victim layer to evict, if the total memory usage of all open layers
+    /// is larger than 'limit'
+    pub fn find_victim_if_needed(&self, limit: usize) -> Option<(LayerId, Arc<InMemoryLayer>)> {
+        let mem_usage = GLOBAL_OPEN_MEM_USAGE.load(Ordering::Relaxed);
+
+        if mem_usage > limit {
+            self.find_victim()
+        } else {
+            None
+        }
+    }
+
+    pub fn find_victim(&self) -> Option<(LayerId, Arc<InMemoryLayer>)> {
+        if self.num_occupied == 0 {
+            return None;
+        }
+
+        // Run the clock algorithm.
+        //
+        // FIXME: It's theoretically possible that a constant stream of get() requests
+        // comes in faster than we advance the clock hand, so that this never finishes.
+        loop {
+            // FIXME: Because we interpret the clock hand variable modulo slots.len(), the
+            // hand effectively jumps to a more or less random place whenever the array is
+            // expanded. That's relatively harmless, it just leads to a non-optimal choice
+            // of victim. Also, in a server that runs for long enough, the array should reach
+            // a steady-state size and not grow anymore.
+            let next_victim = self.next_victim.fetch_add(1, Ordering::Relaxed) % self.slots.len();
+
+            let slot = &self.slots[next_victim];
+
+            if let SlotData::Occupied(data) = &slot.data {
+                fn update_fn(old_usage_count: u8) -> Option<u8> {
+                    if old_usage_count > 0 {
+                        Some(old_usage_count - 1)
+                    } else {
+                        None
+                    }
+                }
+
+                if slot
+                    .usage_count
+                    .fetch_update(Ordering::Relaxed, Ordering::Relaxed, update_fn)
+                    .is_err()
+                {
+                    // Found a slot with usage_count == 0. Return it.
+                    return Some((
+                        LayerId {
+                            index: next_victim,
+                            tag: slot.tag,
+                        },
+                        Arc::clone(data),
+                    ));
+                }
+            }
+        }
+    }
+
+    // TODO this won't be a public API in the future
+    pub fn remove(&mut self, layer_id: &LayerId) {
+        let slot = &mut self.slots[layer_id.index];
+
+        if slot.tag != layer_id.tag {
+            return;
+        }
+
+        match &slot.data {
+            SlotData::Occupied(_layer) => {
+                // TODO evict the layer
+            }
+            SlotData::Vacant(_) => unimplemented!(),
+        }
+
+        slot.data = SlotData::Vacant(self.next_empty_slot_idx);
+        self.next_empty_slot_idx = Some(layer_id.index);
+
+        assert!(self.num_occupied > 0);
+        self.num_occupied -= 1;
+
+        slot.tag = slot.tag.wrapping_add(1);
+    }
+}

pageserver/src/layered_repository/image_layer.rs

@@ -23,7 +23,7 @@
 //!
 use crate::layered_repository::filename::{ImageFileName, PathOrConf};
 use crate::layered_repository::storage_layer::{
-    Layer, PageReconstructData, PageReconstructResult, SegmentTag,
+    Layer, Page, PageReconstructData, PageReconstructResult, SegmentTag,
 };
 use crate::layered_repository::LayeredTimeline;
 use crate::layered_repository::RELISH_SEG_SIZE;
@@ -117,6 +117,10 @@ impl Layer for ImageLayer {
         PathBuf::from(self.layer_name().to_string())
     }
 
+    fn get_tenant_id(&self) -> ZTenantId {
+        self.tenantid
+    }
+
     fn get_timeline_id(&self) -> ZTimelineId {
         self.timelineid
     }
@@ -173,7 +177,7 @@ impl Layer for ImageLayer {
             }
         };
 
-        reconstruct_data.page_img = Some(Bytes::from(buf));
+        reconstruct_data.page_img = Some(Page::from_bytes(&buf));
         Ok(PageReconstructResult::Complete)
     }
 

pageserver/src/layered_repository/inmemory_layer.rs

@@ -3,25 +3,27 @@
 //! are held in a BTreeMap, and there's another BTreeMap to track the size of the relation.
 //!
 use crate::layered_repository::filename::DeltaFileName;
+use crate::layered_repository::global_layer_map::GLOBAL_OPEN_MEM_USAGE;
 use crate::layered_repository::storage_layer::{
     Layer, PageReconstructData, PageReconstructResult, PageVersion, SegmentTag, RELISH_SEG_SIZE,
 };
 use crate::layered_repository::LayeredTimeline;
-use crate::layered_repository::ZERO_PAGE;
 use crate::layered_repository::{DeltaLayer, ImageLayer};
 use crate::repository::WALRecord;
 use crate::PageServerConf;
 use crate::{ZTenantId, ZTimelineId};
-use anyhow::{bail, ensure, Result};
+use anyhow::{ensure, Result};
 use bytes::Bytes;
 use log::*;
 use std::path::PathBuf;
+use std::sync::atomic::Ordering;
 use std::sync::{Arc, RwLock};
 use zenith_utils::vec_map::VecMap;
 
 use zenith_utils::lsn::Lsn;
 
 use super::page_versions::PageVersions;
+use super::storage_layer::Page;
 
 pub struct InMemoryLayer {
     conf: &'static PageServerConf,
@@ -69,6 +71,15 @@ pub struct InMemoryLayerInner {
     /// a non-blocky rel, 'segsizes' is not used and is always empty.
     ///
     segsizes: VecMap<Lsn, u32>,
+
+    /// Approximate amount of memory used by this layer.
+    ///
+    /// TODO: This is currently a very crude metric, we don't take into account allocator
+    /// overhead, memory fragmentation, memory used by the VecMaps, nor many other things.
+    /// Just the actual # of bytes of a page image (8 kB) or the size of a WAL record.
+    ///
+    /// Whenever this is changed, you must also modify GLOBAL_OPEN_MEM_USAGE accordingly!
+    mem_usage: usize,
 }
 
 impl InMemoryLayerInner {
@@ -89,6 +100,15 @@ impl InMemoryLayerInner {
     }
 }
 
+impl Drop for InMemoryLayerInner {
+    fn drop(&mut self) {
+        if self.mem_usage > 0 {
+            GLOBAL_OPEN_MEM_USAGE.fetch_sub(self.mem_usage, Ordering::Relaxed);
+            self.mem_usage = 0;
+        }
+    }
+}
+
 impl Layer for InMemoryLayer {
     // An in-memory layer doesn't really have a filename as it's not stored on disk,
     // but we construct a filename as if it was a delta layer
@@ -113,6 +133,10 @@ impl Layer for InMemoryLayer {
         PathBuf::from(format!("inmem-{}", delta_filename))
     }
 
+    fn get_tenant_id(&self) -> ZTenantId {
+        self.tenantid
+    }
+
     fn get_timeline_id(&self) -> ZTimelineId {
         self.timelineid
     }
@@ -160,21 +184,22 @@ impl Layer for InMemoryLayer {
                 .get_block_lsn_range(blknum, ..=lsn)
                 .iter()
                 .rev();
-            for (entry_lsn, entry) in iter {
-                if let Some(img) = &entry.page_image {
-                    reconstruct_data.page_img = Some(img.clone());
-                    need_image = false;
-                    break;
-                } else if let Some(rec) = &entry.record {
-                    reconstruct_data.records.push((*entry_lsn, rec.clone()));
-                    if rec.will_init {
-                        // This WAL record initializes the page, so no need to go further back
+            for (entry_lsn, token) in iter {
+                match inner.page_versions.get_page_version(token)? {
+                    PageVersion::Page(page) => {
+                        reconstruct_data.page_img = Some(page);
                         need_image = false;
                         break;
                     }
-                } else {
-                    // No base image, and no WAL record. Huh?
-                    bail!("no page image or WAL record for requested page");
+                    PageVersion::Wal(rec) => {
+                        let will_init = rec.will_init();
+                        reconstruct_data.records.push((*entry_lsn, rec));
+                        if will_init {
+                            // This WAL record initializes the page, so no need to go further back
+                            need_image = false;
+                            break;
+                        }
+                    }
                 }
             }
             // release lock on 'inner'
@@ -261,14 +286,13 @@ impl Layer for InMemoryLayer {
             println!("segsizes {}: {}", k, v);
         }
 
-        for (blknum, lsn, pv) in inner.page_versions.ordered_page_version_iter(None) {
-            println!(
-                "blk {} at {}: {}/{}\n",
-                blknum,
-                lsn,
-                pv.page_image.is_some(),
-                pv.record.is_some()
-            );
+        for (blknum, lsn, token) in inner.page_versions.ordered_page_version_iter(None) {
+            let pv_description = match inner.page_versions.get_page_version(token)? {
+                PageVersion::Page(_img) => "page",
+                PageVersion::Wal(_rec) => "wal",
+            };
+
+            println!("blk {} at {}: {}\n", blknum, lsn, pv_description);
         }
 
         Ok(())
@@ -281,12 +305,6 @@ pub struct LayersOnDisk {
     pub image_layers: Vec<ImageLayer>,
 }
 
-impl LayersOnDisk {
-    pub fn is_empty(&self) -> bool {
-        self.delta_layers.is_empty() && self.image_layers.is_empty()
-    }
-}
-
 impl InMemoryLayer {
     /// Return the oldest page version that's stored in this layer
     pub fn get_oldest_pending_lsn(&self) -> Lsn {
@@ -330,6 +348,7 @@ impl InMemoryLayer {
                 dropped: false,
                 page_versions: PageVersions::default(),
                 segsizes,
+                mem_usage: 0,
             }),
         })
     }
@@ -338,26 +357,12 @@ impl InMemoryLayer {
 
     /// Remember new page version, as a WAL record over previous version
     pub fn put_wal_record(&self, lsn: Lsn, blknum: u32, rec: WALRecord) -> u32 {
-        self.put_page_version(
-            blknum,
-            lsn,
-            PageVersion {
-                page_image: None,
-                record: Some(rec),
-            },
-        )
+        self.put_page_version(blknum, lsn, PageVersion::Wal(rec))
     }
 
     /// Remember new page version, as a full page image
     pub fn put_page_image(&self, blknum: u32, lsn: Lsn, img: Bytes) -> u32 {
-        self.put_page_version(
-            blknum,
-            lsn,
-            PageVersion {
-                page_image: Some(img),
-                record: None,
-            },
-        )
+        self.put_page_version(blknum, lsn, PageVersion::Page(Page::from_bytes(&img)))
     }
 
     /// Common subroutine of the public put_wal_record() and put_page_image() functions.
@@ -376,6 +381,12 @@ impl InMemoryLayer {
 
         inner.assert_writeable();
 
+        let mut mem_usage = 0;
+        mem_usage += match &pv {
+            PageVersion::Page(page) => page.image().len(),
+            PageVersion::Wal(rec) => rec.rec().len(),
+        };
+
         let old = inner.page_versions.append_or_update_last(blknum, lsn, pv);
 
         if old.is_some() {
@@ -386,6 +397,9 @@ impl InMemoryLayer {
             );
         }
 
+        inner.mem_usage += mem_usage;
+        GLOBAL_OPEN_MEM_USAGE.fetch_add(mem_usage, Ordering::Relaxed);
+
         // Also update the relation size, if this extended the relation.
         if self.seg.rel.is_blocky() {
             let newsize = blknum - self.seg.segno * RELISH_SEG_SIZE + 1;
@@ -412,10 +426,7 @@ impl InMemoryLayer {
                 // subsequent call to initialize the gap page.
                 let gapstart = self.seg.segno * RELISH_SEG_SIZE + oldsize;
                 for gapblknum in gapstart..blknum {
-                    let zeropv = PageVersion {
-                        page_image: Some(ZERO_PAGE.clone()),
-                        record: None,
-                    };
+                    let zeropv = PageVersion::Page(Page::zero_page());
                     trace!(
                         "filling gap blk {} with zeros for write of {}",
                         gapblknum,
@@ -521,6 +532,7 @@ impl InMemoryLayer {
                 dropped: false,
                 page_versions: PageVersions::default(),
                 segsizes,
+                mem_usage: 0,
             }),
         })
     }
@@ -550,6 +562,16 @@ impl InMemoryLayer {
             for (_blk, lsn, _pv) in inner.page_versions.ordered_page_version_iter(None) {
                 assert!(lsn <= end_lsn);
             }
+
+            // It's a bit premature to subtract the global mem usage here already.
+            // This layer consumes memory until it's written out to disk and dropped.
+            // But GLOBAL_OPEN_MEM_USAGE is used to trigger layer eviction, if there are
+            // too many open layers, and from that point of view this should no longer be
+            // counted against the global mem usage.
+            if inner.mem_usage > 0 {
+                GLOBAL_OPEN_MEM_USAGE.fetch_sub(inner.mem_usage, Ordering::Relaxed);
+                inner.mem_usage = 0;
+            }
         }
     }
 
@@ -589,7 +611,8 @@ impl InMemoryLayer {
                 self.start_lsn,
                 end_lsn_exclusive,
                 true,
-                inner.page_versions.ordered_page_version_iter(None),
+                &inner.page_versions,
+                None,
                 inner.segsizes.clone(),
             )?;
             trace!(
@@ -606,13 +629,9 @@ impl InMemoryLayer {
 
         // Since `end_lsn` is inclusive, subtract 1.
         // We want to make an ImageLayer for the last included LSN,
-        // so the DeltaLayer should exlcude that LSN.
+        // so the DeltaLayer should exclude that LSN.
         let end_lsn_inclusive = Lsn(end_lsn_exclusive.0 - 1);
 
-        let mut page_versions = inner
-            .page_versions
-            .ordered_page_version_iter(Some(end_lsn_inclusive));
-
         let mut delta_layers = Vec::new();
 
         if self.start_lsn != end_lsn_inclusive {
@@ -626,7 +645,8 @@ impl InMemoryLayer {
                 self.start_lsn,
                 end_lsn_inclusive,
                 false,
-                page_versions,
+                &inner.page_versions,
+                Some(end_lsn_inclusive),
                 segsizes,
             )?;
             delta_layers.push(delta_layer);
@@ -637,7 +657,11 @@ impl InMemoryLayer {
                 end_lsn_inclusive
             );
         } else {
-            assert!(page_versions.next().is_none());
+            assert!(inner
+                .page_versions
+                .ordered_page_version_iter(None)
+                .next()
+                .is_none());
         }
 
         drop(inner);

pageserver/src/layered_repository/layer_map.rs

@@ -17,11 +17,12 @@ use anyhow::Result;
 use lazy_static::lazy_static;
 use std::cmp::Ordering;
 use std::collections::{BinaryHeap, HashMap};
-use std::sync::atomic::{self, AtomicU64};
 use std::sync::Arc;
 use zenith_metrics::{register_int_gauge, IntGauge};
 use zenith_utils::lsn::Lsn;
 
+use super::global_layer_map::{LayerId, GLOBAL_LAYER_MAP};
+
 lazy_static! {
     static ref NUM_INMEMORY_LAYERS: IntGauge =
         register_int_gauge!("pageserver_inmemory_layers", "Number of layers in memory")
@@ -31,17 +32,6 @@ lazy_static! {
             .expect("failed to define a metric");
 }
 
-static NEXT_LAYER_ID: AtomicU64 = AtomicU64::new(0);
-
-#[derive(PartialEq, Eq, Hash, Clone, Copy)]
-pub struct LayerId(u64);
-
-impl LayerId {
-    fn next() -> LayerId {
-        Self(NEXT_LAYER_ID.fetch_add(1, atomic::Ordering::Relaxed))
-    }
-}
-
 ///
 /// LayerMap tracks what layers exist on a timeline.
 ///
@@ -55,8 +45,6 @@ pub struct LayerMap {
     /// contains the oldest WAL record.
     open_layers: BinaryHeap<OpenLayerEntry>,
 
-    open_layers_by_id: HashMap<LayerId, Arc<InMemoryLayer>>,
-
     /// Generation number, used to distinguish newly inserted entries in the
     /// binary heap from older entries during checkpoint.
     current_generation: u64,
@@ -82,21 +70,18 @@ impl LayerMap {
     pub fn get_open(&self, tag: &SegmentTag) -> Option<Arc<InMemoryLayer>> {
         let segentry = self.segs.get(tag)?;
 
-        segentry.open.as_ref().map(Arc::clone)
-    }
-
-    #[allow(dead_code)]
-    pub fn get_open_by_id(&self, layer_id: &LayerId) -> Option<Arc<InMemoryLayer>> {
-        self.open_layers_by_id.get(layer_id).cloned()
+        segentry
+            .open_layer_id
+            .and_then(|layer_id| GLOBAL_LAYER_MAP.read().unwrap().get(&layer_id))
     }
 
     ///
     /// Insert an open in-memory layer
     ///
-    pub fn insert_open(&mut self, layer: Arc<InMemoryLayer>) -> LayerId {
+    pub fn insert_open(&mut self, layer: Arc<InMemoryLayer>) {
         let segentry = self.segs.entry(layer.get_seg_tag()).or_default();
 
-        segentry.update_open(Arc::clone(&layer));
+        let layer_id = segentry.update_open(Arc::clone(&layer));
 
         let oldest_pending_lsn = layer.get_oldest_pending_lsn();
 
@@ -105,45 +90,46 @@ impl LayerMap {
         // in the middle of a WAL record.
         assert!(oldest_pending_lsn.is_aligned());
 
-        let id = LayerId::next();
-
         // Also add it to the binary heap
         let open_layer_entry = OpenLayerEntry {
             oldest_pending_lsn: layer.get_oldest_pending_lsn(),
-            layer: Arc::clone(&layer),
+            layer_id,
             generation: self.current_generation,
-            id,
         };
         self.open_layers.push(open_layer_entry);
 
-        let old_layer = self.open_layers_by_id.insert(id, layer);
-        assert!(old_layer.is_none());
-
         NUM_INMEMORY_LAYERS.inc();
-
-        id
     }
 
-    /// Remove the oldest in-memory layer
-    pub fn pop_oldest_open(&mut self) {
-        // Pop it from the binary heap
-        let oldest_entry = self.open_layers.pop().unwrap();
-        let segtag = oldest_entry.layer.get_seg_tag();
+    /// Remove an open in-memory layer
+    pub fn remove_open(&mut self, layer_id: LayerId) {
+        // Note: we don't try to remove the entry from the binary heap.
+        // It will be removed lazily by peek_oldest_open() when it's made it to
+        // the top of the heap.
 
-        // Also remove it from the SegEntry of this segment
-        let mut segentry = self.segs.get_mut(&segtag).unwrap();
-        if Arc::ptr_eq(segentry.open.as_ref().unwrap(), &oldest_entry.layer) {
-            segentry.open = None;
-        } else {
-            // We could have already updated segentry.open for
-            // dropped (non-writeable) layer. This is fine.
-            assert!(!oldest_entry.layer.is_writeable());
-            assert!(oldest_entry.layer.is_dropped());
+        let layer_opt = {
+            let mut global_map = GLOBAL_LAYER_MAP.write().unwrap();
+            let layer_opt = global_map.get(&layer_id);
+            global_map.remove(&layer_id);
+            // TODO it's bad that a ref can still exist after being evicted from cache
+            layer_opt
+        };
+
+        if let Some(layer) = layer_opt {
+            let mut segentry = self.segs.get_mut(&layer.get_seg_tag()).unwrap();
+
+            if segentry.open_layer_id == Some(layer_id) {
+                // Also remove it from the SegEntry of this segment
+                segentry.open_layer_id = None;
+            } else {
+                // We could have already updated segentry.open for
+                // dropped (non-writeable) layer. This is fine.
+                assert!(!layer.is_writeable());
+                assert!(layer.is_dropped());
+            }
+
+            NUM_INMEMORY_LAYERS.dec();
         }
-
-        self.open_layers_by_id.remove(&oldest_entry.id).unwrap();
-
-        NUM_INMEMORY_LAYERS.dec();
     }
 
     ///
@@ -228,10 +214,17 @@ impl LayerMap {
     }
 
     /// Return the oldest in-memory layer, along with its generation number.
-    pub fn peek_oldest_open(&self) -> Option<(Arc<InMemoryLayer>, u64)> {
-        self.open_layers
-            .peek()
-            .map(|oldest_entry| (Arc::clone(&oldest_entry.layer), oldest_entry.generation))
+    pub fn peek_oldest_open(&mut self) -> Option<(LayerId, Arc<InMemoryLayer>, u64)> {
+        let global_map = GLOBAL_LAYER_MAP.read().unwrap();
+
+        while let Some(oldest_entry) = self.open_layers.peek() {
+            if let Some(layer) = global_map.get(&oldest_entry.layer_id) {
+                return Some((oldest_entry.layer_id, layer, oldest_entry.generation));
+            } else {
+                self.open_layers.pop();
+            }
+        }
+        None
     }
 
     /// Increment the generation number used to stamp open in-memory layers. Layers
@@ -254,8 +247,12 @@ impl LayerMap {
     pub fn dump(&self) -> Result<()> {
         println!("Begin dump LayerMap");
         for (seg, segentry) in self.segs.iter() {
-            if let Some(open) = &segentry.open {
-                open.dump()?;
+            if let Some(open) = &segentry.open_layer_id {
+                if let Some(layer) = GLOBAL_LAYER_MAP.read().unwrap().get(open) {
+                    layer.dump()?;
+                } else {
+                    println!("layer not found in global map");
+                }
             }
 
             for layer in segentry.historic.iter() {
@@ -288,7 +285,7 @@ impl IntervalItem for dyn Layer {
 /// IntervalTree.
 #[derive(Default)]
 struct SegEntry {
-    open: Option<Arc<InMemoryLayer>>,
+    open_layer_id: Option<LayerId>,
     historic: IntervalTree<dyn Layer>,
 }
 
@@ -304,10 +301,10 @@ impl SegEntry {
     }
 
     pub fn get(&self, lsn: Lsn) -> Option<Arc<dyn Layer>> {
-        if let Some(open) = &self.open {
-            if open.get_start_lsn() <= lsn {
-                let x: Arc<dyn Layer> = Arc::clone(open) as _;
-                return Some(x);
+        if let Some(open_layer_id) = &self.open_layer_id {
+            let open_layer = GLOBAL_LAYER_MAP.read().unwrap().get(open_layer_id)?;
+            if open_layer.get_start_lsn() <= lsn {
+                return Some(open_layer);
             }
         }
 
@@ -326,11 +323,16 @@ impl SegEntry {
     // Set new open layer for a SegEntry.
     // It's ok to rewrite previous open layer,
     // but only if it is not writeable anymore.
-    pub fn update_open(&mut self, layer: Arc<InMemoryLayer>) {
-        if let Some(prev_open) = &self.open {
-            assert!(!prev_open.is_writeable());
+    pub fn update_open(&mut self, layer: Arc<InMemoryLayer>) -> LayerId {
+        if let Some(prev_open_layer_id) = &self.open_layer_id {
+            if let Some(prev_open_layer) = GLOBAL_LAYER_MAP.read().unwrap().get(prev_open_layer_id)
+            {
+                assert!(!prev_open_layer.is_writeable());
+            }
         }
-        self.open = Some(layer);
+        let open_layer_id = GLOBAL_LAYER_MAP.write().unwrap().insert(layer);
+        self.open_layer_id = Some(open_layer_id);
+        open_layer_id
     }
 
     pub fn insert_historic(&mut self, layer: Arc<dyn Layer>) {
@@ -345,10 +347,9 @@ impl SegEntry {
 /// recently-added entries (i.e after last call to increment_generation()) from older
 /// entries with the same 'oldest_pending_lsn'.
 struct OpenLayerEntry {
-    pub oldest_pending_lsn: Lsn, // copy of layer.get_oldest_pending_lsn()
-    pub generation: u64,
-    pub layer: Arc<InMemoryLayer>,
-    id: LayerId,
+    oldest_pending_lsn: Lsn, // copy of layer.get_oldest_pending_lsn()
+    generation: u64,
+    layer_id: LayerId,
 }
 impl Ord for OpenLayerEntry {
     fn cmp(&self, other: &Self) -> Ordering {
@@ -456,10 +457,10 @@ mod tests {
         // A helper function (closure) to pop the next oldest open entry from the layer map,
         // and assert that it is what we'd expect
         let mut assert_pop_layer = |expected_segno: u32, expected_generation: u64| {
-            let (l, generation) = layers.peek_oldest_open().unwrap();
+            let (layer_id, l, generation) = layers.peek_oldest_open().unwrap();
             assert!(l.get_seg_tag().segno == expected_segno);
             assert!(generation == expected_generation);
-            layers.pop_oldest_open();
+            layers.remove_open(layer_id);
         };
 
         assert_pop_layer(0, gen1); // 0x100

pageserver/src/layered_repository/metadata.rs

@@ -1,7 +1,7 @@
 //! Every image of a certain timeline from [`crate::layered_repository::LayeredRepository`]
 //! has a metadata that needs to be stored persistently.
 //!
-//! Later, the file gets is used in [`crate::relish_storage::storage_sync`] as a part of
+//! Later, the file gets is used in [`crate::remote_storage::storage_sync`] as a part of
 //! external storage import and export operations.
 //!
 //! The module contains all structs and related helper methods related to timeline metadata.

pageserver/src/layered_repository/page_versions.rs

@@ -1,13 +1,23 @@
 use std::{collections::HashMap, ops::RangeBounds, slice};
 
+use anyhow::Result;
+
+use zenith_utils::chunked_buffer::{ChunkToken, ChunkedBuffer};
 use zenith_utils::{lsn::Lsn, vec_map::VecMap};
 
 use super::storage_layer::PageVersion;
 
-const EMPTY_SLICE: &[(Lsn, PageVersion)] = &[];
+const EMPTY_SLICE: &[(Lsn, ChunkToken)] = &[];
 
 #[derive(Debug, Default)]
-pub struct PageVersions(HashMap<u32, VecMap<Lsn, PageVersion>>);
+pub struct PageVersions {
+    map: HashMap<u32, VecMap<Lsn, ChunkToken>>,
+
+    /// The PageVersion structs are stored in a serialized format in this buffer.
+    /// Each serialized PageVersion is preceded by a 'u32' length field.
+    /// The 'map' stores offsets into this buffer.
+    buffer: ChunkedBuffer,
+}
 
 impl PageVersions {
     pub fn append_or_update_last(
@@ -15,14 +25,16 @@ impl PageVersions {
         blknum: u32,
         lsn: Lsn,
         page_version: PageVersion,
-    ) -> Option<PageVersion> {
-        let map = self.0.entry(blknum).or_insert_with(VecMap::default);
-        map.append_or_update_last(lsn, page_version).unwrap()
+    ) -> Option<ChunkToken> {
+        let token = self.buffer.write(page_version.bytes());
+
+        let map = self.map.entry(blknum).or_insert_with(VecMap::default);
+        map.append_or_update_last(lsn, token).unwrap()
     }
 
     /// Get all [`PageVersion`]s in a block
-    pub fn get_block_slice(&self, blknum: u32) -> &[(Lsn, PageVersion)] {
-        self.0
+    fn get_block_slice(&self, blknum: u32) -> &[(Lsn, ChunkToken)] {
+        self.map
             .get(&blknum)
             .map(VecMap::as_slice)
             .unwrap_or(EMPTY_SLICE)
@@ -33,8 +45,8 @@ impl PageVersions {
         &self,
         blknum: u32,
         range: R,
-    ) -> &[(Lsn, PageVersion)] {
-        self.0
+    ) -> &[(Lsn, ChunkToken)] {
+        self.map
             .get(&blknum)
             .map(|vec_map| vec_map.slice_range(range))
             .unwrap_or(EMPTY_SLICE)
@@ -43,7 +55,7 @@ impl PageVersions {
     /// Iterate through [`PageVersion`]s in (block, lsn) order.
     /// If a [`cutoff_lsn`] is set, only show versions with `lsn < cutoff_lsn`
     pub fn ordered_page_version_iter(&self, cutoff_lsn: Option<Lsn>) -> OrderedPageVersionIter<'_> {
-        let mut ordered_blocks: Vec<u32> = self.0.keys().cloned().collect();
+        let mut ordered_blocks: Vec<u32> = self.map.keys().cloned().collect();
         ordered_blocks.sort_unstable();
 
         let slice = ordered_blocks
@@ -59,6 +71,15 @@ impl PageVersions {
             cur_slice_iter: slice.iter(),
         }
     }
+
+    pub fn get_page_version_bytes(&self, token: &ChunkToken) -> Vec<u8> {
+        self.buffer.read(token)
+    }
+
+    pub fn get_page_version(&self, token: &ChunkToken) -> Result<PageVersion> {
+        let buf = self.get_page_version_bytes(token).into_boxed_slice();
+        Ok(PageVersion::from_bytes(buf))
+    }
 }
 
 pub struct OrderedPageVersionIter<'a> {
@@ -69,7 +90,7 @@ pub struct OrderedPageVersionIter<'a> {
 
     cutoff_lsn: Option<Lsn>,
 
-    cur_slice_iter: slice::Iter<'a, (Lsn, PageVersion)>,
+    cur_slice_iter: slice::Iter<'a, (Lsn, ChunkToken)>,
 }
 
 impl OrderedPageVersionIter<'_> {
@@ -83,14 +104,14 @@ impl OrderedPageVersionIter<'_> {
 }
 
 impl<'a> Iterator for OrderedPageVersionIter<'a> {
-    type Item = (u32, Lsn, &'a PageVersion);
+    type Item = (u32, Lsn, &'a ChunkToken);
 
     fn next(&mut self) -> Option<Self::Item> {
         loop {
-            if let Some((lsn, page_version)) = self.cur_slice_iter.next() {
+            if let Some((lsn, token)) = self.cur_slice_iter.next() {
                 if self.is_lsn_before_cutoff(lsn) {
                     let blknum = self.ordered_blocks[self.cur_block_idx];
-                    return Some((blknum, *lsn, page_version));
+                    return Some((blknum, *lsn, token));
                 }
             }
 
@@ -104,12 +125,9 @@ impl<'a> Iterator for OrderedPageVersionIter<'a> {
 
 #[cfg(test)]
 mod tests {
-    use super::*;
+    use crate::layered_repository::storage_layer::Page;
 
-    const EMPTY_PAGE_VERSION: PageVersion = PageVersion {
-        page_image: None,
-        record: None,
-    };
+    use super::*;
 
     #[test]
     fn test_ordered_iter() {
@@ -117,9 +135,15 @@ mod tests {
         const BLOCKS: u32 = 1000;
         const LSNS: u64 = 50;
 
+        let empty_page_version = PageVersion::Page(Page::zero_page());
+
         for blknum in 0..BLOCKS {
             for lsn in 0..LSNS {
-                let old = page_versions.append_or_update_last(blknum, Lsn(lsn), EMPTY_PAGE_VERSION);
+                let old = page_versions.append_or_update_last(
+                    blknum,
+                    Lsn(lsn),
+                    empty_page_version.clone(),
+                );
                 assert!(old.is_none());
             }
         }

pageserver/src/layered_repository/storage_layer.rs

@@ -3,10 +3,9 @@
 //!
 
 use crate::relish::RelishTag;
-use crate::repository::WALRecord;
-use crate::ZTimelineId;
+use crate::repository::{WALRecord, WAL_BIT};
+use crate::{ZTenantId, ZTimelineId};
 use anyhow::Result;
-use bytes::Bytes;
 use serde::{Deserialize, Serialize};
 use std::fmt;
 use std::path::PathBuf;
@@ -51,23 +50,50 @@ impl SegmentTag {
 ///
 /// A page version can be stored as a full page image, or as WAL record that needs
 /// to be applied over the previous page version to reconstruct this version.
-///
-/// It's also possible to have both a WAL record and a page image in the same
-/// PageVersion. That happens if page version is originally stored as a WAL record
-/// but it is later reconstructed by a GetPage@LSN request by performing WAL
-/// redo. The get_page_at_lsn() code will store the reconstructed pag image next to
-/// the WAL record in that case. TODO: That's pretty accidental, not the result
-/// of any grand design. If we want to keep reconstructed page versions around, we
-/// probably should have a separate buffer cache so that we could control the
-/// replacement policy globally. Or if we keep a reconstructed page image, we
-/// could throw away the WAL record.
-///
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct PageVersion {
-    /// an 8kb page image
-    pub page_image: Option<Bytes>,
-    /// WAL record to get from previous page version to this one.
-    pub record: Option<WALRecord>,
+#[derive(Debug, Clone)]
+pub enum PageVersion {
+    Page(Page),
+    Wal(WALRecord),
+}
+
+impl PageVersion {
+    pub fn from_bytes(bytes: Box<[u8]>) -> Self {
+        if bytes[0] & WAL_BIT != 0 {
+            Self::Wal(WALRecord::from_bytes(bytes))
+        } else {
+            Self::Page(Page(bytes))
+        }
+    }
+
+    pub fn bytes(&self) -> &[u8] {
+        match self {
+            Self::Page(page) => page.bytes(),
+            Self::Wal(wal) => wal.bytes(),
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct Page(Box<[u8]>);
+
+impl Page {
+    pub fn zero_page() -> Self {
+        // TODO optimize this
+        Self(vec![0u8; 1 + 8192].into_boxed_slice())
+    }
+    pub fn from_bytes(bytes: &[u8]) -> Self {
+        let mut buf = vec![0u8; 1 + bytes.len()];
+        buf[1..].copy_from_slice(bytes);
+        Self(buf.into_boxed_slice())
+    }
+
+    pub fn image(&self) -> &[u8] {
+        &self.0[1..]
+    }
+
+    pub fn bytes(&self) -> &[u8] {
+        &self.0
+    }
 }
 
 ///
@@ -79,7 +105,7 @@ pub struct PageVersion {
 ///
 pub struct PageReconstructData {
     pub records: Vec<(Lsn, WALRecord)>,
-    pub page_img: Option<Bytes>,
+    pub page_img: Option<Page>,
 }
 
 /// Return value from Layer::get_page_reconstruct_data
@@ -104,6 +130,8 @@ pub enum PageReconstructResult {
 /// in-memory and on-disk layers.
 ///
 pub trait Layer: Send + Sync {
+    fn get_tenant_id(&self) -> ZTenantId;
+
     /// Identify the timeline this relish belongs to
     fn get_timeline_id(&self) -> ZTimelineId;
 

pageserver/src/lib.rs

@@ -14,7 +14,7 @@ pub mod http;
 pub mod layered_repository;
 pub mod page_service;
 pub mod relish;
-pub mod relish_storage;
+pub mod remote_storage;
 pub mod repository;
 pub mod restore_local_repo;
 pub mod tenant_mgr;
@@ -42,7 +42,9 @@ pub mod defaults {
     pub const DEFAULT_GC_PERIOD: Duration = Duration::from_secs(100);
 
     pub const DEFAULT_SUPERUSER: &str = "zenith_admin";
-    pub const DEFAULT_RELISH_STORAGE_MAX_CONCURRENT_SYNC_LIMITS: usize = 100;
+    pub const DEFAULT_REMOTE_STORAGE_MAX_CONCURRENT_SYNC_LIMITS: usize = 100;
+
+    pub const DEFAULT_OPEN_MEM_LIMIT: usize = 128 * 1024 * 1024;
 }
 
 lazy_static! {
@@ -71,6 +73,8 @@ pub struct PageServerConf {
     pub gc_period: Duration,
     pub superuser: String,
 
+    pub open_mem_limit: usize,
+
     // Repository directory, relative to current working directory.
     // Normally, the page server changes the current working directory
     // to the repository, and 'workdir' is always '.'. But we don't do
@@ -84,7 +88,7 @@ pub struct PageServerConf {
     pub auth_type: AuthType,
 
     pub auth_validation_public_key_path: Option<PathBuf>,
-    pub relish_storage_config: Option<RelishStorageConfig>,
+    pub remote_storage_config: Option<RemoteStorageConfig>,
 }
 
 impl PageServerConf {
@@ -153,6 +157,7 @@ impl PageServerConf {
             checkpoint_period: Duration::from_secs(10),
             gc_horizon: defaults::DEFAULT_GC_HORIZON,
             gc_period: Duration::from_secs(10),
+            open_mem_limit: defaults::DEFAULT_OPEN_MEM_LIMIT,
             listen_pg_addr: defaults::DEFAULT_PG_LISTEN_ADDR.to_string(),
             listen_http_addr: defaults::DEFAULT_HTTP_LISTEN_ADDR.to_string(),
             superuser: "zenith_admin".to_string(),
@@ -160,7 +165,7 @@ impl PageServerConf {
             pg_distrib_dir: "".into(),
             auth_type: AuthType::Trust,
             auth_validation_public_key_path: None,
-            relish_storage_config: None,
+            remote_storage_config: None,
         }
     }
 }
@@ -174,18 +179,18 @@ pub enum CheckpointConfig {
     Forced,
 }
 
-/// External relish storage configuration, enough for creating a client for that storage.
+/// External backup storage configuration, enough for creating a client for that storage.
 #[derive(Debug, Clone)]
-pub struct RelishStorageConfig {
-    /// Limits the number of concurrent sync operations between pageserver and relish storage.
+pub struct RemoteStorageConfig {
+    /// Limits the number of concurrent sync operations between pageserver and the remote storage.
     pub max_concurrent_sync: usize,
     /// The storage connection configuration.
-    pub storage: RelishStorageKind,
+    pub storage: RemoteStorageKind,
 }
 
-/// A kind of a relish storage to connect to, with its connection configuration.
+/// A kind of a remote storage to connect to, with its connection configuration.
 #[derive(Debug, Clone)]
-pub enum RelishStorageKind {
+pub enum RemoteStorageKind {
     /// Storage based on local file system.
     /// Specify a root folder to place all stored relish data into.
     LocalFs(PathBuf),

pageserver/src/remote_storage.rs

@@ -3,7 +3,7 @@
 //! No other modules from this tree are supposed to be used directly by the external code.
 //!
 //! There are a few components the storage machinery consists of:
-//! * [`RelishStorage`] trait a CRUD-like generic abstraction to use for adapting external storages with a few implementations:
+//! * [`RemoteStorage`] trait a CRUD-like generic abstraction to use for adapting external storages with a few implementations:
 //!     * [`local_fs`] allows to use local file system as an external storage
 //!     * [`rust_s3`] uses AWS S3 bucket entirely as an external storage
 //!
@@ -11,7 +11,7 @@
 //!
 //! * public API via to interact with the external world: [`run_storage_sync_thread`] and [`schedule_timeline_upload`]
 //!
-//! Here's a schematic overview of all interactions relish storage and the rest of the pageserver perform:
+//! Here's a schematic overview of all interactions backup and the rest of the pageserver perform:
 //!
 //! +------------------------+                                    +--------->-------+
 //! |                        |  - - - (init async loop) - - - ->  |                 |
@@ -29,7 +29,7 @@
 //!                                                                         V
 //!                                                            +------------------------+
 //!                                                            |                        |
-//!                                                            | [`RelishStorage`] impl |
+//!                                                            | [`RemoteStorage`] impl |
 //!                                                            |                        |
 //!                                                            | pageserver assumes it  |
 //!                                                            | owns exclusive write   |
@@ -56,7 +56,7 @@
 //! When the pageserver terminates, the upload loop finishes a current image sync task (if any) and exits.
 //!
 //! NOTES:
-//! * pageserver assumes it has exclusive write access to the relish storage. If supported, the way multiple pageservers can be separated in the same storage
+//! * pageserver assumes it has exclusive write access to the remote storage. If supported, the way multiple pageservers can be separated in the same storage
 //! (i.e. using different directories in the local filesystem external storage), but totally up to the storage implementation and not covered with the trait API.
 //!
 //! * the uploads do not happen right after pageserver startup, they are registered when
@@ -65,7 +65,7 @@
 //!
 //! * the uploads do not happen right after the upload registration: the sync loop might be occupied with other tasks, or tasks with bigger priority could be waiting already
 //!
-//! * all synchronization tasks (including the public API to register uploads and downloads and the sync queue management) happens on an image scale: a big set of relish files,
+//! * all synchronization tasks (including the public API to register uploads and downloads and the sync queue management) happens on an image scale: a big set of remote files,
 //! enough to represent (and recover, if needed) a certain timeline state. On the contrary, all internal storage CRUD calls are made per reilsh file from those images.
 //! This way, the synchronization is able to download the image partially, if some state was synced before, but exposes correctly synced images only.
 
@@ -79,13 +79,14 @@ use std::{
 };
 
 use anyhow::{anyhow, ensure, Context};
+use tokio::io;
 use zenith_utils::zid::{ZTenantId, ZTimelineId};
 
 pub use self::storage_sync::schedule_timeline_upload;
 use self::{local_fs::LocalFs, rust_s3::S3};
 use crate::{
     layered_repository::{TENANTS_SEGMENT_NAME, TIMELINES_SEGMENT_NAME},
-    PageServerConf, RelishStorageKind,
+    PageServerConf, RemoteStorageKind,
 };
 
 /// Based on the config, initiates the remote storage connection and starts a separate thread
@@ -94,16 +95,16 @@ use crate::{
 pub fn run_storage_sync_thread(
     config: &'static PageServerConf,
 ) -> anyhow::Result<Option<thread::JoinHandle<anyhow::Result<()>>>> {
-    match &config.relish_storage_config {
-        Some(relish_storage_config) => {
-            let max_concurrent_sync = relish_storage_config.max_concurrent_sync;
-            let handle = match &relish_storage_config.storage {
-                RelishStorageKind::LocalFs(root) => storage_sync::spawn_storage_sync_thread(
+    match &config.remote_storage_config {
+        Some(storage_config) => {
+            let max_concurrent_sync = storage_config.max_concurrent_sync;
+            let handle = match &storage_config.storage {
+                RemoteStorageKind::LocalFs(root) => storage_sync::spawn_storage_sync_thread(
                     config,
                     LocalFs::new(root.clone(), &config.workdir)?,
                     max_concurrent_sync,
                 ),
-                RelishStorageKind::AwsS3(s3_config) => storage_sync::spawn_storage_sync_thread(
+                RemoteStorageKind::AwsS3(s3_config) => storage_sync::spawn_storage_sync_thread(
                     config,
                     S3::new(s3_config, &config.workdir)?,
                     max_concurrent_sync,
@@ -119,41 +120,39 @@ pub fn run_storage_sync_thread(
 /// This storage tries to be unaware of any layered repository context,
 /// providing basic CRUD operations with storage files.
 #[async_trait::async_trait]
-trait RelishStorage: Send + Sync {
-    /// A way to uniquely reference relish in the remote storage.
-    type RelishStoragePath;
+trait RemoteStorage: Send + Sync {
+    /// A way to uniquely reference a file in the remote storage.
+    type StoragePath;
 
     /// Attempts to derive the storage path out of the local path, if the latter is correct.
-    fn storage_path(&self, local_path: &Path) -> anyhow::Result<Self::RelishStoragePath>;
+    fn storage_path(&self, local_path: &Path) -> anyhow::Result<Self::StoragePath>;
 
     /// Gets the layered storage information about the given entry.
-    fn info(&self, storage_path: &Self::RelishStoragePath) -> anyhow::Result<RemoteRelishInfo>;
+    fn info(&self, storage_path: &Self::StoragePath) -> anyhow::Result<RemoteFileInfo>;
 
     /// Lists all items the storage has right now.
-    async fn list_relishes(&self) -> anyhow::Result<Vec<Self::RelishStoragePath>>;
-
-    /// Streams the remote storage entry contents into the buffered writer given, returns the filled writer.
-    async fn download_relish<W: 'static + std::io::Write + Send>(
-        &self,
-        from: &Self::RelishStoragePath,
-        // rust_s3 `get_object_stream` method requires `std::io::BufWriter` for some reason, not the async counterpart
-        // that forces us to consume and return the writer to satisfy the blocking operation async wrapper requirements
-        to: std::io::BufWriter<W>,
-    ) -> anyhow::Result<std::io::BufWriter<W>>;
-
-    async fn delete_relish(&self, path: &Self::RelishStoragePath) -> anyhow::Result<()>;
+    async fn list(&self) -> anyhow::Result<Vec<Self::StoragePath>>;
 
     /// Streams the local file contents into remote into the remote storage entry.
-    async fn upload_relish<R: tokio::io::AsyncRead + std::marker::Unpin + Send>(
+    async fn upload(
         &self,
-        from: &mut tokio::io::BufReader<R>,
-        to: &Self::RelishStoragePath,
+        from: impl io::AsyncRead + Unpin + Send + Sync + 'static,
+        to: &Self::StoragePath,
     ) -> anyhow::Result<()>;
+
+    /// Streams the remote storage entry contents into the buffered writer given, returns the filled writer.
+    async fn download(
+        &self,
+        from: &Self::StoragePath,
+        to: &mut (impl io::AsyncWrite + Unpin + Send + Sync),
+    ) -> anyhow::Result<()>;
+
+    async fn delete(&self, path: &Self::StoragePath) -> anyhow::Result<()>;
 }
 
 /// Information about a certain remote storage entry.
 #[derive(Debug, PartialEq, Eq)]
-struct RemoteRelishInfo {
+struct RemoteFileInfo {
     tenant_id: ZTenantId,
     timeline_id: ZTimelineId,
     /// Path in the pageserver workdir where the file should go to.

pageserver/src/remote_storage/README.md

@@ -1,7 +1,7 @@
 # Non-implementation details
 
 This document describes the current state of the backup system in pageserver, existing limitations and concerns, why some things are done the way they are the future development plans.
-Detailed description on how the synchronization works and how it fits into the rest of the pageserver can be found in the [storage module](./../relish_storage.rs) and its submodules.
+Detailed description on how the synchronization works and how it fits into the rest of the pageserver can be found in the [storage module](./../remote_storage.rs) and its submodules.
 Ideally, this document should disappear after current implementation concerns are mitigated, with the remaining useful knowledge bits moved into rustdocs.
 
 ## Approach
@@ -28,7 +28,7 @@ As mentioned, the backup component is rather new and under development currently
 Here's the list of known compromises with comments:
 
 * Remote storage model is the same as the `tenants/` directory contents of the pageserver's local workdir storage.
-This is relatively simple to implement, but may be costly to use in AWS S3: an initial data image contains ~782 relish file and a metadata file, ~31 MB combined.
+This is relatively simple to implement, but may be costly to use in AWS S3: an initial data image contains ~782 relish files and a metadata file, ~31 MB combined.
 AWS charges per API call and for traffic either, layers are expected to be updated frequently, so this model most probably is ineffective.
 Additionally, pageservers might need to migrate images between tenants, which does not improve the situation.
 

pageserver/src/remote_storage/local_fs.rs

@@ -1,4 +1,4 @@
-//! Local filesystem relish storage.
+//! Local filesystem acting as a remote storage.
 //! Multiple pageservers can use the same "storage" of this kind by using different storage roots.
 //!
 //! This storage used in pageserver tests, but can also be used in cases when a certain persistent
@@ -7,18 +7,20 @@
 use std::{
     ffi::OsStr,
     future::Future,
-    io::Write,
     path::{Path, PathBuf},
     pin::Pin,
 };
 
 use anyhow::{bail, Context};
-use tokio::{fs, io};
+use tokio::{
+    fs,
+    io::{self, AsyncWriteExt},
+};
 use tracing::*;
 
 use crate::layered_repository::metadata::METADATA_FILE_NAME;
 
-use super::{parse_ids_from_path, strip_path_prefix, RelishStorage, RemoteRelishInfo};
+use super::{parse_ids_from_path, strip_path_prefix, RemoteFileInfo, RemoteStorage};
 
 pub struct LocalFs {
     pageserver_workdir: &'static Path,
@@ -26,7 +28,7 @@ pub struct LocalFs {
 }
 
 impl LocalFs {
-    /// Attempts to create local FS relish storage, along with the storage root directory.
+    /// Attempts to create local FS storage, along with its root directory.
     pub fn new(root: PathBuf, pageserver_workdir: &'static Path) -> anyhow::Result<Self> {
         if !root.exists() {
             std::fs::create_dir_all(&root).with_context(|| {
@@ -57,17 +59,17 @@ impl LocalFs {
 }
 
 #[async_trait::async_trait]
-impl RelishStorage for LocalFs {
-    type RelishStoragePath = PathBuf;
+impl RemoteStorage for LocalFs {
+    type StoragePath = PathBuf;
 
-    fn storage_path(&self, local_path: &Path) -> anyhow::Result<Self::RelishStoragePath> {
+    fn storage_path(&self, local_path: &Path) -> anyhow::Result<Self::StoragePath> {
         Ok(self.root.join(
             strip_path_prefix(self.pageserver_workdir, local_path)
                 .context("local path does not belong to this storage")?,
         ))
     }
 
-    fn info(&self, storage_path: &Self::RelishStoragePath) -> anyhow::Result<RemoteRelishInfo> {
+    fn info(&self, storage_path: &Self::StoragePath) -> anyhow::Result<RemoteFileInfo> {
         let is_metadata =
             storage_path.file_name().and_then(OsStr::to_str) == Some(METADATA_FILE_NAME);
         let relative_path = strip_path_prefix(&self.root, storage_path)
@@ -77,7 +79,7 @@ impl RelishStorage for LocalFs {
             relative_path.iter().filter_map(|segment| segment.to_str()),
             &relative_path.display(),
         )?;
-        Ok(RemoteRelishInfo {
+        Ok(RemoteFileInfo {
             tenant_id,
             timeline_id,
             download_destination,
@@ -85,62 +87,14 @@ impl RelishStorage for LocalFs {
         })
     }
 
-    async fn list_relishes(&self) -> anyhow::Result<Vec<Self::RelishStoragePath>> {
+    async fn list(&self) -> anyhow::Result<Vec<Self::StoragePath>> {
         Ok(get_all_files(&self.root).await?.into_iter().collect())
     }
 
-    async fn download_relish<W: 'static + std::io::Write + Send>(
+    async fn upload(
         &self,
-        from: &Self::RelishStoragePath,
-        mut to: std::io::BufWriter<W>,
-    ) -> anyhow::Result<std::io::BufWriter<W>> {
-        let file_path = self.resolve_in_storage(from)?;
-
-        if file_path.exists() && file_path.is_file() {
-            let updated_buffer = tokio::task::spawn_blocking(move || {
-                let mut source = std::io::BufReader::new(
-                    std::fs::OpenOptions::new()
-                        .read(true)
-                        .open(&file_path)
-                        .with_context(|| {
-                            format!(
-                                "Failed to open source file '{}' to use in the download",
-                                file_path.display()
-                            )
-                        })?,
-                );
-                std::io::copy(&mut source, &mut to)
-                    .context("Failed to download the relish file")?;
-                to.flush().context("Failed to flush the download buffer")?;
-                Ok::<_, anyhow::Error>(to)
-            })
-            .await
-            .context("Failed to spawn a blocking task")??;
-            Ok(updated_buffer)
-        } else {
-            bail!(
-                "File '{}' either does not exist or is not a file",
-                file_path.display()
-            )
-        }
-    }
-
-    async fn delete_relish(&self, path: &Self::RelishStoragePath) -> anyhow::Result<()> {
-        let file_path = self.resolve_in_storage(path)?;
-        if file_path.exists() && file_path.is_file() {
-            Ok(fs::remove_file(file_path).await?)
-        } else {
-            bail!(
-                "File '{}' either does not exist or is not a file",
-                file_path.display()
-            )
-        }
-    }
-
-    async fn upload_relish<R: io::AsyncRead + std::marker::Unpin + Send>(
-        &self,
-        from: &mut io::BufReader<R>,
-        to: &Self::RelishStoragePath,
+        mut from: impl io::AsyncRead + Unpin + Send + Sync + 'static,
+        to: &Self::StoragePath,
     ) -> anyhow::Result<()> {
         let target_file_path = self.resolve_in_storage(to)?;
         create_target_directory(&target_file_path).await?;
@@ -158,11 +112,59 @@ impl RelishStorage for LocalFs {
                 })?,
         );
 
-        io::copy_buf(from, &mut destination)
+        io::copy(&mut from, &mut destination)
             .await
-            .context("Failed to upload relish to local storage")?;
+            .context("Failed to upload a file to the local storage")?;
+        destination
+            .flush()
+            .await
+            .context("Failed to upload a file to the local storage")?;
         Ok(())
     }
+
+    async fn download(
+        &self,
+        from: &Self::StoragePath,
+        to: &mut (impl io::AsyncWrite + Unpin + Send + Sync),
+    ) -> anyhow::Result<()> {
+        let file_path = self.resolve_in_storage(from)?;
+
+        if file_path.exists() && file_path.is_file() {
+            let mut source = io::BufReader::new(
+                fs::OpenOptions::new()
+                    .read(true)
+                    .open(&file_path)
+                    .await
+                    .with_context(|| {
+                        format!(
+                            "Failed to open source file '{}' to use in the download",
+                            file_path.display()
+                        )
+                    })?,
+            );
+            io::copy(&mut source, to)
+                .await
+                .context("Failed to download a file from the local storage")?;
+            Ok(())
+        } else {
+            bail!(
+                "File '{}' either does not exist or is not a file",
+                file_path.display()
+            )
+        }
+    }
+
+    async fn delete(&self, path: &Self::StoragePath) -> anyhow::Result<()> {
+        let file_path = self.resolve_in_storage(path)?;
+        if file_path.exists() && file_path.is_file() {
+            Ok(fs::remove_file(file_path).await?)
+        } else {
+            bail!(
+                "File '{}' either does not exist or is not a file",
+                file_path.display()
+            )
+        }
+    }
 }
 
 fn get_all_files<'a, P>(
@@ -202,7 +204,7 @@ async fn create_target_directory(target_file_path: &Path) -> anyhow::Result<()>
     let target_dir = match target_file_path.parent() {
         Some(parent_dir) => parent_dir,
         None => bail!(
-            "Relish path '{}' has no parent directory",
+            "File path '{}' has no parent directory",
             target_file_path.display()
         ),
     };
@@ -216,7 +218,7 @@ async fn create_target_directory(target_file_path: &Path) -> anyhow::Result<()>
 mod pure_tests {
     use crate::{
         layered_repository::metadata::METADATA_FILE_NAME,
-        relish_storage::test_utils::{
+        remote_storage::test_utils::{
             custom_tenant_id_path, custom_timeline_id_path, relative_timeline_path,
         },
         repository::repo_harness::{RepoHarness, TIMELINE_ID},
@@ -233,13 +235,13 @@ mod pure_tests {
             root: storage_root.clone(),
         };
 
-        let local_path = repo_harness.timeline_path(&TIMELINE_ID).join("relish_name");
+        let local_path = repo_harness.timeline_path(&TIMELINE_ID).join("file_name");
         let expected_path = storage_root.join(local_path.strip_prefix(&repo_harness.conf.workdir)?);
 
         assert_eq!(
             expected_path,
             storage.storage_path(&local_path).expect("Matching path should map to storage path normally"),
-            "Relish paths from pageserver workdir should be stored in local fs storage with the same path they have relative to the workdir"
+            "File paths from pageserver workdir should be stored in local fs storage with the same path they have relative to the workdir"
         );
 
         Ok(())
@@ -297,7 +299,7 @@ mod pure_tests {
         let name = "not a metadata";
         let local_path = repo_harness.timeline_path(&TIMELINE_ID).join(name);
         assert_eq!(
-            RemoteRelishInfo {
+            RemoteFileInfo {
                 tenant_id: repo_harness.tenant_id,
                 timeline_id: TIMELINE_ID,
                 download_destination: local_path.clone(),
@@ -306,7 +308,7 @@ mod pure_tests {
             storage
                 .info(&storage_root.join(local_path.strip_prefix(&repo_harness.conf.workdir)?))
                 .expect("For a valid input, valid S3 info should be parsed"),
-            "Should be able to parse metadata out of the correctly named remote delta relish"
+            "Should be able to parse metadata out of the correctly named remote delta file"
         );
 
         let local_metadata_path = repo_harness
@@ -314,7 +316,7 @@ mod pure_tests {
             .join(METADATA_FILE_NAME);
         let remote_metadata_path = storage.storage_path(&local_metadata_path)?;
         assert_eq!(
-            RemoteRelishInfo {
+            RemoteFileInfo {
                 tenant_id: repo_harness.tenant_id,
                 timeline_id: TIMELINE_ID,
                 download_destination: local_metadata_path,
@@ -336,7 +338,7 @@ mod pure_tests {
         fn storage_info_error(storage: &LocalFs, storage_path: &PathBuf) -> String {
             match storage.info(storage_path) {
                 Ok(wrong_info) => panic!(
-                    "Expected storage path input {:?} to cause an error, but got relish info: {:?}",
+                    "Expected storage path input {:?} to cause an error, but got file info: {:?}",
                     storage_path, wrong_info,
                 ),
                 Err(e) => format!("{:?}", e),
@@ -356,24 +358,23 @@ mod pure_tests {
 
         let relative_timeline_path = relative_timeline_path(&repo_harness)?;
 
-        let relative_relish_path =
-            custom_tenant_id_path(&relative_timeline_path, "wrong_tenant_id")?
-                .join("wrong_tenant_id_name");
-        let wrong_tenant_id_path = storage_root.join(&relative_relish_path);
+        let relative_file_path = custom_tenant_id_path(&relative_timeline_path, "wrong_tenant_id")?
+            .join("wrong_tenant_id_name");
+        let wrong_tenant_id_path = storage_root.join(&relative_file_path);
         let error_message = storage_info_error(&storage, &wrong_tenant_id_path);
         assert!(
-            error_message.contains(relative_relish_path.to_str().unwrap()),
+            error_message.contains(relative_file_path.to_str().unwrap()),
             "Error message '{}' does not contain the expected substring",
             error_message
         );
 
-        let relative_relish_path =
+        let relative_file_path =
             custom_timeline_id_path(&relative_timeline_path, "wrong_timeline_id")?
                 .join("wrong_timeline_id_name");
-        let wrong_timeline_id_path = storage_root.join(&relative_relish_path);
+        let wrong_timeline_id_path = storage_root.join(&relative_file_path);
         let error_message = storage_info_error(&storage, &wrong_timeline_id_path);
         assert!(
-            error_message.contains(relative_relish_path.to_str().unwrap()),
+            error_message.contains(relative_file_path.to_str().unwrap()),
             "Error message '{}' does not contain the expected substring",
             error_message
         );
@@ -406,26 +407,26 @@ mod pure_tests {
 
 #[cfg(test)]
 mod fs_tests {
+    use super::*;
     use crate::{
-        relish_storage::test_utils::relative_timeline_path, repository::repo_harness::RepoHarness,
+        remote_storage::test_utils::relative_timeline_path, repository::repo_harness::RepoHarness,
     };
 
-    use super::*;
-
+    use std::io::Write;
     use tempfile::tempdir;
 
     #[tokio::test]
-    async fn upload_relish() -> anyhow::Result<()> {
-        let repo_harness = RepoHarness::create("upload_relish")?;
+    async fn upload_file() -> anyhow::Result<()> {
+        let repo_harness = RepoHarness::create("upload_file")?;
         let storage = create_storage()?;
 
-        let mut source = create_file_for_upload(
+        let source = create_file_for_upload(
             &storage.pageserver_workdir.join("whatever"),
             "whatever_contents",
         )
         .await?;
         let target_path = PathBuf::from("/").join("somewhere").join("else");
-        match storage.upload_relish(&mut source, &target_path).await {
+        match storage.upload(source, &target_path).await {
             Ok(()) => panic!("Should not allow storing files with wrong target path"),
             Err(e) => {
                 let message = format!("{:?}", e);
@@ -433,23 +434,23 @@ mod fs_tests {
                 assert!(message.contains("does not belong to the current storage"));
             }
         }
-        assert!(storage.list_relishes().await?.is_empty());
+        assert!(storage.list().await?.is_empty());
 
         let target_path_1 = upload_dummy_file(&repo_harness, &storage, "upload_1").await?;
         assert_eq!(
-            storage.list_relishes().await?,
+            storage.list().await?,
             vec![target_path_1.clone()],
             "Should list a single file after first upload"
         );
 
         let target_path_2 = upload_dummy_file(&repo_harness, &storage, "upload_2").await?;
         assert_eq!(
-            list_relishes_sorted(&storage).await?,
+            list_files_sorted(&storage).await?,
             vec![target_path_1.clone(), target_path_2.clone()],
             "Should list a two different files after second upload"
         );
 
-        // match storage.upload_relish(&mut source, &target_path_1).await {
+        // match storage.upload_file(&mut source, &target_path_1).await {
         //     Ok(()) => panic!("Should not allow reuploading storage files"),
         //     Err(e) => {
         //         let message = format!("{:?}", e);
@@ -458,7 +459,7 @@ mod fs_tests {
         //     }
         // }
         assert_eq!(
-            list_relishes_sorted(&storage).await?,
+            list_files_sorted(&storage).await?,
             vec![target_path_1, target_path_2],
             "Should list a two different files after all upload attempts"
         );
@@ -473,17 +474,17 @@ mod fs_tests {
     }
 
     #[tokio::test]
-    async fn download_relish() -> anyhow::Result<()> {
-        let repo_harness = RepoHarness::create("download_relish")?;
+    async fn download_file() -> anyhow::Result<()> {
+        let repo_harness = RepoHarness::create("download_file")?;
         let storage = create_storage()?;
         let upload_name = "upload_1";
         let upload_target = upload_dummy_file(&repo_harness, &storage, upload_name).await?;
 
-        let contents_bytes = storage
-            .download_relish(&upload_target, std::io::BufWriter::new(Vec::new()))
-            .await?
-            .into_inner()?;
-        let contents = String::from_utf8(contents_bytes)?;
+        let mut content_bytes = io::BufWriter::new(std::io::Cursor::new(Vec::new()));
+        storage.download(&upload_target, &mut content_bytes).await?;
+        content_bytes.flush().await?;
+
+        let contents = String::from_utf8(content_bytes.into_inner().into_inner())?;
         assert_eq!(
             dummy_contents(upload_name),
             contents,
@@ -491,10 +492,7 @@ mod fs_tests {
         );
 
         let non_existing_path = PathBuf::from("somewhere").join("else");
-        match storage
-            .download_relish(&non_existing_path, std::io::BufWriter::new(Vec::new()))
-            .await
-        {
+        match storage.download(&non_existing_path, &mut io::sink()).await {
             Ok(_) => panic!("Should not allow downloading non-existing storage files"),
             Err(e) => {
                 let error_string = e.to_string();
@@ -506,16 +504,16 @@ mod fs_tests {
     }
 
     #[tokio::test]
-    async fn delete_relish() -> anyhow::Result<()> {
-        let repo_harness = RepoHarness::create("delete_relish")?;
+    async fn delete_file() -> anyhow::Result<()> {
+        let repo_harness = RepoHarness::create("delete_file")?;
         let storage = create_storage()?;
         let upload_name = "upload_1";
         let upload_target = upload_dummy_file(&repo_harness, &storage, upload_name).await?;
 
-        storage.delete_relish(&upload_target).await?;
-        assert!(storage.list_relishes().await?.is_empty());
+        storage.delete(&upload_target).await?;
+        assert!(storage.list().await?.is_empty());
 
-        match storage.delete_relish(&upload_target).await {
+        match storage.delete(&upload_target).await {
             Ok(()) => panic!("Should not allow deleting non-existing storage files"),
             Err(e) => {
                 let error_string = e.to_string();
@@ -536,8 +534,8 @@ mod fs_tests {
             .join(relative_timeline_path(harness)?)
             .join(name);
         storage
-            .upload_relish(
-                &mut create_file_for_upload(
+            .upload(
+                create_file_for_upload(
                     &storage.pageserver_workdir.join(name),
                     &dummy_contents(name),
                 )
@@ -568,9 +566,9 @@ mod fs_tests {
         format!("contents for {}", name)
     }
 
-    async fn list_relishes_sorted(storage: &LocalFs) -> anyhow::Result<Vec<PathBuf>> {
-        let mut relishes = storage.list_relishes().await?;
-        relishes.sort();
-        Ok(relishes)
+    async fn list_files_sorted(storage: &LocalFs) -> anyhow::Result<Vec<PathBuf>> {
+        let mut files = storage.list().await?;
+        files.sort();
+        Ok(files)
     }
 }

pageserver/src/remote_storage/rust_s3.rs

@@ -1,18 +1,16 @@
-//! AWS S3 relish storage wrapper around `rust_s3` library.
-//! Currently does not allow multiple pageservers to use the same bucket concurrently: relishes are
+//! AWS S3 storage wrapper around `rust_s3` library.
+//! Currently does not allow multiple pageservers to use the same bucket concurrently: objects are
 //! placed in the root of the bucket.
 
-use std::{
-    io::Write,
-    path::{Path, PathBuf},
-};
+use std::path::{Path, PathBuf};
 
 use anyhow::Context;
 use s3::{bucket::Bucket, creds::Credentials, region::Region};
+use tokio::io::{self, AsyncWriteExt};
 
 use crate::{
     layered_repository::metadata::METADATA_FILE_NAME,
-    relish_storage::{parse_ids_from_path, strip_path_prefix, RelishStorage, RemoteRelishInfo},
+    remote_storage::{parse_ids_from_path, strip_path_prefix, RemoteFileInfo, RemoteStorage},
     S3Config,
 };
 
@@ -31,14 +29,14 @@ impl S3ObjectKey {
     }
 }
 
-/// AWS S3 relish storage.
+/// AWS S3 storage.
 pub struct S3 {
     pageserver_workdir: &'static Path,
     bucket: Bucket,
 }
 
 impl S3 {
-    /// Creates the relish storage, errors if incorrect AWS S3 configuration provided.
+    /// Creates the storage, errors if incorrect AWS S3 configuration provided.
     pub fn new(aws_config: &S3Config, pageserver_workdir: &'static Path) -> anyhow::Result<Self> {
         let region = aws_config
             .bucket_region
@@ -65,10 +63,10 @@ impl S3 {
 }
 
 #[async_trait::async_trait]
-impl RelishStorage for S3 {
-    type RelishStoragePath = S3ObjectKey;
+impl RemoteStorage for S3 {
+    type StoragePath = S3ObjectKey;
 
-    fn storage_path(&self, local_path: &Path) -> anyhow::Result<Self::RelishStoragePath> {
+    fn storage_path(&self, local_path: &Path) -> anyhow::Result<Self::StoragePath> {
         let relative_path = strip_path_prefix(self.pageserver_workdir, local_path)?;
         let mut key = String::new();
         for segment in relative_path {
@@ -78,14 +76,14 @@ impl RelishStorage for S3 {
         Ok(S3ObjectKey(key))
     }
 
-    fn info(&self, storage_path: &Self::RelishStoragePath) -> anyhow::Result<RemoteRelishInfo> {
+    fn info(&self, storage_path: &Self::StoragePath) -> anyhow::Result<RemoteFileInfo> {
         let storage_path_key = &storage_path.0;
         let is_metadata =
             storage_path_key.ends_with(&format!("{}{}", S3_FILE_SEPARATOR, METADATA_FILE_NAME));
         let download_destination = storage_path.download_destination(self.pageserver_workdir);
         let (tenant_id, timeline_id) =
             parse_ids_from_path(storage_path_key.split(S3_FILE_SEPARATOR), storage_path_key)?;
-        Ok(RemoteRelishInfo {
+        Ok(RemoteFileInfo {
             tenant_id,
             timeline_id,
             download_destination,
@@ -93,7 +91,7 @@ impl RelishStorage for S3 {
         })
     }
 
-    async fn list_relishes(&self) -> anyhow::Result<Vec<Self::RelishStoragePath>> {
+    async fn list(&self) -> anyhow::Result<Vec<Self::StoragePath>> {
         let list_response = self
             .bucket
             .list(String::new(), None)
@@ -107,14 +105,45 @@ impl RelishStorage for S3 {
             .collect())
     }
 
-    async fn download_relish<W: 'static + std::io::Write + Send>(
+    async fn upload(
         &self,
-        from: &Self::RelishStoragePath,
-        mut to: std::io::BufWriter<W>,
-    ) -> anyhow::Result<std::io::BufWriter<W>> {
-        let code = self
+        mut from: impl io::AsyncRead + Unpin + Send + Sync + 'static,
+        to: &Self::StoragePath,
+    ) -> anyhow::Result<()> {
+        let mut upload_contents = io::BufWriter::new(std::io::Cursor::new(Vec::new()));
+        io::copy(&mut from, &mut upload_contents)
+            .await
+            .context("Failed to read the upload contents")?;
+        upload_contents
+            .flush()
+            .await
+            .context("Failed to read the upload contents")?;
+        let upload_contents = upload_contents.into_inner().into_inner();
+
+        let (_, code) = self
             .bucket
-            .get_object_stream(from.key(), &mut to)
+            .put_object(to.key(), &upload_contents)
+            .await
+            .with_context(|| format!("Failed to create s3 object with key {}", to.key()))?;
+        if code != 200 {
+            Err(anyhow::format_err!(
+                "Received non-200 exit code during creating object with key '{}', code: {}",
+                to.key(),
+                code
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
+    async fn download(
+        &self,
+        from: &Self::StoragePath,
+        to: &mut (impl io::AsyncWrite + Unpin + Send + Sync),
+    ) -> anyhow::Result<()> {
+        let (data, code) = self
+            .bucket
+            .get_object(from.key())
             .await
             .with_context(|| format!("Failed to download s3 object with key {}", from.key()))?;
         if code != 200 {
@@ -123,16 +152,14 @@ impl RelishStorage for S3 {
                 code
             ))
         } else {
-            tokio::task::spawn_blocking(move || {
-                to.flush().context("Failed to flush the download buffer")?;
-                Ok::<_, anyhow::Error>(to)
-            })
-            .await
-            .context("Failed to join the download buffer flush task")?
+            io::copy(&mut io::BufReader::new(data.as_slice()), to)
+                .await
+                .context("Failed to write downloaded data into the destination buffer")?;
+            Ok(())
         }
     }
 
-    async fn delete_relish(&self, path: &Self::RelishStoragePath) -> anyhow::Result<()> {
+    async fn delete(&self, path: &Self::StoragePath) -> anyhow::Result<()> {
         let (_, code) = self
             .bucket
             .delete_object(path.key())
@@ -148,33 +175,12 @@ impl RelishStorage for S3 {
             Ok(())
         }
     }
-
-    async fn upload_relish<R: tokio::io::AsyncRead + std::marker::Unpin + Send>(
-        &self,
-        from: &mut tokio::io::BufReader<R>,
-        to: &Self::RelishStoragePath,
-    ) -> anyhow::Result<()> {
-        let code = self
-            .bucket
-            .put_object_stream(from, to.key())
-            .await
-            .with_context(|| format!("Failed to create s3 object with key {}", to.key()))?;
-        if code != 200 {
-            Err(anyhow::format_err!(
-                "Received non-200 exit code during creating object with key '{}', code: {}",
-                to.key(),
-                code
-            ))
-        } else {
-            Ok(())
-        }
-    }
 }
 
 #[cfg(test)]
 mod tests {
     use crate::{
-        relish_storage::test_utils::{
+        remote_storage::test_utils::{
             custom_tenant_id_path, custom_timeline_id_path, relative_timeline_path,
         },
         repository::repo_harness::{RepoHarness, TIMELINE_ID},
@@ -213,7 +219,7 @@ mod tests {
         let repo_harness = RepoHarness::create("storage_path_positive")?;
 
         let segment_1 = "matching";
-        let segment_2 = "relish";
+        let segment_2 = "file";
         let local_path = &repo_harness.conf.workdir.join(segment_1).join(segment_2);
         let expected_key = S3ObjectKey(format!(
             "{SEPARATOR}{}{SEPARATOR}{}",
@@ -285,7 +291,7 @@ mod tests {
 
         let s3_key = create_s3_key(&relative_timeline_path.join("not a metadata"));
         assert_eq!(
-            RemoteRelishInfo {
+            RemoteFileInfo {
                 tenant_id: repo_harness.tenant_id,
                 timeline_id: TIMELINE_ID,
                 download_destination: s3_key.download_destination(&repo_harness.conf.workdir),
@@ -294,12 +300,12 @@ mod tests {
             storage
                 .info(&s3_key)
                 .expect("For a valid input, valid S3 info should be parsed"),
-            "Should be able to parse metadata out of the correctly named remote delta relish"
+            "Should be able to parse metadata out of the correctly named remote delta file"
         );
 
         let s3_key = create_s3_key(&relative_timeline_path.join(METADATA_FILE_NAME));
         assert_eq!(
-            RemoteRelishInfo {
+            RemoteFileInfo {
                 tenant_id: repo_harness.tenant_id,
                 timeline_id: TIMELINE_ID,
                 download_destination: s3_key.download_destination(&repo_harness.conf.workdir),
@@ -320,7 +326,7 @@ mod tests {
         fn storage_info_error(storage: &S3, s3_key: &S3ObjectKey) -> String {
             match storage.info(s3_key) {
                 Ok(wrong_info) => panic!(
-                    "Expected key {:?} to error, but got relish info: {:?}",
+                    "Expected key {:?} to error, but got file info: {:?}",
                     s3_key, wrong_info,
                 ),
                 Err(e) => e.to_string(),
@@ -381,9 +387,9 @@ mod tests {
         }
     }
 
-    fn create_s3_key(relative_relish_path: &Path) -> S3ObjectKey {
+    fn create_s3_key(relative_file_path: &Path) -> S3ObjectKey {
         S3ObjectKey(
-            relative_relish_path
+            relative_file_path
                 .iter()
                 .fold(String::new(), |mut path_string, segment| {
                     path_string.push(S3_FILE_SEPARATOR);

pageserver/src/remote_storage/storage_sync.rs

@@ -1,4 +1,4 @@
-//! A synchronization logic for the [`RelishStorage`] and the state to ensure the correct synchronizations.
+//! A synchronization logic for the [`RemoteStorage`] and the state to ensure the correct synchronizations.
 //!
 //! The synchronization does not aim to be immediate, instead
 //! doing all the job in a separate thread asynchronously, attempting to fully replicate the
@@ -9,12 +9,12 @@
 //!
 //! During the loop startup, an initial loop state is constructed from all remote storage entries.
 //! It's enough to poll the remote state once on startup only, due to agreement that the pageserver has
-//! an exclusive write access to the relish storage: new files appear in the storage only after the same
+//! an exclusive write access to the remote storage: new files appear in the storage only after the same
 //! pageserver writes them.
 //!
 //! The list construction is currently the only place where the storage sync can return an [`Err`] to the user.
 //! New upload tasks are accepted via [`schedule_timeline_upload`] function disregarding of the corresponding loop startup,
-//! it's up to the caller to avoid uploading of the new relishes, if that caller did not enable the loop.
+//! it's up to the caller to avoid uploading of the new file, if that caller did not enable the loop.
 //! After the initial state is loaded into memory and the loop starts, any further [`Err`] results do not stop the loop, but rather
 //! reschedules the same task, with possibly less files to sync in it.
 //!
@@ -51,7 +51,7 @@
 //! and two files are considered "equal", if their paths match. Such files are uploaded/downloaded over, no real contents checks are done.
 //! NOTE: No real contents or checksum check happens right now and is a subject to improve later.
 //!
-//! After the whole is downloaded, [`crate::tenant_mgr::register_relish_download`] function is used to register the image in pageserver.
+//! After the whole timeline is downloaded, [`crate::tenant_mgr::register_timeline_download`] function is used to register the image in pageserver.
 //!
 //! When pageserver signals shutdown, current sync task gets finished and the loop exists.
 //!
@@ -70,13 +70,18 @@ use std::{
 use anyhow::{ensure, Context};
 use futures::stream::{FuturesUnordered, StreamExt};
 use lazy_static::lazy_static;
-use tokio::{sync::Semaphore, time::Instant};
+use tokio::{
+    fs,
+    io::{self, AsyncWriteExt},
+    sync::Semaphore,
+    time::Instant,
+};
 use tracing::*;
 
-use super::{RelishStorage, RemoteRelishInfo};
+use super::{RemoteFileInfo, RemoteStorage};
 use crate::{
     layered_repository::metadata::{metadata_path, TimelineMetadata},
-    tenant_mgr::register_relish_download,
+    tenant_mgr::register_timeline_download,
     PageServerConf,
 };
 use zenith_metrics::{register_histogram_vec, register_int_gauge, HistogramVec, IntGauge};
@@ -87,12 +92,12 @@ use zenith_utils::{
 
 lazy_static! {
     static ref REMAINING_SYNC_ITEMS: IntGauge = register_int_gauge!(
-        "pageserver_backup_remaining_sync_items",
+        "pageserver_remote_storage_remaining_sync_items",
         "Number of storage sync items left in the queue"
     )
-    .expect("failed to register pageserver backup remaining sync items int gauge");
+    .expect("failed to register pageserver remote storage remaining sync items int gauge");
     static ref IMAGE_SYNC_TIME: HistogramVec = register_histogram_vec!(
-        "pageserver_backup_image_sync_time",
+        "pageserver_remote_storage_image_sync_time",
         "Time took to synchronize (download or upload) a whole pageserver image. \
         Grouped by `operation_kind` (upload|download) and `status` (success|failure)",
         &["operation_kind", "status"],
@@ -176,14 +181,14 @@ pub fn schedule_timeline_upload(
         }))
 }
 
-/// Uses a relish storage given to start the storage sync loop.
+/// Uses a remote storage given to start the storage sync loop.
 /// See module docs for loop step description.
 pub(super) fn spawn_storage_sync_thread<
     P: std::fmt::Debug,
-    S: 'static + RelishStorage<RelishStoragePath = P>,
+    S: 'static + RemoteStorage<StoragePath = P>,
 >(
     config: &'static PageServerConf,
-    relish_storage: S,
+    remote_storage: S,
     max_concurrent_sync: usize,
 ) -> anyhow::Result<thread::JoinHandle<anyhow::Result<()>>> {
     ensure!(
@@ -192,10 +197,10 @@ pub(super) fn spawn_storage_sync_thread<
     );
 
     let handle = thread::Builder::new()
-        .name("Queue based relish storage sync".to_string())
+        .name("Queue based remote storage sync".to_string())
         .spawn(move || {
             let concurrent_sync_limit = Semaphore::new(max_concurrent_sync);
-            let thread_result = storage_sync_loop(config, relish_storage, &concurrent_sync_limit);
+            let thread_result = storage_sync_loop(config, remote_storage, &concurrent_sync_limit);
             concurrent_sync_limit.close();
             if let Err(e) = &thread_result {
                 error!("Failed to run storage sync thread: {:#}", e);
@@ -205,16 +210,16 @@ pub(super) fn spawn_storage_sync_thread<
     Ok(handle)
 }
 
-fn storage_sync_loop<P: std::fmt::Debug, S: 'static + RelishStorage<RelishStoragePath = P>>(
+fn storage_sync_loop<P: std::fmt::Debug, S: 'static + RemoteStorage<StoragePath = P>>(
     config: &'static PageServerConf,
-    relish_storage: S,
+    remote_storage: S,
     concurrent_sync_limit: &Semaphore,
 ) -> anyhow::Result<()> {
     let runtime = tokio::runtime::Builder::new_current_thread()
         .enable_all()
         .build()?;
     let mut remote_timelines = runtime
-        .block_on(fetch_existing_uploads(&relish_storage))
+        .block_on(fetch_existing_uploads(&remote_storage))
         .context("Failed to determine previously uploaded timelines")?;
 
     let urgent_downloads = latest_timelines(&remote_timelines)
@@ -226,7 +231,7 @@ fn storage_sync_loop<P: std::fmt::Debug, S: 'static + RelishStorage<RelishStorag
             let exists_locally = config.timeline_path(&timeline_id, &tenant_id).exists();
             if exists_locally {
                 debug!(
-                    "Timeline with tenant id {}, relish id {} exists locally, not downloading",
+                    "Timeline with tenant id {}, timeline id {} exists locally, not downloading",
                     tenant_id, timeline_id
                 );
                 false
@@ -266,7 +271,7 @@ fn storage_sync_loop<P: std::fmt::Debug, S: 'static + RelishStorage<RelishStorag
                             let sync_status = download_timeline(
                                 config,
                                 concurrent_sync_limit,
-                                &relish_storage,
+                                &remote_storage,
                                 download_data,
                                 false,
                             )
@@ -277,7 +282,7 @@ fn storage_sync_loop<P: std::fmt::Debug, S: 'static + RelishStorage<RelishStorag
                             let sync_status = download_timeline(
                                 config,
                                 concurrent_sync_limit,
-                                &relish_storage,
+                                &remote_storage,
                                 download_data,
                                 true,
                             )
@@ -289,7 +294,7 @@ fn storage_sync_loop<P: std::fmt::Debug, S: 'static + RelishStorage<RelishStorag
                                 config,
                                 concurrent_sync_limit,
                                 &mut remote_timelines,
-                                &relish_storage,
+                                &remote_storage,
                                 layer_upload,
                             )
                             .await;
@@ -305,7 +310,7 @@ fn storage_sync_loop<P: std::fmt::Debug, S: 'static + RelishStorage<RelishStorag
             }
         };
     }
-    log::debug!("Queue based relish storage sync thread shut down");
+    debug!("Queue based remote storage sync thread shut down");
     Ok(())
 }
 
@@ -315,7 +320,7 @@ fn add_to_queue(task: SyncTask) {
 
 fn register_sync_status(sync_start: Instant, sync_name: &str, sync_status: Option<bool>) {
     let secs_elapsed = sync_start.elapsed().as_secs_f64();
-    log::debug!("Processed a sync task in {} seconds", secs_elapsed);
+    debug!("Processed a sync task in {} seconds", secs_elapsed);
     match sync_status {
         Some(true) => IMAGE_SYNC_TIME.with_label_values(&[sync_name, "success"]),
         Some(false) => IMAGE_SYNC_TIME.with_label_values(&[sync_name, "failure"]),
@@ -349,33 +354,30 @@ fn latest_timelines(
         .collect()
 }
 
-async fn fetch_existing_uploads<
-    P: std::fmt::Debug,
-    S: 'static + RelishStorage<RelishStoragePath = P>,
->(
-    relish_storage: &S,
+async fn fetch_existing_uploads<P: std::fmt::Debug, S: 'static + RemoteStorage<StoragePath = P>>(
+    remote_storage: &S,
 ) -> anyhow::Result<HashMap<(ZTenantId, ZTimelineId), RemoteTimeline>> {
-    let uploaded_relishes = relish_storage
-        .list_relishes()
+    let uploaded_files = remote_storage
+        .list()
         .await
-        .context("Failed to list relish uploads")?;
+        .context("Failed to list the uploads")?;
 
-    let mut relish_data_fetches = uploaded_relishes
+    let mut data_fetches = uploaded_files
         .into_iter()
         .map(|remote_path| async {
             (
-                remote_relish_info(relish_storage, &remote_path).await,
+                remote_file_info(remote_storage, &remote_path).await,
                 remote_path,
             )
         })
         .collect::<FuturesUnordered<_>>();
 
     let mut fetched = HashMap::new();
-    while let Some((fetch_result, remote_path)) = relish_data_fetches.next().await {
+    while let Some((fetch_result, remote_path)) = data_fetches.next().await {
         match fetch_result {
-            Ok((relish_info, remote_metadata)) => {
-                let tenant_id = relish_info.tenant_id;
-                let timeline_id = relish_info.timeline_id;
+            Ok((file_info, remote_metadata)) => {
+                let tenant_id = file_info.tenant_id;
+                let timeline_id = file_info.timeline_id;
                 let remote_timeline =
                     fetched
                         .entry((tenant_id, timeline_id))
@@ -388,14 +390,12 @@ async fn fetch_existing_uploads<
                 if remote_metadata.is_some() {
                     remote_timeline.metadata = remote_metadata;
                 } else {
-                    remote_timeline
-                        .layers
-                        .push(relish_info.download_destination);
+                    remote_timeline.layers.push(file_info.download_destination);
                 }
             }
             Err(e) => {
                 warn!(
-                    "Failed to fetch relish info for path {:?}, reason: {:#}",
+                    "Failed to fetch file info for path {:?}, reason: {:#}",
                     remote_path, e
                 );
                 continue;
@@ -406,22 +406,29 @@ async fn fetch_existing_uploads<
     Ok(fetched)
 }
 
-async fn remote_relish_info<P, S: 'static + RelishStorage<RelishStoragePath = P>>(
-    relish_storage: &S,
+async fn remote_file_info<P, S: 'static + RemoteStorage<StoragePath = P>>(
+    remote_storage: &S,
     remote_path: &P,
-) -> anyhow::Result<(RemoteRelishInfo, Option<TimelineMetadata>)> {
-    let info = relish_storage.info(remote_path)?;
+) -> anyhow::Result<(RemoteFileInfo, Option<TimelineMetadata>)> {
+    let info = remote_storage.info(remote_path)?;
     let metadata = if info.is_metadata {
-        let metadata_bytes = relish_storage
-            .download_relish(remote_path, std::io::BufWriter::new(Vec::new()))
+        let mut metadata_bytes = io::BufWriter::new(std::io::Cursor::new(Vec::new()));
+        remote_storage
+            .download(remote_path, &mut metadata_bytes)
             .await
-            .and_then(|buf_writer| Ok(buf_writer.into_inner()?))
             .with_context(|| {
                 format!(
                     "Failed to download metadata file contents for tenant {}, timeline {}",
                     info.tenant_id, info.timeline_id
                 )
             })?;
+        metadata_bytes.flush().await.with_context(|| {
+            format!(
+                "Failed to download metadata file contents for tenant {}, timeline {}",
+                info.tenant_id, info.timeline_id
+            )
+        })?;
+        let metadata_bytes = metadata_bytes.into_inner().into_inner();
         Some(TimelineMetadata::from_bytes(&metadata_bytes)?)
     } else {
         None
@@ -429,10 +436,10 @@ async fn remote_relish_info<P, S: 'static + RelishStorage<RelishStoragePath = P>
     Ok((info, metadata))
 }
 
-async fn download_timeline<'a, P, S: 'static + RelishStorage<RelishStoragePath = P>>(
+async fn download_timeline<'a, P, S: 'static + RemoteStorage<StoragePath = P>>(
     config: &'static PageServerConf,
     concurrent_sync_limit: &'a Semaphore,
-    relish_storage: &'a S,
+    remote_storage: &'a S,
     remote_timeline: RemoteTimeline,
     urgent: bool,
 ) -> Option<bool> {
@@ -451,7 +458,7 @@ async fn download_timeline<'a, P, S: 'static + RelishStorage<RelishStoragePath =
     let sync_result = synchronize_layers(
         config,
         concurrent_sync_limit,
-        relish_storage,
+        remote_storage,
         remote_timeline.layers.into_iter(),
         SyncOperation::Download,
         &new_metadata,
@@ -462,7 +469,7 @@ async fn download_timeline<'a, P, S: 'static + RelishStorage<RelishStoragePath =
 
     match sync_result {
         SyncResult::Success { .. } => {
-            register_relish_download(config, tenant_id, timeline_id);
+            register_timeline_download(config, tenant_id, timeline_id);
             Some(true)
         }
         SyncResult::MetadataSyncError { .. } => {
@@ -497,11 +504,11 @@ async fn download_timeline<'a, P, S: 'static + RelishStorage<RelishStoragePath =
 }
 
 #[allow(clippy::unnecessary_filter_map)]
-async fn upload_timeline<'a, P, S: 'static + RelishStorage<RelishStoragePath = P>>(
+async fn upload_timeline<'a, P, S: 'static + RemoteStorage<StoragePath = P>>(
     config: &'static PageServerConf,
     concurrent_sync_limit: &'a Semaphore,
     remote_timelines: &'a mut HashMap<(ZTenantId, ZTimelineId), RemoteTimeline>,
-    relish_storage: &'a S,
+    remote_storage: &'a S,
     mut new_upload: LocalTimeline,
 ) -> Option<bool> {
     let tenant_id = new_upload.tenant_id;
@@ -547,7 +554,7 @@ async fn upload_timeline<'a, P, S: 'static + RelishStorage<RelishStoragePath = P
     let sync_result = synchronize_layers(
         config,
         concurrent_sync_limit,
-        relish_storage,
+        remote_storage,
         new_layers.into_iter(),
         SyncOperation::Upload,
         &new_metadata,
@@ -595,9 +602,9 @@ async fn upload_timeline<'a, P, S: 'static + RelishStorage<RelishStoragePath = P
 
 /// Layer sync operation kind.
 ///
-/// This enum allows to unify the logic for image relish uploads and downloads.
+/// This enum allows to unify the logic for image uploads and downloads.
 /// When image's layers are synchronized, the only difference
-/// between downloads and uploads is the [`RelishStorage`] method we need to call.
+/// between downloads and uploads is the [`RemoteStorage`] method we need to call.
 #[derive(Debug, Copy, Clone)]
 enum SyncOperation {
     Download,
@@ -607,13 +614,13 @@ enum SyncOperation {
 /// Image sync result.
 #[derive(Debug)]
 enum SyncResult {
-    /// All relish files are synced (their paths returned).
+    /// All regular files are synced (their paths returned).
     /// Metadata file is synced too (path not returned).
     Success { synced: Vec<PathBuf> },
-    /// All relish files are synced (their paths returned).
+    /// All regular files are synced (their paths returned).
     /// Metadata file is not synced (path not returned).
     MetadataSyncError { synced: Vec<PathBuf> },
-    /// Some relish files are not synced, some are (paths returned).
+    /// Some regular files are not synced, some are (paths returned).
     /// Metadata file is not synced (path not returned).
     LayerSyncError {
         synced: Vec<PathBuf>,
@@ -622,13 +629,13 @@ enum SyncResult {
 }
 
 /// Synchronizes given layers and metadata contents of a certain image.
-/// Relishes are always synced before metadata files are, the latter gets synced only if
+/// Regular files are always synced before metadata files are, the latter gets synced only if
 /// the rest of the files are successfully processed.
 #[allow(clippy::too_many_arguments)]
-async fn synchronize_layers<'a, P, S: 'static + RelishStorage<RelishStoragePath = P>>(
+async fn synchronize_layers<'a, P, S: 'static + RemoteStorage<StoragePath = P>>(
     config: &'static PageServerConf,
     concurrent_sync_limit: &'a Semaphore,
-    relish_storage: &'a S,
+    remote_storage: &'a S,
     layers: impl Iterator<Item = PathBuf>,
     sync_operation: SyncOperation,
     new_metadata: &'a TimelineMetadata,
@@ -643,8 +650,8 @@ async fn synchronize_layers<'a, P, S: 'static + RelishStorage<RelishStoragePath
                 .await
                 .expect("Semaphore should not be closed yet");
             let sync_result = match sync_operation {
-                SyncOperation::Download => download(relish_storage, &layer_path).await,
-                SyncOperation::Upload => upload(relish_storage, &layer_path).await,
+                SyncOperation::Download => download(remote_storage, &layer_path).await,
+                SyncOperation::Upload => upload(remote_storage, &layer_path).await,
             };
             drop(permit);
             (layer_path, sync_result)
@@ -677,7 +684,7 @@ async fn synchronize_layers<'a, P, S: 'static + RelishStorage<RelishStoragePath
         trace!("Synced layers: {:?}", synced);
         match sync_metadata(
             config,
-            relish_storage,
+            remote_storage,
             sync_operation,
             new_metadata,
             tenant_id,
@@ -702,9 +709,9 @@ async fn synchronize_layers<'a, P, S: 'static + RelishStorage<RelishStoragePath
     }
 }
 
-async fn sync_metadata<'a, P, S: 'static + RelishStorage<RelishStoragePath = P>>(
+async fn sync_metadata<'a, P, S: 'static + RemoteStorage<StoragePath = P>>(
     config: &'static PageServerConf,
-    relish_storage: &'a S,
+    remote_storage: &'a S,
     sync_operation: SyncOperation,
     new_metadata: &'a TimelineMetadata,
     tenant_id: ZTenantId,
@@ -727,7 +734,7 @@ async fn sync_metadata<'a, P, S: 'static + RelishStorage<RelishStoragePath = P>>
             .await?;
         }
         SyncOperation::Upload => {
-            let remote_path = relish_storage
+            let remote_path = remote_storage
                 .storage_path(&local_metadata_path)
                 .with_context(|| {
                     format!(
@@ -735,26 +742,28 @@ async fn sync_metadata<'a, P, S: 'static + RelishStorage<RelishStoragePath = P>>
                         local_metadata_path.display()
                     )
                 })?;
-            let mut bytes = tokio::io::BufReader::new(std::io::Cursor::new(new_metadata_bytes));
-            relish_storage
-                .upload_relish(&mut bytes, &remote_path)
+            remote_storage
+                .upload(
+                    io::BufReader::new(std::io::Cursor::new(new_metadata_bytes)),
+                    &remote_path,
+                )
                 .await?;
         }
     }
     Ok(())
 }
 
-async fn upload<P, S: 'static + RelishStorage<RelishStoragePath = P>>(
-    relish_storage: &S,
+async fn upload<P, S: 'static + RemoteStorage<StoragePath = P>>(
+    remote_storage: &S,
     source: &Path,
 ) -> anyhow::Result<()> {
-    let destination = relish_storage.storage_path(source).with_context(|| {
+    let destination = remote_storage.storage_path(source).with_context(|| {
         format!(
             "Failed to derive storage destination out of upload path {}",
             source.display()
         )
     })?;
-    let mut source_file = tokio::io::BufReader::new(
+    let source_file = io::BufReader::new(
         tokio::fs::OpenOptions::new()
             .read(true)
             .open(source)
@@ -766,19 +775,17 @@ async fn upload<P, S: 'static + RelishStorage<RelishStoragePath = P>>(
                 )
             })?,
     );
-    relish_storage
-        .upload_relish(&mut source_file, &destination)
-        .await
+    remote_storage.upload(source_file, &destination).await
 }
 
-async fn download<P, S: 'static + RelishStorage<RelishStoragePath = P>>(
-    relish_storage: &S,
+async fn download<P, S: 'static + RemoteStorage<StoragePath = P>>(
+    remote_storage: &S,
     destination: &Path,
 ) -> anyhow::Result<()> {
     if destination.exists() {
         Ok(())
     } else {
-        let source = relish_storage.storage_path(destination).with_context(|| {
+        let source = remote_storage.storage_path(destination).with_context(|| {
             format!(
                 "Failed to derive storage source out of download destination '{}'",
                 destination.display()
@@ -797,11 +804,12 @@ async fn download<P, S: 'static + RelishStorage<RelishStoragePath = P>>(
                     })?;
             }
         }
-        let destination_file = std::io::BufWriter::new(
-            std::fs::OpenOptions::new()
+        let mut destination_file = io::BufWriter::new(
+            fs::OpenOptions::new()
                 .write(true)
                 .create_new(true)
                 .open(destination)
+                .await
                 .with_context(|| {
                     format!(
                         "Failed to open download destination file '{}'",
@@ -810,9 +818,10 @@ async fn download<P, S: 'static + RelishStorage<RelishStoragePath = P>>(
                 })?,
         );
 
-        relish_storage
-            .download_relish(&source, destination_file)
+        remote_storage
+            .download(&source, &mut destination_file)
             .await?;
+        destination_file.flush().await?;
         Ok(())
     }
 }
@@ -827,7 +836,7 @@ mod tests {
 
     use super::*;
     use crate::{
-        relish_storage::local_fs::LocalFs,
+        remote_storage::local_fs::LocalFs,
         repository::repo_harness::{RepoHarness, TIMELINE_ID},
     };
     use hex_literal::hex;
@@ -1001,7 +1010,7 @@ mod tests {
         let repo_harness = RepoHarness::create("reupload_missing_metadata")?;
         let storage = LocalFs::new(tempdir()?.path().to_owned(), &repo_harness.conf.workdir)?;
         let mut remote_timelines =
-            store_incorrect_metadata_relishes(&repo_harness, &storage).await?;
+            store_timelines_with_incorrect_metadata(&repo_harness, &storage).await?;
         assert_timelines_equal(
             remote_timelines.clone(),
             fetch_existing_uploads(&storage).await?,
@@ -1080,7 +1089,7 @@ mod tests {
         let repo_harness = RepoHarness::create("test_download_timeline")?;
         let storage = LocalFs::new(tempdir()?.path().to_owned(), &repo_harness.conf.workdir)?;
         let mut remote_timelines =
-            store_incorrect_metadata_relishes(&repo_harness, &storage).await?;
+            store_timelines_with_incorrect_metadata(&repo_harness, &storage).await?;
         fs::remove_dir_all(repo_harness.timeline_path(&NO_METADATA_TIMELINE_ID))?;
         fs::remove_dir_all(repo_harness.timeline_path(&CORRUPT_METADATA_TIMELINE_ID))?;
 
@@ -1314,20 +1323,20 @@ mod tests {
     async fn ensure_correct_timeline_upload<'a>(
         harness: &RepoHarness,
         remote_timelines: &'a mut HashMap<(ZTenantId, ZTimelineId), RemoteTimeline>,
-        relish_storage: &'a LocalFs,
+        remote_storage: &'a LocalFs,
         new_upload: LocalTimeline,
     ) {
         upload_timeline(
             harness.conf,
             &LIMIT,
             remote_timelines,
-            relish_storage,
+            remote_storage,
             new_upload.clone(),
         )
         .await;
         assert_timelines_equal(
             remote_timelines.clone(),
-            fetch_existing_uploads(relish_storage).await.unwrap(),
+            fetch_existing_uploads(remote_storage).await.unwrap(),
         );
 
         let new_remote_files = remote_timelines
@@ -1437,7 +1446,7 @@ mod tests {
             let actual_remote_paths = fs::read_dir(
                 remote_file
                     .parent()
-                    .expect("Remote relishes are expected to have their timeline dir as parent"),
+                    .expect("Remote files are expected to have their timeline dir as parent"),
             )
             .unwrap()
             .map(|dir| dir.unwrap().path())
@@ -1454,7 +1463,7 @@ mod tests {
         }
     }
 
-    async fn store_incorrect_metadata_relishes(
+    async fn store_timelines_with_incorrect_metadata(
         harness: &RepoHarness,
         storage: &LocalFs,
     ) -> anyhow::Result<HashMap<(ZTenantId, ZTimelineId), RemoteTimeline>> {
@@ -1486,15 +1495,15 @@ mod tests {
         .await;
 
         storage
-            .delete_relish(&storage.storage_path(&metadata_path(
+            .delete(&storage.storage_path(&metadata_path(
                 harness.conf,
                 NO_METADATA_TIMELINE_ID,
                 harness.tenant_id,
             ))?)
             .await?;
         storage
-            .upload_relish(
-                &mut BufReader::new(Cursor::new("corrupt meta".to_string().into_bytes())),
+            .upload(
+                BufReader::new(Cursor::new("corrupt meta".to_string().into_bytes())),
                 &storage.storage_path(&metadata_path(
                     harness.conf,
                     CORRUPT_METADATA_TIMELINE_ID,
@@ -1503,8 +1512,8 @@ mod tests {
             )
             .await?;
 
-        for remote_relish in remote_timelines.values_mut() {
-            remote_relish.metadata = None;
+        for remote_file in remote_timelines.values_mut() {
+            remote_file.metadata = None;
         }
 
         Ok(remote_timelines)

pageserver/src/repository.rs

@@ -1,9 +1,9 @@
 use crate::relish::*;
 use crate::CheckpointConfig;
 use anyhow::Result;
-use bytes::{Buf, BufMut, Bytes, BytesMut};
-use serde::{Deserialize, Serialize};
+use bytes::Bytes;
 use std::collections::HashSet;
+use std::convert::TryInto;
 use std::ops::{AddAssign, Deref};
 use std::sync::Arc;
 use std::time::Duration;
@@ -160,6 +160,9 @@ pub trait Timeline: Send + Sync {
     /// Does the same as get_current_logical_size but counted on demand.
     /// Used in tests to ensure thet incremental and non incremental variants match.
     fn get_current_logical_size_non_incremental(&self, lsn: Lsn) -> Result<usize>;
+
+    /// An escape hatch to allow "casting" a generic Timeline to LayeredTimeline.
+    fn upgrade_to_layered_timeline(&self) -> &crate::layered_repository::LayeredTimeline;
 }
 
 /// Various functions to mutate the timeline.
@@ -189,38 +192,50 @@ pub trait TimelineWriter: Deref<Target = dyn Timeline> {
     fn advance_last_record_lsn(&self, lsn: Lsn);
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
-pub struct WALRecord {
-    pub will_init: bool,
-    pub rec: Bytes,
+#[derive(Debug, Clone)]
+pub struct WALRecord(Box<[u8]>);
+
+pub const WAL_BIT: u8 = 0b1;
+const WILL_INIT_BIT: u8 = 0b10;
+
+impl WALRecord {
+    pub fn new(will_init: bool, main_data_offset: u32, rec: &[u8]) -> Self {
+        // TODO avoid init
+        let mut buf = vec![0u8; 1 + 4 + rec.len()];
+        buf[0] = WAL_BIT | if will_init { WILL_INIT_BIT } else { 0 };
+        let mdo = u32::to_le_bytes(main_data_offset);
+        buf[1..5].copy_from_slice(&mdo);
+        buf[5..].copy_from_slice(rec);
+        Self(buf.into_boxed_slice())
+    }
+
+    pub fn from_bytes(bytes: Box<[u8]>) -> Self {
+        Self(bytes)
+    }
+
+    pub fn will_init(&self) -> bool {
+        self.0[0] & WILL_INIT_BIT != 0
+    }
+
     // Remember the offset of main_data in rec,
     // so that we don't have to parse the record again.
     // If record has no main_data, this offset equals rec.len().
-    pub main_data_offset: u32,
-}
-
-impl WALRecord {
-    pub fn pack(&self, buf: &mut BytesMut) {
-        buf.put_u8(self.will_init as u8);
-        buf.put_u32(self.main_data_offset);
-        buf.put_u32(self.rec.len() as u32);
-        buf.put_slice(&self.rec[..]);
+    pub fn main_data_offset(&self) -> u32 {
+        u32::from_le_bytes(self.0[1..5].try_into().unwrap())
     }
-    pub fn unpack(buf: &mut Bytes) -> WALRecord {
-        let will_init = buf.get_u8() != 0;
-        let main_data_offset = buf.get_u32();
-        let rec_len = buf.get_u32() as usize;
-        let rec = buf.split_to(rec_len);
-        WALRecord {
-            will_init,
-            rec,
-            main_data_offset,
-        }
+
+    pub fn rec(&self) -> &[u8] {
+        &self.0[5..]
+    }
+
+    pub fn bytes(&self) -> &[u8] {
+        &self.0[..]
     }
 }
 
 #[cfg(test)]
 pub mod repo_harness {
+    use bytes::BytesMut;
     use std::{fs, path::PathBuf};
 
     use crate::{

pageserver/src/restore_local_repo.rs

@@ -427,11 +427,11 @@ pub fn save_decoded_record(
             forknum: blk.forknum as u8,
         });
 
-        let rec = WALRecord {
-            will_init: blk.will_init || blk.apply_image,
-            rec: recdata.clone(),
-            main_data_offset: decoded.main_data_offset as u32,
-        };
+        let rec = WALRecord::new(
+            blk.will_init || blk.apply_image,
+            decoded.main_data_offset as u32,
+            &recdata[..],
+        );
 
         timeline.put_wal_record(lsn, tag, blk.blkno, rec)?;
     }
@@ -770,11 +770,7 @@ fn save_xact_record(
 
     let segno = pageno / pg_constants::SLRU_PAGES_PER_SEGMENT;
     let rpageno = pageno % pg_constants::SLRU_PAGES_PER_SEGMENT;
-    let rec = WALRecord {
-        will_init: false,
-        rec: decoded.record.clone(),
-        main_data_offset: decoded.main_data_offset as u32,
-    };
+    let rec = WALRecord::new(false, decoded.main_data_offset as u32, &decoded.record[..]);
     timeline.put_wal_record(
         lsn,
         RelishTag::Slru {
@@ -886,11 +882,7 @@ fn save_multixact_create_record(
     xlrec: &XlMultiXactCreate,
     decoded: &DecodedWALRecord,
 ) -> Result<()> {
-    let rec = WALRecord {
-        will_init: false,
-        rec: decoded.record.clone(),
-        main_data_offset: decoded.main_data_offset as u32,
-    };
+    let rec = WALRecord::new(false, decoded.main_data_offset as u32, &decoded.record[..]);
     let pageno = xlrec.mid / pg_constants::MULTIXACT_OFFSETS_PER_PAGE as u32;
     let segno = pageno / pg_constants::SLRU_PAGES_PER_SEGMENT;
     let rpageno = pageno % pg_constants::SLRU_PAGES_PER_SEGMENT;

pageserver/src/tenant_mgr.rs

@@ -101,13 +101,10 @@ fn init_repo(conf: &'static PageServerConf, tenant_id: ZTenantId) {
     let mut m = access_tenants();
     let tenant = m.get_mut(&tenant_id).unwrap();
     tenant.repo = Some(repo);
-    tenant.state = TenantState::Active;
-
-    // TODO Start these threads only if tenant actively receives some WAL
-    tenant_threads::start_tenant_threads(conf, tenant_id);
+    tenant.state = TenantState::Idle;
 }
 
-pub fn register_relish_download(
+pub fn register_timeline_download(
     conf: &'static PageServerConf,
     tenant_id: ZTenantId,
     timeline_id: ZTimelineId,
@@ -128,12 +125,12 @@ pub fn register_relish_download(
         match &tenant.repo {
             Some(repo) => {
                 init_timeline(repo.as_ref(), timeline_id);
-                tenant.state = TenantState::Active;
+                tenant.state = TenantState::Idle;
                 return;
             }
             None => log::warn!("Initialize new repo"),
         }
-        tenant.state = TenantState::Active;
+        tenant.state = TenantState::Idle;
     }
 
     // init repo updates Tenant state
@@ -197,7 +194,7 @@ pub fn create_repository_for_tenant(
     let mut m = access_tenants();
     let tenant = m.get_mut(&tenantid).unwrap();
     tenant.repo = Some(repo);
-    tenant.state = TenantState::Active;
+    tenant.state = TenantState::Idle;
 
     Ok(())
 }
@@ -211,18 +208,18 @@ pub fn get_tenant_state(tenantid: ZTenantId) -> TenantState {
     }
 }
 
-pub fn set_tenant_state(tenantid: ZTenantId, state: TenantState) -> Result<TenantState> {
+pub fn set_tenant_state(tenantid: ZTenantId, newstate: TenantState) -> Result<TenantState> {
     let mut m = access_tenants();
     let tenant = m.get_mut(&tenantid);
 
     match tenant {
         Some(tenant) => {
-            if state == TenantState::Idle && tenant.state != TenantState::Active {
+            if newstate == TenantState::Idle && tenant.state != TenantState::Active {
                 // Only Active tenant can become Idle
                 return Ok(tenant.state);
             }
-            info!("set_tenant_state: {} -> {}", tenant.state, state);
-            tenant.state = state;
+            info!("set_tenant_state: {} -> {}", tenant.state, newstate);
+            tenant.state = newstate;
             Ok(tenant.state)
         }
         None => bail!("Tenant not found for tenant {}", tenantid),

pageserver/src/tenant_threads.rs

@@ -11,6 +11,7 @@ use std::sync::Mutex;
 use std::thread::JoinHandle;
 use std::time::Duration;
 use tracing::*;
+use zenith_metrics::{register_int_gauge_vec, IntGaugeVec};
 use zenith_utils::zid::ZTenantId;
 
 struct TenantHandleEntry {
@@ -25,33 +26,44 @@ lazy_static! {
         Mutex::new(HashMap::new());
 }
 
+lazy_static! {
+    static ref TENANT_THREADS_COUNT: IntGaugeVec = register_int_gauge_vec!(
+        "tenant_threads_count",
+        "Number of live tenant threads",
+        &["tenant_thread_type"]
+    )
+    .expect("failed to define a metric");
+}
+
+// Launch checkpointer and GC for the tenant.
+// It's possible that the threads are running already,
+// if so, just don't spawn new ones.
 pub fn start_tenant_threads(conf: &'static PageServerConf, tenantid: ZTenantId) {
-    //ensure that old threads are stopeed
-    wait_for_tenant_threads_to_stop(tenantid);
-
-    let checkpointer_handle = std::thread::Builder::new()
-        .name("Checkpointer thread".into())
-        .spawn(move || {
-            checkpoint_loop(tenantid, conf).expect("Checkpointer thread died");
-        })
-        .ok();
-
-    let gc_handle = std::thread::Builder::new()
-        .name("GC thread".into())
-        .spawn(move || {
-            gc_loop(tenantid, conf).expect("GC thread died");
-        })
-        .ok();
-
-    // TODO handle thread errors if any
-
     let mut handles = TENANT_HANDLES.lock().unwrap();
-    let h = TenantHandleEntry {
-        checkpointer_handle,
-        gc_handle,
-    };
+    let h = handles
+        .entry(tenantid)
+        .or_insert_with(|| TenantHandleEntry {
+            checkpointer_handle: None,
+            gc_handle: None,
+        });
 
-    handles.insert(tenantid, h);
+    if h.checkpointer_handle.is_none() {
+        h.checkpointer_handle = std::thread::Builder::new()
+            .name("Checkpointer thread".into())
+            .spawn(move || {
+                checkpoint_loop(tenantid, conf).expect("Checkpointer thread died");
+            })
+            .ok();
+    }
+
+    if h.gc_handle.is_none() {
+        h.gc_handle = std::thread::Builder::new()
+            .name("GC thread".into())
+            .spawn(move || {
+                gc_loop(tenantid, conf).expect("GC thread died");
+            })
+            .ok();
+    }
 }
 
 pub fn wait_for_tenant_threads_to_stop(tenantid: ZTenantId) {
@@ -69,6 +81,12 @@ pub fn wait_for_tenant_threads_to_stop(tenantid: ZTenantId) {
 /// Checkpointer thread's main loop
 ///
 fn checkpoint_loop(tenantid: ZTenantId, conf: &'static PageServerConf) -> Result<()> {
+    let gauge = TENANT_THREADS_COUNT.with_label_values(&["checkpointer"]);
+    gauge.inc();
+    scopeguard::defer! {
+        gauge.dec();
+    }
+
     loop {
         if tenant_mgr::get_tenant_state(tenantid) != TenantState::Active {
             break;
@@ -95,6 +113,12 @@ fn checkpoint_loop(tenantid: ZTenantId, conf: &'static PageServerConf) -> Result
 /// GC thread's main loop
 ///
 fn gc_loop(tenantid: ZTenantId, conf: &'static PageServerConf) -> Result<()> {
+    let gauge = TENANT_THREADS_COUNT.with_label_values(&["gc"]);
+    gauge.inc();
+    scopeguard::defer! {
+        gauge.dec();
+    }
+
     loop {
         if tenant_mgr::get_tenant_state(tenantid) != TenantState::Active {
             break;

pageserver/src/walreceiver.rs

@@ -5,9 +5,12 @@
 //!
 //! We keep one WAL receiver active per timeline.
 
+use crate::layered_repository;
 use crate::relish::*;
 use crate::restore_local_repo;
 use crate::tenant_mgr;
+use crate::tenant_mgr::TenantState;
+use crate::tenant_threads;
 use crate::waldecoder::*;
 use crate::PageServerConf;
 use anyhow::{bail, Error, Result};
@@ -38,6 +41,7 @@ use zenith_utils::zid::ZTimelineId;
 struct WalReceiverEntry {
     wal_producer_connstr: String,
     wal_receiver_handle: Option<JoinHandle<()>>,
+    tenantid: ZTenantId,
 }
 
 lazy_static! {
@@ -65,6 +69,23 @@ pub fn stop_wal_receiver(timelineid: ZTimelineId) {
     }
 }
 
+pub fn drop_wal_receiver(timelineid: ZTimelineId, tenantid: ZTenantId) {
+    let mut receivers = WAL_RECEIVERS.lock().unwrap();
+    receivers.remove(&timelineid);
+
+    // Check if it was the last walreceiver of the tenant.
+    // TODO now we store one WalReceiverEntry per timeline,
+    // so this iterator looks a bit strange.
+    for (_timelineid, entry) in receivers.iter() {
+        if entry.tenantid == tenantid {
+            return;
+        }
+    }
+
+    // When last walreceiver of the tenant is gone, change state to Idle
+    tenant_mgr::set_tenant_state(tenantid, TenantState::Idle).unwrap();
+}
+
 // Launch a new WAL receiver, or tell one that's running about change in connection string
 pub fn launch_wal_receiver(
     conf: &'static PageServerConf,
@@ -90,8 +111,13 @@ pub fn launch_wal_receiver(
             let receiver = WalReceiverEntry {
                 wal_producer_connstr: wal_producer_connstr.into(),
                 wal_receiver_handle: Some(wal_receiver_handle),
+                tenantid,
             };
             receivers.insert(timelineid, receiver);
+
+            // Update tenant state and start tenant threads, if they are not running yet.
+            tenant_mgr::set_tenant_state(tenantid, TenantState::Active).unwrap();
+            tenant_threads::start_tenant_threads(conf, tenantid);
         }
     };
 }
@@ -114,11 +140,15 @@ fn thread_main(conf: &'static PageServerConf, timelineid: ZTimelineId, tenantid:
     let _enter = info_span!("WAL receiver", timeline = %timelineid, tenant = %tenantid).entered();
     info!("WAL receiver thread started");
 
+    let mut retry_count = 10;
+
     //
     // Make a connection to the WAL safekeeper, or directly to the primary PostgreSQL server,
     // and start streaming WAL from it. If the connection is lost, keep retrying.
+    // TODO How long should we retry in case of losing connection?
+    // Should we retry at all or we can wait for the next callmemaybe request?
     //
-    while !tenant_mgr::shutdown_requested() {
+    while !tenant_mgr::shutdown_requested() && retry_count > 0 {
         // Look up the current WAL producer address
         let wal_producer_connstr = get_wal_producer_connstr(timelineid);
 
@@ -129,14 +159,24 @@ fn thread_main(conf: &'static PageServerConf, timelineid: ZTimelineId, tenantid:
                 "WAL streaming connection failed ({}), retrying in 1 second",
                 e
             );
+            retry_count -= 1;
             sleep(Duration::from_secs(1));
+        } else {
+            info!(
+                "walreceiver disconnected tenant {}, timelineid {}",
+                tenantid, timelineid
+            );
+            break;
         }
     }
-    debug!("WAL streaming shut down");
+    info!("WAL streaming shut down");
+    // Drop it from list of active WAL_RECEIVERS
+    // so that next callmemaybe request launched a new thread
+    drop_wal_receiver(timelineid, tenantid);
 }
 
 fn walreceiver_main(
-    _conf: &PageServerConf,
+    conf: &PageServerConf,
     timelineid: ZTimelineId,
     wal_producer_connstr: &str,
     tenantid: ZTenantId,
@@ -256,6 +296,9 @@ fn walreceiver_main(
                     caught_up = true;
                 }
 
+                // Release memory if needed
+                layered_repository::evict_layer_if_needed(conf)?;
+
                 Some(endlsn)
             }
 
@@ -300,6 +343,7 @@ fn walreceiver_main(
             break;
         }
     }
+
     Ok(())
 }
 

pageserver/src/walredo.rs

@@ -22,23 +22,23 @@ use byteorder::{ByteOrder, LittleEndian};
 use bytes::{Buf, BufMut, Bytes, BytesMut};
 use lazy_static::lazy_static;
 use log::*;
+use nix::poll::*;
 use serde::Serialize;
 use std::fs;
 use std::fs::OpenOptions;
 use std::io::prelude::*;
-use std::io::Error;
+use std::io::{Error, ErrorKind};
+use std::os::unix::io::AsRawFd;
 use std::path::PathBuf;
 use std::process::Stdio;
+use std::process::{Child, ChildStderr, ChildStdin, ChildStdout, Command};
 use std::sync::Mutex;
 use std::time::Duration;
 use std::time::Instant;
-use tokio::io::AsyncBufReadExt;
-use tokio::io::{AsyncReadExt, AsyncWriteExt};
-use tokio::process::{ChildStdin, ChildStdout, Command};
-use tokio::time::timeout;
 use zenith_metrics::{register_histogram, register_int_counter, Histogram, IntCounter};
 use zenith_utils::bin_ser::BeSer;
 use zenith_utils::lsn::Lsn;
+use zenith_utils::nonblock::set_nonblock;
 use zenith_utils::zid::ZTenantId;
 
 use crate::relish::*;
@@ -139,7 +139,6 @@ pub struct PostgresRedoManager {
     tenantid: ZTenantId,
     conf: &'static PageServerConf,
 
-    runtime: tokio::runtime::Runtime,
     process: Mutex<Option<PostgresRedoProcess>>,
 }
 
@@ -215,20 +214,23 @@ impl WalRedoManager for PostgresRedoManager {
 
             // launch the WAL redo process on first use
             if process_guard.is_none() {
-                let p = self
-                    .runtime
-                    .block_on(PostgresRedoProcess::launch(self.conf, &self.tenantid))?;
+                let p = PostgresRedoProcess::launch(self.conf, &self.tenantid)?;
                 *process_guard = Some(p);
             }
             let process = process_guard.as_mut().unwrap();
 
-            result = self
-                .runtime
-                .block_on(self.handle_apply_request_postgres(process, &request));
+            result = self.handle_apply_request_postgres(process, &request);
 
             WAL_REDO_WAIT_TIME.observe(lock_time.duration_since(start_time).as_secs_f64());
             end_time = Instant::now();
             WAL_REDO_TIME.observe(end_time.duration_since(lock_time).as_secs_f64());
+
+            // If something went wrong, don't try to reuse the process. Kill it, and
+            // next request will launch a new one.
+            if result.is_err() {
+                let process = process_guard.take().unwrap();
+                process.kill();
+            }
         }
 
         result
@@ -240,17 +242,8 @@ impl PostgresRedoManager {
     /// Create a new PostgresRedoManager.
     ///
     pub fn new(conf: &'static PageServerConf, tenantid: ZTenantId) -> PostgresRedoManager {
-        // We block on waiting for requests on the walredo request channel, but
-        // use async I/O to communicate with the child process. Initialize the
-        // runtime for the async part.
-        let runtime = tokio::runtime::Builder::new_current_thread()
-            .enable_all()
-            .build()
-            .unwrap();
-
         // The actual process is launched lazily, on first request.
         PostgresRedoManager {
-            runtime,
             tenantid,
             conf,
             process: Mutex::new(None),
@@ -260,7 +253,7 @@ impl PostgresRedoManager {
     ///
     /// Process one request for WAL redo using wal-redo postgres
     ///
-    async fn handle_apply_request_postgres(
+    fn handle_apply_request_postgres(
         &self,
         process: &mut PostgresRedoProcess,
         request: &WalRedoRequest,
@@ -278,14 +271,14 @@ impl PostgresRedoManager {
         if let RelishTag::Relation(rel) = request.rel {
             // Relational WAL records are applied using wal-redo-postgres
             let buf_tag = BufferTag { rel, blknum };
-            apply_result = process.apply_wal_records(buf_tag, base_img, records).await;
+            apply_result = process.apply_wal_records(buf_tag, base_img, records);
 
             let duration = start.elapsed();
 
             debug!(
-                "postgres applied {} WAL records in {} ms to reconstruct page image at LSN {}",
+                "postgres applied {} WAL records in {} us to reconstruct page image at LSN {}",
                 nrecords,
-                duration.as_millis(),
+                duration.as_micros(),
                 lsn
             );
 
@@ -324,7 +317,7 @@ impl PostgresRedoManager {
         }
         // Apply all collected WAL records
         for (_lsn, record) in records {
-            let mut buf = record.rec.clone();
+            let mut buf = Bytes::from(record.rec().to_vec());
 
             WAL_REDO_RECORD_COUNTER.inc();
 
@@ -335,7 +328,7 @@ impl PostgresRedoManager {
             //move to main data
             // TODO probably, we should store some records in our special format
             // to avoid this weird parsing on replay
-            let skip = (record.main_data_offset - pg_constants::SIZEOF_XLOGRECORD) as usize;
+            let skip = (record.main_data_offset() - pg_constants::SIZEOF_XLOGRECORD) as usize;
             if buf.remaining() > skip {
                 buf.advance(skip);
             }
@@ -469,18 +462,17 @@ impl PostgresRedoManager {
 /// Handle to the Postgres WAL redo process
 ///
 struct PostgresRedoProcess {
+    child: Child,
     stdin: ChildStdin,
     stdout: ChildStdout,
+    stderr: ChildStderr,
 }
 
 impl PostgresRedoProcess {
     //
     // Start postgres binary in special WAL redo mode.
     //
-    async fn launch(
-        conf: &PageServerConf,
-        tenantid: &ZTenantId,
-    ) -> Result<PostgresRedoProcess, Error> {
+    fn launch(conf: &PageServerConf, tenantid: &ZTenantId) -> Result<PostgresRedoProcess, Error> {
         // FIXME: We need a dummy Postgres cluster to run the process in. Currently, we
         // just create one with constant name. That fails if you try to launch more than
         // one WAL redo manager concurrently.
@@ -501,7 +493,6 @@ impl PostgresRedoProcess {
             .env("LD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap())
             .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir().to_str().unwrap())
             .output()
-            .await
             .expect("failed to execute initdb");
 
         if !initdb.status.success() {
@@ -538,102 +529,139 @@ impl PostgresRedoProcess {
             datadir.display()
         );
 
-        let stdin = child.stdin.take().expect("failed to open child's stdin");
-        let stderr = child.stderr.take().expect("failed to open child's stderr");
-        let stdout = child.stdout.take().expect("failed to open child's stdout");
+        let stdin = child.stdin.take().unwrap();
+        let stdout = child.stdout.take().unwrap();
+        let stderr = child.stderr.take().unwrap();
 
-        // This async block reads the child's stderr, and forwards it to the logger
-        let f_stderr = async {
-            let mut stderr_buffered = tokio::io::BufReader::new(stderr);
+        set_nonblock(stdin.as_raw_fd())?;
+        set_nonblock(stdout.as_raw_fd())?;
+        set_nonblock(stderr.as_raw_fd())?;
 
-            let mut line = String::new();
-            loop {
-                let res = stderr_buffered.read_line(&mut line).await;
-                if res.is_err() {
-                    debug!("could not convert line to utf-8");
-                    continue;
-                }
-                if res.unwrap() == 0 {
-                    break;
-                }
-                error!("wal-redo-postgres: {}", line.trim());
-                line.clear();
-            }
-            Ok::<(), Error>(())
-        };
-        tokio::spawn(f_stderr);
+        Ok(PostgresRedoProcess {
+            child,
+            stdin,
+            stdout,
+            stderr,
+        })
+    }
 
-        Ok(PostgresRedoProcess { stdin, stdout })
+    fn kill(mut self) {
+        let _ = self.child.kill();
+        if let Ok(exit_status) = self.child.wait() {
+            error!("wal-redo-postgres exited with code {}", exit_status);
+        }
+        drop(self);
     }
 
     //
     // Apply given WAL records ('records') over an old page image. Returns
     // new page image.
     //
-    async fn apply_wal_records(
+    fn apply_wal_records(
         &mut self,
         tag: BufferTag,
         base_img: Option<Bytes>,
         records: &[(Lsn, WALRecord)],
     ) -> Result<Bytes, std::io::Error> {
-        let stdout = &mut self.stdout;
-        // Buffer the writes to avoid a lot of small syscalls.
-        let mut stdin = tokio::io::BufWriter::new(&mut self.stdin);
+        // Serialize all the messages to send the WAL redo process first.
+        //
+        // This could be problematic if there are millions of records to replay,
+        // but in practice the number of records is usually so small that it doesn't
+        // matter, and it's better to keep this code simple.
+        let mut writebuf: Vec<u8> = Vec::new();
+        build_begin_redo_for_block_msg(tag, &mut writebuf);
+        if let Some(img) = base_img {
+            build_push_page_msg(tag, &img, &mut writebuf);
+        }
+        for (lsn, rec) in records.iter() {
+            build_apply_record_msg(*lsn, rec.rec(), &mut writebuf);
+        }
+        build_get_page_msg(tag, &mut writebuf);
+        WAL_REDO_RECORD_COUNTER.inc_by(records.len() as u64);
+
+        // The input is now in 'writebuf'. Do a blind write first, writing as much as
+        // we can, before calling poll(). That skips one call to poll() if the stdin is
+        // already available for writing, which it almost certainly is because the
+        // process is idle.
+        let mut nwrite = self.stdin.write(&writebuf)?;
+
+        // We expect the WAL redo process to respond with an 8k page image. We read it
+        // into this buffer.
+        let mut resultbuf = vec![0; pg_constants::BLCKSZ.into()];
+        let mut nresult: usize = 0; // # of bytes read into 'resultbuf' so far
+
+        // Prepare for calling poll()
+        let mut pollfds = [
+            PollFd::new(self.stdout.as_raw_fd(), PollFlags::POLLIN),
+            PollFd::new(self.stderr.as_raw_fd(), PollFlags::POLLIN),
+            PollFd::new(self.stdin.as_raw_fd(), PollFlags::POLLOUT),
+        ];
 
         // We do three things simultaneously: send the old base image and WAL records to
         // the child process's stdin, read the result from child's stdout, and forward any logging
         // information that the child writes to its stderr to the page server's log.
-        //
-        // 'f_stdin' handles writing the base image and WAL records to the child process.
-        // 'f_stdout' below reads the result back. And 'f_stderr', which was spawned into the
-        // tokio runtime in the 'launch' function already, forwards the logging.
-        let f_stdin = async {
-            // Send base image, if any. (If the record initializes the page, previous page
-            // version is not needed.)
-            timeout(
-                TIMEOUT,
-                stdin.write_all(&build_begin_redo_for_block_msg(tag)),
-            )
-            .await??;
-            if let Some(img) = base_img {
-                timeout(TIMEOUT, stdin.write_all(&build_push_page_msg(tag, &img))).await??;
+        while nresult < pg_constants::BLCKSZ.into() {
+            // If we have more data to write, wake up if 'stdin' becomes writeable or
+            // we have data to read. Otherwise only wake up if there's data to read.
+            let nfds = if nwrite < writebuf.len() { 3 } else { 2 };
+            let n = nix::poll::poll(&mut pollfds[0..nfds], TIMEOUT.as_millis() as i32)?;
+
+            if n == 0 {
+                return Err(Error::new(ErrorKind::Other, "WAL redo timed out"));
             }
 
-            // Send WAL records.
-            for (lsn, rec) in records.iter() {
-                WAL_REDO_RECORD_COUNTER.inc();
+            // If we have some messages in stderr, forward them to the log.
+            let err_revents = pollfds[1].revents().unwrap();
+            if err_revents & (PollFlags::POLLERR | PollFlags::POLLIN) != PollFlags::empty() {
+                let mut errbuf: [u8; 16384] = [0; 16384];
+                let n = self.stderr.read(&mut errbuf)?;
 
-                stdin
-                    .write_all(&build_apply_record_msg(*lsn, &rec.rec))
-                    .await?;
+                // The message might not be split correctly into lines here. But this is
+                // good enough, the important thing is to get the message to the log.
+                if n > 0 {
+                    error!(
+                        "wal-redo-postgres: {}",
+                        String::from_utf8_lossy(&errbuf[0..n])
+                    );
 
-                //debug!("sent WAL record to wal redo postgres process ({:X}/{:X}",
-                //       r.lsn >> 32, r.lsn & 0xffff_ffff);
+                    // To make sure we capture all log from the process if it fails, keep
+                    // reading from the stderr, before checking the stdout.
+                    continue;
+                }
+            } else if err_revents.contains(PollFlags::POLLHUP) {
+                return Err(Error::new(
+                    ErrorKind::BrokenPipe,
+                    "WAL redo process closed its stderr unexpectedly",
+                ));
             }
-            //debug!("sent {} WAL records to wal redo postgres process ({:X}/{:X}",
-            //       records.len(), lsn >> 32, lsn & 0xffff_ffff);
 
-            // Send GetPage command to get the result back
-            timeout(TIMEOUT, stdin.write_all(&build_get_page_msg(tag))).await??;
-            timeout(TIMEOUT, stdin.flush()).await??;
-            //debug!("sent GetPage for {}", tag.blknum);
-            Ok::<(), Error>(())
-        };
+            // If we have more data to write and 'stdin' is writeable, do write.
+            if nwrite < writebuf.len() {
+                let in_revents = pollfds[2].revents().unwrap();
+                if in_revents & (PollFlags::POLLERR | PollFlags::POLLOUT) != PollFlags::empty() {
+                    nwrite += self.stdin.write(&writebuf[nwrite..])?;
+                } else if in_revents.contains(PollFlags::POLLHUP) {
+                    // We still have more data to write, but the process closed the pipe.
+                    return Err(Error::new(
+                        ErrorKind::BrokenPipe,
+                        "WAL redo process closed its stdin unexpectedly",
+                    ));
+                }
+            }
 
-        // Read back new page image
-        let f_stdout = async {
-            let mut buf = [0u8; 8192];
+            // If we have some data in stdout, read it to the result buffer.
+            let out_revents = pollfds[0].revents().unwrap();
+            if out_revents & (PollFlags::POLLERR | PollFlags::POLLIN) != PollFlags::empty() {
+                nresult += self.stdout.read(&mut resultbuf[nresult..])?;
+            } else if out_revents.contains(PollFlags::POLLHUP) {
+                return Err(Error::new(
+                    ErrorKind::BrokenPipe,
+                    "WAL redo process closed its stdout unexpectedly",
+                ));
+            }
+        }
 
-            timeout(TIMEOUT, stdout.read_exact(&mut buf)).await??;
-            //debug!("got response for {}", tag.blknum);
-            Ok::<[u8; 8192], Error>(buf)
-        };
-
-        let res = tokio::try_join!(f_stdout, f_stdin)?;
-
-        let buf = res.0;
-
-        Ok::<Bytes, Error>(Bytes::from(std::vec::Vec::from(buf)))
+        Ok(Bytes::from(resultbuf))
     }
 }
 
@@ -641,62 +669,42 @@ impl PostgresRedoProcess {
 // process. See vendor/postgres/src/backend/tcop/zenith_wal_redo.c for
 // explanation of the protocol.
 
-fn build_begin_redo_for_block_msg(tag: BufferTag) -> Vec<u8> {
+fn build_begin_redo_for_block_msg(tag: BufferTag, buf: &mut Vec<u8>) {
     let len = 4 + 1 + 4 * 4;
-    let mut buf = Vec::with_capacity(1 + len);
 
     buf.put_u8(b'B');
     buf.put_u32(len as u32);
 
-    tag.ser_into(&mut buf)
+    tag.ser_into(buf)
         .expect("serialize BufferTag should always succeed");
-
-    debug_assert!(buf.len() == 1 + len);
-
-    buf
 }
 
-fn build_push_page_msg(tag: BufferTag, base_img: &[u8]) -> Vec<u8> {
+fn build_push_page_msg(tag: BufferTag, base_img: &[u8], buf: &mut Vec<u8>) {
     assert!(base_img.len() == 8192);
 
     let len = 4 + 1 + 4 * 4 + base_img.len();
-    let mut buf = Vec::with_capacity(1 + len);
 
     buf.put_u8(b'P');
     buf.put_u32(len as u32);
-    tag.ser_into(&mut buf)
+    tag.ser_into(buf)
         .expect("serialize BufferTag should always succeed");
     buf.put(base_img);
-
-    debug_assert!(buf.len() == 1 + len);
-
-    buf
 }
 
-fn build_apply_record_msg(endlsn: Lsn, rec: &[u8]) -> Vec<u8> {
+fn build_apply_record_msg(endlsn: Lsn, rec: &[u8], buf: &mut Vec<u8>) {
     let len = 4 + 8 + rec.len();
-    let mut buf: Vec<u8> = Vec::with_capacity(1 + len);
 
     buf.put_u8(b'A');
     buf.put_u32(len as u32);
     buf.put_u64(endlsn.0);
     buf.put(rec);
-
-    debug_assert!(buf.len() == 1 + len);
-
-    buf
 }
 
-fn build_get_page_msg(tag: BufferTag) -> Vec<u8> {
+fn build_get_page_msg(tag: BufferTag, buf: &mut Vec<u8>) {
     let len = 4 + 1 + 4 * 4;
-    let mut buf = Vec::with_capacity(1 + len);
 
     buf.put_u8(b'G');
     buf.put_u32(len as u32);
-    tag.ser_into(&mut buf)
+    tag.ser_into(buf)
         .expect("serialize BufferTag should always succeed");
-
-    debug_assert!(buf.len() == 1 + len);
-
-    buf
 }

proxy/src/main.rs

@@ -17,7 +17,7 @@ use clap::{App, Arg, ArgMatches};
 
 use cplane_api::DatabaseInfo;
 use rustls::{internal::pemfile, NoClientAuth, ProtocolVersion, ServerConfig};
-use zenith_utils::tcp_listener;
+use zenith_utils::{tcp_listener, GIT_VERSION};
 
 mod cplane_api;
 mod mgmt;
@@ -78,6 +78,7 @@ fn configure_ssl(arg_matches: &ArgMatches) -> anyhow::Result<Option<Arc<ServerCo
 
 fn main() -> anyhow::Result<()> {
     let arg_matches = App::new("Zenith proxy/router")
+        .version(GIT_VERSION)
         .arg(
             Arg::with_name("proxy")
                 .short("p")
@@ -139,6 +140,8 @@ fn main() -> anyhow::Result<()> {
     };
     let state: &'static ProxyState = Box::leak(Box::new(state));
 
+    println!("Version: {}", GIT_VERSION);
+
     // Check that we can bind to address before further initialization
     println!("Starting proxy on {}", state.conf.proxy_address);
     let pageserver_listener = tcp_listener::bind(state.conf.proxy_address)?;

scripts/generate_perf_report_page.py

@@ -0,0 +1,218 @@
+#!/usr/bin/env python3
+import argparse
+from dataclasses import dataclass
+from pathlib import Path
+import json
+from typing import Any, Dict, List, Optional, Tuple, cast
+from jinja2 import Template
+
+# skip 'input' columns. They are included in the header and just blow the table
+EXCLUDE_COLUMNS = frozenset({
+    'scale',
+    'duration',
+    'number_of_clients',
+    'number_of_threads',
+    'init_start_timestamp',
+    'init_end_timestamp',
+    'run_start_timestamp',
+    'run_end_timestamp',
+})
+
+KEY_EXCLUDE_FIELDS = frozenset({
+    'init_start_timestamp',
+    'init_end_timestamp',
+    'run_start_timestamp',
+    'run_end_timestamp',
+})
+NEGATIVE_COLOR = 'negative'
+POSITIVE_COLOR = 'positive'
+
+
+@dataclass
+class SuitRun:
+    revision: str
+    values: Dict[str, Any]
+
+
+@dataclass
+class SuitRuns:
+    platform: str
+    suit: str
+    common_columns: List[str]
+    value_columns: List[str]
+    runs: List[SuitRun]
+
+
+@dataclass
+class RowValue:
+    value: str
+    color: str
+    ratio: str
+
+
+def get_columns(values: List[Dict]) -> Tuple[List[Tuple[str, str]], List[str]]:
+    value_columns = []
+    common_columns = []
+    for item in values:
+        if item['name'] in KEY_EXCLUDE_FIELDS:
+            continue
+        if item['report'] != 'test_param':
+            value_columns.append(cast(str, item['name']))
+        else:
+            common_columns.append(
+                (cast(str, item['name']), cast(str, item['value'])))
+    value_columns.sort()
+    common_columns.sort(key=lambda x: x[0])  # sort by name
+    return common_columns, value_columns
+
+
+def format_ratio(ratio: float, report: str) -> Tuple[str, str]:
+    color = ''
+    sign = '+' if ratio > 0 else ''
+    if abs(ratio) < 0.05:
+        return f'&nbsp({sign}{ratio:.2f})', color
+
+    if report not in {'test_param', 'higher_is_better', 'lower_is_better'}:
+        raise ValueError(f'Unknown report type: {report}')
+
+    if report == 'test_param':
+        return f'{ratio:.2f}', color
+
+    if ratio > 0:
+        if report == 'higher_is_better':
+            color = POSITIVE_COLOR
+        elif report == 'lower_is_better':
+            color = NEGATIVE_COLOR
+    elif ratio < 0:
+        if report == 'higher_is_better':
+            color = NEGATIVE_COLOR
+        elif report == 'lower_is_better':
+            color = POSITIVE_COLOR
+
+    return f'&nbsp({sign}{ratio:.2f})', color
+
+
+def extract_value(name: str, suit_run: SuitRun) -> Optional[Dict[str, Any]]:
+    for item in suit_run.values['data']:
+        if item['name'] == name:
+            return item
+    return None
+
+
+def get_row_values(columns: List[str], run_result: SuitRun,
+                   prev_result: Optional[SuitRun]) -> List[RowValue]:
+    row_values = []
+    for column in columns:
+        current_value = extract_value(column, run_result)
+        if current_value is None:
+            # should never happen
+            raise ValueError(f'{column} not found in {run_result.values}')
+
+        value = current_value["value"]
+        if isinstance(value, float):
+            value = f'{value:.2f}'
+
+        if prev_result is None:
+            row_values.append(RowValue(value, '', ''))
+            continue
+
+        prev_value = extract_value(column, prev_result)
+        if prev_value is None:
+            # this might happen when new metric is added and there is no value for it in previous run
+            # let this be here, TODO add proper handling when this actually happens
+            raise ValueError(f'{column} not found in previous result')
+        ratio = float(value) / float(prev_value['value']) - 1
+        ratio_display, color = format_ratio(ratio,
+                                            current_value['report'])
+        row_values.append(RowValue(value, color, ratio_display))
+    return row_values
+
+
+@dataclass
+class SuiteRunTableRow:
+    revision: str
+    values: List[RowValue]
+
+
+def prepare_rows_from_runs(value_columns: List[str],
+                           runs: List[SuitRun]) -> List[SuiteRunTableRow]:
+    rows = []
+    prev_run = None
+    for run in runs:
+        rows.append(
+            SuiteRunTableRow(revision=run.revision,
+                             values=get_row_values(value_columns, run,
+                                                   prev_run)))
+        prev_run = run
+
+    return rows
+
+
+def main(args: argparse.Namespace) -> None:
+    input_dir = Path(args.input_dir)
+    grouped_runs = {}
+    # we have files in form: <ctr>_<rev>.json
+    # fill them in the hashmap so we have grouped items for the
+    # same run configuration (scale, duration etc.) ordered by counter.
+    for item in sorted(input_dir.iterdir(),
+                       key=lambda x: int(x.name.split('_')[0])):
+        run_data = json.loads(item.read_text())
+        revision = run_data['revision']
+
+        for suit_result in run_data['result']:
+            key = "{}{}".format(run_data['platform'], suit_result['suit'])
+            # pack total duration as a synthetic value
+            total_duration = suit_result['total_duration']
+            suit_result['data'].append({
+                'name': 'total_duration',
+                'value': total_duration,
+                'unit': 's',
+                'report': 'lower_is_better',
+            })
+            common_columns, value_columns = get_columns(suit_result['data'])
+
+            grouped_runs.setdefault(
+                key,
+                SuitRuns(
+                    platform=run_data['platform'],
+                    suit=suit_result['suit'],
+                    common_columns=common_columns,
+                    value_columns=value_columns,
+                    runs=[],
+                ),
+            )
+
+            grouped_runs[key].runs.append(
+                SuitRun(revision=revision, values=suit_result))
+    context = {}
+    for result in grouped_runs.values():
+        suit = result.suit
+        context[suit] = {
+            'common_columns':
+            result.common_columns,
+            'value_columns':
+            result.value_columns,
+            'platform':
+            result.platform,
+            # reverse the order so newest results are on top of the table
+            'rows':
+            reversed(prepare_rows_from_runs(result.value_columns,
+                                            result.runs)),
+        }
+
+    template = Template((Path(__file__).parent / 'perf_report_template.html').read_text())
+
+    Path(args.out).write_text(template.render(context=context))
+
+
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
+        '--input-dir',
+        dest='input_dir',
+        required=True,
+        help='Directory with jsons generated by the test suite',
+    )
+    parser.add_argument('--out', required=True, help='Output html file path')
+    args = parser.parse_args()
+    main(args)

scripts/perf_report_template.html

@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<html>
+
+<body>
+    <style>
+        table,
+        th,
+        td {
+            border: 1px solid black;
+            border-collapse: collapse;
+        }
+
+        .positive {
+            background-color: rgba(0, 255, 0, 0.8)
+        }
+
+        .negative {
+            background-color: rgba(255, 0, 0, 0.65)
+        }
+    </style>
+
+    <h2>Zenith Performance Tests</h2>
+
+    {% for suit_name, suit_data in context.items() %}
+    <h3>Runs for {{ suit_name }} </h3>
+    <b>platform:</b> {{ suit_data.platform }}<br>
+    {% for common_column_name, common_column_value in suit_data.common_columns %}
+    <b>{{ common_column_name }}</b>: {{ common_column_value }}<br>
+    {% endfor %}
+    <br>
+
+    <table>
+        <tr>
+            <th>revision</th>
+            {% for column_name in suit_data.value_columns %}
+            <th>{{ column_name }}</th>
+            {% endfor %}
+        </tr>
+        {% for row in suit_data.rows %}
+        <tr>
+            <td><a href=https://github.com/zenithdb/zenith/commit/{{ row.revision }}>{{ row.revision[:6] }}</a></td>
+            {% for column_value in row.values %}
+            <td class="{{ column_value.color }}">{{ column_value.value }}{{column_value.ratio}}</td>
+            {% endfor %}
+        </tr>
+        {% endfor %}
+    </table>
+    {% endfor %}
+
+</body>
+
+</html>

test_runner/Pipfile

@@ -5,13 +5,14 @@ name = "pypi"
 
 [packages]
 pytest = ">=6.0.0"
-psycopg2 = "*"
 typing-extensions = "*"
 pyjwt = {extras = ["crypto"], version = "*"}
 requests = "*"
 pytest-xdist = "*"
 asyncpg = "*"
 cached-property = "*"
+psycopg2-binary = "*"
+jinja2 = "*"
 
 [dev-packages]
 # Behavior may change slightly between versions. These are run continuously,

test_runner/Pipfile.lock

@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "63b72760ef37375186a638066ba0ad5804dbace99ddc503ea654e9749070ab24"
+            "sha256": "c309cb963a7b07ae3d30e9cbf08b495f77bdecc0e5356fc89d133c4fbcb65b2b"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -177,13 +177,96 @@
             ],
             "version": "==1.1.1"
         },
-        "packaging": {
+        "jinja2": {
             "hashes": [
-                "sha256:7dc96269f53a4ccec5c0670940a4281106dd0bb343f47b7471f779df49c2fbe7",
-                "sha256:c86254f9220d55e31cc94d69bade760f0847da8000def4dfe1c6b872fd14ff14"
+                "sha256:827a0e32839ab1600d4eb1c4c33ec5a8edfbc5cb42dafa13b81f182f97784b45",
+                "sha256:8569982d3f0889eed11dd620c706d39b60c36d6d25843961f33f77fb6bc6b20c"
+            ],
+            "index": "pypi",
+            "version": "==3.0.2"
+        },
+        "markupsafe": {
+            "hashes": [
+                "sha256:01a9b8ea66f1658938f65b93a85ebe8bc016e6769611be228d797c9d998dd298",
+                "sha256:023cb26ec21ece8dc3907c0e8320058b2e0cb3c55cf9564da612bc325bed5e64",
+                "sha256:0446679737af14f45767963a1a9ef7620189912317d095f2d9ffa183a4d25d2b",
+                "sha256:04635854b943835a6ea959e948d19dcd311762c5c0c6e1f0e16ee57022669194",
+                "sha256:0717a7390a68be14b8c793ba258e075c6f4ca819f15edfc2a3a027c823718567",
+                "sha256:0955295dd5eec6cb6cc2fe1698f4c6d84af2e92de33fbcac4111913cd100a6ff",
+                "sha256:0d4b31cc67ab36e3392bbf3862cfbadac3db12bdd8b02a2731f509ed5b829724",
+                "sha256:10f82115e21dc0dfec9ab5c0223652f7197feb168c940f3ef61563fc2d6beb74",
+                "sha256:168cd0a3642de83558a5153c8bd34f175a9a6e7f6dc6384b9655d2697312a646",
+                "sha256:1d609f577dc6e1aa17d746f8bd3c31aa4d258f4070d61b2aa5c4166c1539de35",
+                "sha256:1f2ade76b9903f39aa442b4aadd2177decb66525062db244b35d71d0ee8599b6",
+                "sha256:20dca64a3ef2d6e4d5d615a3fd418ad3bde77a47ec8a23d984a12b5b4c74491a",
+                "sha256:2a7d351cbd8cfeb19ca00de495e224dea7e7d919659c2841bbb7f420ad03e2d6",
+                "sha256:2d7d807855b419fc2ed3e631034685db6079889a1f01d5d9dac950f764da3dad",
+                "sha256:2ef54abee730b502252bcdf31b10dacb0a416229b72c18b19e24a4509f273d26",
+                "sha256:36bc903cbb393720fad60fc28c10de6acf10dc6cc883f3e24ee4012371399a38",
+                "sha256:37205cac2a79194e3750b0af2a5720d95f786a55ce7df90c3af697bfa100eaac",
+                "sha256:3c112550557578c26af18a1ccc9e090bfe03832ae994343cfdacd287db6a6ae7",
+                "sha256:3dd007d54ee88b46be476e293f48c85048603f5f516008bee124ddd891398ed6",
+                "sha256:4296f2b1ce8c86a6aea78613c34bb1a672ea0e3de9c6ba08a960efe0b0a09047",
+                "sha256:47ab1e7b91c098ab893b828deafa1203de86d0bc6ab587b160f78fe6c4011f75",
+                "sha256:49e3ceeabbfb9d66c3aef5af3a60cc43b85c33df25ce03d0031a608b0a8b2e3f",
+                "sha256:4dc8f9fb58f7364b63fd9f85013b780ef83c11857ae79f2feda41e270468dd9b",
+                "sha256:4efca8f86c54b22348a5467704e3fec767b2db12fc39c6d963168ab1d3fc9135",
+                "sha256:53edb4da6925ad13c07b6d26c2a852bd81e364f95301c66e930ab2aef5b5ddd8",
+                "sha256:5855f8438a7d1d458206a2466bf82b0f104a3724bf96a1c781ab731e4201731a",
+                "sha256:594c67807fb16238b30c44bdf74f36c02cdf22d1c8cda91ef8a0ed8dabf5620a",
+                "sha256:5b6d930f030f8ed98e3e6c98ffa0652bdb82601e7a016ec2ab5d7ff23baa78d1",
+                "sha256:5bb28c636d87e840583ee3adeb78172efc47c8b26127267f54a9c0ec251d41a9",
+                "sha256:60bf42e36abfaf9aff1f50f52644b336d4f0a3fd6d8a60ca0d054ac9f713a864",
+                "sha256:611d1ad9a4288cf3e3c16014564df047fe08410e628f89805e475368bd304914",
+                "sha256:6300b8454aa6930a24b9618fbb54b5a68135092bc666f7b06901f897fa5c2fee",
+                "sha256:63f3268ba69ace99cab4e3e3b5840b03340efed0948ab8f78d2fd87ee5442a4f",
+                "sha256:6557b31b5e2c9ddf0de32a691f2312a32f77cd7681d8af66c2692efdbef84c18",
+                "sha256:693ce3f9e70a6cf7d2fb9e6c9d8b204b6b39897a2c4a1aa65728d5ac97dcc1d8",
+                "sha256:6a7fae0dd14cf60ad5ff42baa2e95727c3d81ded453457771d02b7d2b3f9c0c2",
+                "sha256:6c4ca60fa24e85fe25b912b01e62cb969d69a23a5d5867682dd3e80b5b02581d",
+                "sha256:6fcf051089389abe060c9cd7caa212c707e58153afa2c649f00346ce6d260f1b",
+                "sha256:7d91275b0245b1da4d4cfa07e0faedd5b0812efc15b702576d103293e252af1b",
+                "sha256:89c687013cb1cd489a0f0ac24febe8c7a666e6e221b783e53ac50ebf68e45d86",
+                "sha256:8d206346619592c6200148b01a2142798c989edcb9c896f9ac9722a99d4e77e6",
+                "sha256:905fec760bd2fa1388bb5b489ee8ee5f7291d692638ea5f67982d968366bef9f",
+                "sha256:97383d78eb34da7e1fa37dd273c20ad4320929af65d156e35a5e2d89566d9dfb",
+                "sha256:984d76483eb32f1bcb536dc27e4ad56bba4baa70be32fa87152832cdd9db0833",
+                "sha256:99df47edb6bda1249d3e80fdabb1dab8c08ef3975f69aed437cb69d0a5de1e28",
+                "sha256:9f02365d4e99430a12647f09b6cc8bab61a6564363f313126f775eb4f6ef798e",
+                "sha256:a30e67a65b53ea0a5e62fe23682cfe22712e01f453b95233b25502f7c61cb415",
+                "sha256:ab3ef638ace319fa26553db0624c4699e31a28bb2a835c5faca8f8acf6a5a902",
+                "sha256:aca6377c0cb8a8253e493c6b451565ac77e98c2951c45f913e0b52facdcff83f",
+                "sha256:add36cb2dbb8b736611303cd3bfcee00afd96471b09cda130da3581cbdc56a6d",
+                "sha256:b2f4bf27480f5e5e8ce285a8c8fd176c0b03e93dcc6646477d4630e83440c6a9",
+                "sha256:b7f2d075102dc8c794cbde1947378051c4e5180d52d276987b8d28a3bd58c17d",
+                "sha256:baa1a4e8f868845af802979fcdbf0bb11f94f1cb7ced4c4b8a351bb60d108145",
+                "sha256:be98f628055368795d818ebf93da628541e10b75b41c559fdf36d104c5787066",
+                "sha256:bf5d821ffabf0ef3533c39c518f3357b171a1651c1ff6827325e4489b0e46c3c",
+                "sha256:c47adbc92fc1bb2b3274c4b3a43ae0e4573d9fbff4f54cd484555edbf030baf1",
+                "sha256:cdfba22ea2f0029c9261a4bd07e830a8da012291fbe44dc794e488b6c9bb353a",
+                "sha256:d6c7ebd4e944c85e2c3421e612a7057a2f48d478d79e61800d81468a8d842207",
+                "sha256:d7f9850398e85aba693bb640262d3611788b1f29a79f0c93c565694658f4071f",
+                "sha256:d8446c54dc28c01e5a2dbac5a25f071f6653e6e40f3a8818e8b45d790fe6ef53",
+                "sha256:deb993cacb280823246a026e3b2d81c493c53de6acfd5e6bfe31ab3402bb37dd",
+                "sha256:e0f138900af21926a02425cf736db95be9f4af72ba1bb21453432a07f6082134",
+                "sha256:e9936f0b261d4df76ad22f8fee3ae83b60d7c3e871292cd42f40b81b70afae85",
+                "sha256:f0567c4dc99f264f49fe27da5f735f414c4e7e7dd850cfd8e69f0862d7c74ea9",
+                "sha256:f5653a225f31e113b152e56f154ccbe59eeb1c7487b39b9d9f9cdb58e6c79dc5",
+                "sha256:f826e31d18b516f653fe296d967d700fddad5901ae07c622bb3705955e1faa94",
+                "sha256:f8ba0e8349a38d3001fae7eadded3f6606f0da5d748ee53cc1dab1d6527b9509",
+                "sha256:f9081981fe268bd86831e5c75f7de206ef275defcb82bc70740ae6dc507aee51",
+                "sha256:fa130dd50c57d53368c9d59395cb5526eda596d3ffe36666cd81a44d56e48872"
             ],
             "markers": "python_version >= '3.6'",
-            "version": "==21.0"
+            "version": "==2.0.1"
+        },
+        "packaging": {
+            "hashes": [
+                "sha256:096d689d78ca690e4cd8a89568ba06d07ca097e3306a4381635073ca91479966",
+                "sha256:14317396d1e8cdb122989b916fa2c7e9ca8e2be9e8060a6eff75b6b7b4d8a7e0"
+            ],
+            "markers": "python_version >= '3.6'",
+            "version": "==21.2"
         },
         "pluggy": {
             "hashes": [
@@ -193,17 +276,42 @@
             "markers": "python_version >= '3.6'",
             "version": "==1.0.0"
         },
-        "psycopg2": {
+        "psycopg2-binary": {
             "hashes": [
-                "sha256:079d97fc22de90da1d370c90583659a9f9a6ee4007355f5825e5f1c70dffc1fa",
-                "sha256:2087013c159a73e09713294a44d0c8008204d06326006b7f652bef5ace66eebb",
-                "sha256:2c992196719fadda59f72d44603ee1a2fdcc67de097eea38d41c7ad9ad246e62",
-                "sha256:7640e1e4d72444ef012e275e7b53204d7fab341fb22bc76057ede22fe6860b25",
-                "sha256:7f91312f065df517187134cce8e395ab37f5b601a42446bdc0f0d51773621854",
-                "sha256:830c8e8dddab6b6716a4bf73a09910c7954a92f40cf1d1e702fb93c8a919cc56",
-                "sha256:89409d369f4882c47f7ea20c42c5046879ce22c1e4ea20ef3b00a4dfc0a7f188",
-                "sha256:bf35a25f1aaa8a3781195595577fcbb59934856ee46b4f252f56ad12b8043bcf",
-                "sha256:de5303a6f1d0a7a34b9d40e4d3bef684ccc44a49bbe3eb85e3c0bffb4a131b7c"
+                "sha256:0b7dae87f0b729922e06f85f667de7bf16455d411971b2043bbd9577af9d1975",
+                "sha256:0f2e04bd2a2ab54fa44ee67fe2d002bb90cee1c0f1cc0ebc3148af7b02034cbd",
+                "sha256:123c3fb684e9abfc47218d3784c7b4c47c8587951ea4dd5bc38b6636ac57f616",
+                "sha256:1473c0215b0613dd938db54a653f68251a45a78b05f6fc21af4326f40e8360a2",
+                "sha256:14db1752acdd2187d99cb2ca0a1a6dfe57fc65c3281e0f20e597aac8d2a5bd90",
+                "sha256:1e3a362790edc0a365385b1ac4cc0acc429a0c0d662d829a50b6ce743ae61b5a",
+                "sha256:1e85b74cbbb3056e3656f1cc4781294df03383127a8114cbc6531e8b8367bf1e",
+                "sha256:20f1ab44d8c352074e2d7ca67dc00843067788791be373e67a0911998787ce7d",
+                "sha256:24b0b6688b9f31a911f2361fe818492650795c9e5d3a1bc647acbd7440142a4f",
+                "sha256:2f62c207d1740b0bde5c4e949f857b044818f734a3d57f1d0d0edc65050532ed",
+                "sha256:3242b9619de955ab44581a03a64bdd7d5e470cc4183e8fcadd85ab9d3756ce7a",
+                "sha256:35c4310f8febe41f442d3c65066ca93cccefd75013df3d8c736c5b93ec288140",
+                "sha256:4235f9d5ddcab0b8dbd723dca56ea2922b485ea00e1dafacf33b0c7e840b3d32",
+                "sha256:542875f62bc56e91c6eac05a0deadeae20e1730be4c6334d8f04c944fcd99759",
+                "sha256:5ced67f1e34e1a450cdb48eb53ca73b60aa0af21c46b9b35ac3e581cf9f00e31",
+                "sha256:661509f51531ec125e52357a489ea3806640d0ca37d9dada461ffc69ee1e7b6e",
+                "sha256:7360647ea04db2e7dff1648d1da825c8cf68dc5fbd80b8fb5b3ee9f068dcd21a",
+                "sha256:736b8797b58febabb85494142c627bd182b50d2a7ec65322983e71065ad3034c",
+                "sha256:8c13d72ed6af7fd2c8acbd95661cf9477f94e381fce0792c04981a8283b52917",
+                "sha256:988b47ac70d204aed01589ed342303da7c4d84b56c2f4c4b8b00deda123372bf",
+                "sha256:995fc41ebda5a7a663a254a1dcac52638c3e847f48307b5416ee373da15075d7",
+                "sha256:a36c7eb6152ba5467fb264d73844877be8b0847874d4822b7cf2d3c0cb8cdcb0",
+                "sha256:aed4a9a7e3221b3e252c39d0bf794c438dc5453bc2963e8befe9d4cd324dff72",
+                "sha256:aef9aee84ec78af51107181d02fe8773b100b01c5dfde351184ad9223eab3698",
+                "sha256:b0221ca5a9837e040ebf61f48899926b5783668b7807419e4adae8175a31f773",
+                "sha256:b4d7679a08fea64573c969f6994a2631908bb2c0e69a7235648642f3d2e39a68",
+                "sha256:c250a7ec489b652c892e4f0a5d122cc14c3780f9f643e1a326754aedf82d9a76",
+                "sha256:ca86db5b561b894f9e5f115d6a159fff2a2570a652e07889d8a383b5fae66eb4",
+                "sha256:cfc523edecddaef56f6740d7de1ce24a2fdf94fd5e704091856a201872e37f9f",
+                "sha256:d92272c7c16e105788efe2cfa5d680f07e34e0c29b03c1908f8636f55d5f915a",
+                "sha256:da113b70f6ec40e7d81b43d1b139b9db6a05727ab8be1ee559f3a69854a69d34",
+                "sha256:f6fac64a38f6768e7bc7b035b9e10d8a538a9fadce06b983fb3e6fa55ac5f5ce",
+                "sha256:f8559617b1fcf59a9aedba2c9838b5b6aa211ffedecabca412b92a1ff75aac1a",
+                "sha256:fbb42a541b1093385a2d8c7eec94d26d30437d0e77c1d25dae1dcc46741a385e"
             ],
             "index": "pypi",
             "version": "==2.9.1"
@@ -334,11 +442,11 @@
         },
         "filelock": {
             "hashes": [
-                "sha256:2b5eb3589e7fdda14599e7eb1a50e09b4cc14f34ed98b8ba56d33bfaafcbef2f",
-                "sha256:34a9f35f95c441e7b38209775d6e0337f9a3759f3565f6c5798f19618527c76f"
+                "sha256:7afc856f74fa7006a289fd10fa840e1eebd8bbff6bffb69c26c54a0512ea8cf8",
+                "sha256:bb2a1c717df74c48a2d00ed625e5a66f8572a3a30baacb7657add1d7bac4097b"
             ],
             "markers": "python_version >= '3.6'",
-            "version": "==3.3.1"
+            "version": "==3.3.2"
         },
         "flake8": {
             "hashes": [
@@ -510,11 +618,11 @@
         },
         "virtualenv": {
             "hashes": [
-                "sha256:10062e34c204b5e4ec5f62e6ef2473f8ba76513a9a617e873f1f8fb4a519d300",
-                "sha256:bcc17f0b3a29670dd777d6f0755a4c04f28815395bca279cdcb213b97199a6b8"
+                "sha256:4b02e52a624336eece99c96e3ab7111f469c24ba226a53ec474e8e787b365814",
+                "sha256:576d05b46eace16a9c348085f7d0dc8ef28713a2cabaa1cf0aea41e8f12c9218"
             ],
             "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'",
-            "version": "==20.8.1"
+            "version": "==20.10.0"
         },
         "virtualenv-clone": {
             "hashes": [

test_runner/batch_others/test_wal_acceptor_async.py

@@ -109,7 +109,7 @@ async def wait_for_lsn(safekeeper: Safekeeper,
                        timeline_id: str,
                        wait_lsn: str,
                        polling_interval=1,
-                       timeout=600):
+                       timeout=200):
     """
     Poll flush_lsn from safekeeper until it's greater or equal than
     provided wait_lsn. To do that, timeline_status is fetched from
@@ -147,6 +147,10 @@ async def run_restarts_under_load(pg: Postgres, acceptors: List[Safekeeper], n_w
     period_time = 10
     iterations = 6
 
+    # Set timeout for this test at 15 minutes. It should be enough for
+    # test to complete and less than CircleCI's no_output_timeout.
+    test_timeout_at = time.monotonic() + 15 * 60
+
     pg_conn = await pg.connect_async()
     tenant_id = await pg_conn.fetchval("show zenith.zenith_tenant")
     timeline_id = await pg_conn.fetchval("show zenith.zenith_timeline")
@@ -162,6 +166,8 @@ async def run_restarts_under_load(pg: Postgres, acceptors: List[Safekeeper], n_w
         workers.append(asyncio.create_task(worker))
 
     for it in range(iterations):
+        assert time.monotonic() < test_timeout_at, 'test timed out'
+
         victim_idx = it % len(acceptors)
         victim = acceptors[victim_idx]
         victim.stop()

test_runner/conftest.py

@@ -1 +1 @@
-pytest_plugins = ("fixtures.zenith_fixtures")
+pytest_plugins = ("fixtures.zenith_fixtures", "fixtures.benchmark_fixture")

test_runner/fixtures/benchmark_fixture.py

@@ -1,29 +1,22 @@
+import dataclasses
+import json
 import os
+from pathlib import Path
 import re
+import subprocess
 import timeit
-import pathlib
-import uuid
-import psycopg2
+import calendar
+import enum
+from datetime import datetime
 import pytest
 from _pytest.config import Config
-from _pytest.runner import CallInfo
 from _pytest.terminal import TerminalReporter
-import shutil
-import signal
-import subprocess
-import time
+import warnings
 
 from contextlib import contextmanager
-from contextlib import closing
-from pathlib import Path
-from dataclasses import dataclass
 
 # Type-related stuff
-from psycopg2.extensions import connection as PgConnection
-from typing import Any, Callable, Dict, Iterator, List, Optional, TypeVar, cast
-from typing_extensions import Literal
-
-from .utils import (get_self_dir, mkdir_if_needed, subprocess_capture)
+from typing import Iterator
 """
 This file contains fixtures for micro-benchmarks.
 
@@ -39,7 +32,7 @@ def test_mybench(zenith_simple_env: env, zenbenchmark):
 
     # Initialize the test
     ...
-    
+
     # Run the test, timing how long it takes
     with zenbenchmark.record_duration('test_query'):
         cur.execute('SELECT test_query(...)')
@@ -55,36 +48,91 @@ in the test initialization, or measure disk usage after the test query.
 """
 
 
-# TODO: It would perhaps be better to store the results as additional
-# properties in the pytest TestReport objects, to make them visible to
-# other pytest tools.
-class ZenithBenchmarkResults:
-    """ An object for recording benchmark results. """
-    def __init__(self):
-        self.results = []
+@dataclasses.dataclass
+class PgBenchRunResult:
+    scale: int
+    number_of_clients: int
+    number_of_threads: int
+    number_of_transactions_actually_processed: int
+    latency_average: float
+    latency_stddev: float
+    tps_including_connection_time: float
+    tps_excluding_connection_time: float
+    init_duration: float
+    init_start_timestamp: int
+    init_end_timestamp: int
+    run_duration: float
+    run_start_timestamp: int
+    run_end_timestamp: int
 
-    def record(self, test_name: str, metric_name: str, metric_value: float, unit: str):
-        """
-        Record a benchmark result.
-        """
+    # TODO progress
 
-        self.results.append((test_name, metric_name, metric_value, unit))
+    @classmethod
+    def parse_from_output(
+        cls,
+        out: 'subprocess.CompletedProcess[str]',
+        init_duration: float,
+        init_start_timestamp: int,
+        init_end_timestamp: int,
+        run_duration: float,
+        run_start_timestamp: int,
+        run_end_timestamp: int,
+    ):
+        stdout_lines = out.stdout.splitlines()
+        # we know significant parts of these values from test input
+        # but to be precise take them from output
+        # scaling factor: 5
+        assert "scaling factor" in stdout_lines[1]
+        scale = int(stdout_lines[1].split()[-1])
+        # number of clients: 1
+        assert "number of clients" in stdout_lines[3]
+        number_of_clients = int(stdout_lines[3].split()[-1])
+        # number of threads: 1
+        assert "number of threads" in stdout_lines[4]
+        number_of_threads = int(stdout_lines[4].split()[-1])
+        # number of transactions actually processed: 1000/1000
+        assert "number of transactions actually processed" in stdout_lines[6]
+        number_of_transactions_actually_processed = int(stdout_lines[6].split("/")[1])
+        # latency average = 19.894 ms
+        assert "latency average" in stdout_lines[7]
+        latency_average = stdout_lines[7].split()[-2]
+        # latency stddev = 3.387 ms
+        assert "latency stddev" in stdout_lines[8]
+        latency_stddev = stdout_lines[8].split()[-2]
+        # tps = 50.219689 (including connections establishing)
+        assert "(including connections establishing)" in stdout_lines[9]
+        tps_including_connection_time = stdout_lines[9].split()[2]
+        # tps = 50.264435 (excluding connections establishing)
+        assert "(excluding connections establishing)" in stdout_lines[10]
+        tps_excluding_connection_time = stdout_lines[10].split()[2]
+
+        return cls(
+            scale=scale,
+            number_of_clients=number_of_clients,
+            number_of_threads=number_of_threads,
+            number_of_transactions_actually_processed=number_of_transactions_actually_processed,
+            latency_average=float(latency_average),
+            latency_stddev=float(latency_stddev),
+            tps_including_connection_time=float(tps_including_connection_time),
+            tps_excluding_connection_time=float(tps_excluding_connection_time),
+            init_duration=init_duration,
+            init_start_timestamp=init_start_timestamp,
+            init_end_timestamp=init_end_timestamp,
+            run_duration=run_duration,
+            run_start_timestamp=run_start_timestamp,
+            run_end_timestamp=run_end_timestamp,
+        )
 
 
-# Will be recreated in each session.
-zenbenchmark_results: ZenithBenchmarkResults = ZenithBenchmarkResults()
-
-
-# Session scope fixture that initializes the results object
-@pytest.fixture(autouse=True, scope='session')
-def zenbenchmark_global(request) -> Iterator[ZenithBenchmarkResults]:
-    """
-    This is a python decorator for benchmark fixtures
-    """
-    global zenbenchmark_results
-    zenbenchmark_results = ZenithBenchmarkResults()
-
-    yield zenbenchmark_results
+@enum.unique
+class MetricReport(str, enum.Enum):  # str is a hack to make it json serializable
+    # this means that this is a constant test parameter
+    # like number of transactions, or number of clients
+    TEST_PARAM = 'test_param'
+    # reporter can use it to mark test runs with higher values as improvements
+    HIGHER_IS_BETTER = 'higher_is_better'
+    # the same but for lower values
+    LOWER_IS_BETTER = 'lower_is_better'
 
 
 class ZenithBenchmarker:
@@ -92,30 +140,109 @@ class ZenithBenchmarker:
     An object for recording benchmark results. This is created for each test
     function by the zenbenchmark fixture
     """
-    def __init__(self, results, request):
-        self.results = results
-        self.request = request
+    def __init__(self, property_recorder):
+        # property recorder here is a pytest fixture provided by junitxml module
+        # https://docs.pytest.org/en/6.2.x/reference.html#pytest.junitxml.record_property
+        self.property_recorder = property_recorder
 
-    def record(self, metric_name: str, metric_value: float, unit: str):
+    def record(
+        self,
+        metric_name: str,
+        metric_value: float,
+        unit: str,
+        report: MetricReport,
+    ):
         """
         Record a benchmark result.
         """
-        self.results.record(self.request.node.name, metric_name, metric_value, unit)
+        # just to namespace the value
+        name = f"zenith_benchmarker_{metric_name}"
+        self.property_recorder(
+            name,
+            {
+                "name": metric_name,
+                "value": metric_value,
+                "unit": unit,
+                "report": report,
+            },
+        )
 
     @contextmanager
-    def record_duration(self, metric_name):
+    def record_duration(self, metric_name: str):
         """
         Record a duration. Usage:
-        
+
         with zenbenchmark.record_duration('foobar_runtime'):
             foobar()   # measure this
-        
         """
         start = timeit.default_timer()
         yield
         end = timeit.default_timer()
 
-        self.results.record(self.request.node.name, metric_name, end - start, 's')
+        self.record(
+            metric_name=metric_name,
+            metric_value=end - start,
+            unit="s",
+            report=MetricReport.LOWER_IS_BETTER,
+        )
+
+    def record_pg_bench_result(self, pg_bench_result: PgBenchRunResult):
+        self.record("scale", pg_bench_result.scale, '', MetricReport.TEST_PARAM)
+        self.record("number_of_clients",
+                    pg_bench_result.number_of_clients,
+                    '',
+                    MetricReport.TEST_PARAM)
+        self.record("number_of_threads",
+                    pg_bench_result.number_of_threads,
+                    '',
+                    MetricReport.TEST_PARAM)
+        self.record(
+            "number_of_transactions_actually_processed",
+            pg_bench_result.number_of_transactions_actually_processed,
+            '',
+            # thats because this is predefined by test matrix and doesnt change across runs
+            report=MetricReport.TEST_PARAM,
+        )
+        self.record("latency_average",
+                    pg_bench_result.latency_average,
+                    unit="ms",
+                    report=MetricReport.LOWER_IS_BETTER)
+        self.record("latency_stddev",
+                    pg_bench_result.latency_stddev,
+                    unit="ms",
+                    report=MetricReport.LOWER_IS_BETTER)
+        self.record("tps_including_connection_time",
+                    pg_bench_result.tps_including_connection_time,
+                    '',
+                    report=MetricReport.HIGHER_IS_BETTER)
+        self.record("tps_excluding_connection_time",
+                    pg_bench_result.tps_excluding_connection_time,
+                    '',
+                    report=MetricReport.HIGHER_IS_BETTER)
+        self.record("init_duration",
+                    pg_bench_result.init_duration,
+                    unit="s",
+                    report=MetricReport.LOWER_IS_BETTER)
+        self.record("init_start_timestamp",
+                    pg_bench_result.init_start_timestamp,
+                    '',
+                    MetricReport.TEST_PARAM)
+        self.record("init_end_timestamp",
+                    pg_bench_result.init_end_timestamp,
+                    '',
+                    MetricReport.TEST_PARAM)
+        self.record("run_duration",
+                    pg_bench_result.run_duration,
+                    unit="s",
+                    report=MetricReport.LOWER_IS_BETTER)
+        self.record("run_start_timestamp",
+                    pg_bench_result.run_start_timestamp,
+                    '',
+                    MetricReport.TEST_PARAM)
+        self.record("run_end_timestamp",
+                    pg_bench_result.run_end_timestamp,
+                    '',
+                    MetricReport.TEST_PARAM)
 
     def get_io_writes(self, pageserver) -> int:
         """
@@ -149,7 +276,7 @@ class ZenithBenchmarker:
         assert matches
         return int(round(float(matches.group(1))))
 
-    def get_timeline_size(self, repo_dir: str, tenantid: str, timelineid: str):
+    def get_timeline_size(self, repo_dir: Path, tenantid: str, timelineid: str):
         """
         Calculate the on-disk size of a timeline
         """
@@ -171,47 +298,90 @@ class ZenithBenchmarker:
         yield
         after = self.get_io_writes(pageserver)
 
-        self.results.record(self.request.node.name,
-                            metric_name,
-                            round((after - before) / (1024 * 1024)),
-                            'MB')
+        self.record(metric_name,
+                    round((after - before) / (1024 * 1024)),
+                    "MB",
+                    report=MetricReport.LOWER_IS_BETTER)
 
 
-@pytest.fixture(scope='function')
-def zenbenchmark(zenbenchmark_global, request) -> Iterator[ZenithBenchmarker]:
+@pytest.fixture(scope="function")
+def zenbenchmark(record_property) -> Iterator[ZenithBenchmarker]:
     """
     This is a python decorator for benchmark fixtures. It contains functions for
     recording measurements, and prints them out at the end.
     """
-    benchmarker = ZenithBenchmarker(zenbenchmark_global, request)
+    benchmarker = ZenithBenchmarker(record_property)
     yield benchmarker
 
 
+def pytest_addoption(parser):
+    parser.addoption(
+        "--out-dir",
+        dest="out_dir",
+        help="Directory to ouput performance tests results to.",
+    )
+
+
+def get_out_path(target_dir: Path, revision: str) -> Path:
+    """
+    get output file path
+    if running in the CI uses commit revision
+    to avoid duplicates uses counter
+    """
+    # use UTC timestamp as a counter marker to avoid weird behaviour
+    # when for example files are deleted
+    ts = calendar.timegm(datetime.utcnow().utctimetuple())
+    path = target_dir / f"{ts}_{revision}.json"
+    assert not path.exists()
+    return path
+
+
 # Hook to print the results at the end
 @pytest.hookimpl(hookwrapper=True)
 def pytest_terminal_summary(terminalreporter: TerminalReporter, exitstatus: int, config: Config):
     yield
+    revision = os.getenv("GITHUB_SHA", "local")
+    platform = os.getenv("PLATFORM", "local")
 
-    global zenbenchmark_results
+    terminalreporter.section("Benchmark results", "-")
 
-    if not zenbenchmark_results:
+    result = []
+    for test_report in terminalreporter.stats.get("passed", []):
+        result_entry = []
+
+        for _, recorded_property in test_report.user_properties:
+            terminalreporter.write("{}.{}: ".format(test_report.head_line,
+                                                    recorded_property["name"]))
+            unit = recorded_property["unit"]
+            value = recorded_property["value"]
+            if unit == "MB":
+                terminalreporter.write("{0:,.0f}".format(value), green=True)
+            elif unit in ("s", "ms") and isinstance(value, float):
+                terminalreporter.write("{0:,.3f}".format(value), green=True)
+            elif isinstance(value, float):
+                terminalreporter.write("{0:,.4f}".format(value), green=True)
+            else:
+                terminalreporter.write(str(value), green=True)
+            terminalreporter.line(" {}".format(unit))
+
+            result_entry.append(recorded_property)
+
+        result.append({
+            "suit": test_report.nodeid,
+            "total_duration": test_report.duration,
+            "data": result_entry,
+        })
+
+    out_dir = config.getoption("out_dir")
+    if out_dir is None:
+        warnings.warn("no out dir provided to store performance test results")
         return
 
-    terminalreporter.section('Benchmark results', "-")
+    if not result:
+        warnings.warn("no results to store (no passed test suites)")
+        return
 
-    for result in zenbenchmark_results.results:
-        func = result[0]
-        metric_name = result[1]
-        metric_value = result[2]
-        unit = result[3]
-
-        terminalreporter.write("{}.{}: ".format(func, metric_name))
-
-        if unit == 'MB':
-            terminalreporter.write("{0:,.0f}".format(metric_value), green=True)
-        elif unit == 's':
-            terminalreporter.write("{0:,.3f}".format(metric_value), green=True)
-        else:
-            terminalreporter.write("{0:,.4f}".format(metric_value), green=True)
-
-        terminalreporter.line(" {}".format(unit))
+    get_out_path(Path(out_dir), revision=revision).write_text(
+        json.dumps({
+            "revision": revision, "platform": platform, "result": result
+        }, indent=4))

test_runner/fixtures/zenith_fixtures.py

@@ -6,19 +6,19 @@ import asyncpg
 import os
 import pathlib
 import uuid
+import warnings
 import jwt
 import json
 import psycopg2
 import pytest
 import shutil
-import signal
 import socket
 import subprocess
 import time
 import filecmp
 import tempfile
 
-from contextlib import closing, suppress
+from contextlib import closing
 from pathlib import Path
 from dataclasses import dataclass
 
@@ -26,6 +26,7 @@ from dataclasses import dataclass
 from psycopg2.extensions import connection as PgConnection
 from typing import Any, Callable, Dict, Iterator, List, Optional, TypeVar, cast
 from typing_extensions import Literal
+import pytest
 
 import requests
 
@@ -58,6 +59,16 @@ DEFAULT_POSTGRES_DIR = 'tmp_install'
 BASE_PORT = 15000
 WORKER_PORT_NUM = 100
 
+
+def pytest_addoption(parser):
+    parser.addoption(
+        "--skip-interfering-proc-check",
+        dest="skip_interfering_proc_check",
+        action="store_true",
+        help="skip check for interferring processes",
+    )
+
+
 # These are set in pytest_configure()
 base_dir = ""
 zenith_binpath = ""
@@ -65,14 +76,10 @@ pg_distrib_dir = ""
 top_output_dir = ""
 
 
-def pytest_configure(config):
-    """
-    Ensure that no unwanted daemons are running before we start testing.
-    Check that we do not owerflow available ports range.
-    """
-    numprocesses = config.getoption('numprocesses')
-    if numprocesses is not None and BASE_PORT + numprocesses * WORKER_PORT_NUM > 32768:  # do not use ephemeral ports
-        raise Exception('Too many workers configured. Cannot distrubute ports for services.')
+def check_interferring_processes(config):
+    if config.getoption("skip_interfering_proc_check"):
+        warnings.warn("interferring process check is skipped")
+        return
 
     # does not use -c as it is not supported on macOS
     cmd = ['pgrep', 'pageserver|postgres|safekeeper']
@@ -86,11 +93,36 @@ def pytest_configure(config):
             'Found interfering processes running. Stop all Zenith pageservers, nodes, safekeepers, as well as stand-alone Postgres.'
         )
 
+
+def pytest_configure(config):
+    """
+    Ensure that no unwanted daemons are running before we start testing.
+    Check that we do not owerflow available ports range.
+    """
+    check_interferring_processes(config)
+
+    numprocesses = config.getoption('numprocesses')
+    if numprocesses is not None and BASE_PORT + numprocesses * WORKER_PORT_NUM > 32768:  # do not use ephemeral ports
+        raise Exception('Too many workers configured. Cannot distrubute ports for services.')
+
     # find the base directory (currently this is the git root)
     global base_dir
     base_dir = os.path.normpath(os.path.join(get_self_dir(), '../..'))
     log.info(f'base_dir is {base_dir}')
 
+    # Compute the top-level directory for all tests.
+    global top_output_dir
+    env_test_output = os.environ.get('TEST_OUTPUT')
+    if env_test_output is not None:
+        top_output_dir = env_test_output
+    else:
+        top_output_dir = os.path.join(base_dir, DEFAULT_OUTPUT_DIR)
+    mkdir_if_needed(top_output_dir)
+
+    if os.getenv("REMOTE_ENV"):
+        # we are in remote env and do not have zenith binaries locally
+        # this is the case for benchmarks run on self-hosted runner
+        return
     # Find the zenith binaries.
     global zenith_binpath
     env_zenith_bin = os.environ.get('ZENITH_BIN')
@@ -100,7 +132,7 @@ def pytest_configure(config):
         zenith_binpath = os.path.join(base_dir, 'target/debug')
     log.info(f'zenith_binpath is {zenith_binpath}')
     if not os.path.exists(os.path.join(zenith_binpath, 'pageserver')):
-        raise Exception('zenith binaries not found at "{}"'.format(zenith_dir))
+        raise Exception('zenith binaries not found at "{}"'.format(zenith_binpath))
 
     # Find the postgres installation.
     global pg_distrib_dir
@@ -113,15 +145,6 @@ def pytest_configure(config):
     if not os.path.exists(os.path.join(pg_distrib_dir, 'bin/postgres')):
         raise Exception('postgres not found at "{}"'.format(pg_distrib_dir))
 
-    # Compute the top-level directory for all tests.
-    global top_output_dir
-    env_test_output = os.environ.get('TEST_OUTPUT')
-    if env_test_output is not None:
-        top_output_dir = env_test_output
-    else:
-        top_output_dir = os.path.join(base_dir, DEFAULT_OUTPUT_DIR)
-    mkdir_if_needed(top_output_dir)
-
 
 def zenfixture(func: Fn) -> Fn:
     """

test_runner/performance/test_bulk_insert.py

@@ -1,7 +1,7 @@
-import os
 from contextlib import closing
 from fixtures.zenith_fixtures import ZenithEnv
 from fixtures.log_helper import log
+from fixtures.benchmark_fixture import MetricReport, ZenithBenchmarker
 
 pytest_plugins = ("fixtures.zenith_fixtures", "fixtures.benchmark_fixture")
 
@@ -16,7 +16,7 @@ pytest_plugins = ("fixtures.zenith_fixtures", "fixtures.benchmark_fixture")
 # 3. Disk space used
 # 4. Peak memory usage
 #
-def test_bulk_insert(zenith_simple_env: ZenithEnv, zenbenchmark):
+def test_bulk_insert(zenith_simple_env: ZenithEnv, zenbenchmark: ZenithBenchmarker):
     env = zenith_simple_env
     # Create a branch for us
     env.zenith_cli(["branch", "test_bulk_insert", "empty"])
@@ -47,10 +47,16 @@ def test_bulk_insert(zenith_simple_env: ZenithEnv, zenbenchmark):
                     pscur.execute(f"do_gc {env.initial_tenant} {timeline} 0")
 
             # Record peak memory usage
-            zenbenchmark.record("peak_mem", zenbenchmark.get_peak_mem(env.pageserver) / 1024, 'MB')
+            zenbenchmark.record("peak_mem",
+                                zenbenchmark.get_peak_mem(env.pageserver) / 1024,
+                                'MB',
+                                report=MetricReport.LOWER_IS_BETTER)
 
             # Report disk space used by the repository
             timeline_size = zenbenchmark.get_timeline_size(env.repo_dir,
                                                            env.initial_tenant,
                                                            timeline)
-            zenbenchmark.record('size', timeline_size / (1024 * 1024), 'MB')
+            zenbenchmark.record('size',
+                                timeline_size / (1024 * 1024),
+                                'MB',
+                                report=MetricReport.LOWER_IS_BETTER)

test_runner/performance/test_bulk_tenant_create.py

@@ -1,4 +1,5 @@
 import timeit
+from fixtures.benchmark_fixture import MetricReport
 import pytest
 
 from fixtures.zenith_fixtures import ZenithEnvBuilder
@@ -54,4 +55,7 @@ def test_bulk_tenant_create(
 
         pg_tenant.stop()
 
-    zenbenchmark.record('tenant_creation_time', sum(time_slices) / len(time_slices), 's')
+    zenbenchmark.record('tenant_creation_time',
+                        sum(time_slices) / len(time_slices),
+                        's',
+                        report=MetricReport.LOWER_IS_BETTER)

test_runner/performance/test_gist_build.py

@@ -1,5 +1,6 @@
 import os
 from contextlib import closing
+from fixtures.benchmark_fixture import MetricReport
 from fixtures.zenith_fixtures import ZenithEnv
 from fixtures.log_helper import log
 
@@ -48,10 +49,16 @@ def test_gist_buffering_build(zenith_simple_env: ZenithEnv, zenbenchmark):
                     pscur.execute(f"do_gc {env.initial_tenant} {timeline} 1000000")
 
             # Record peak memory usage
-            zenbenchmark.record("peak_mem", zenbenchmark.get_peak_mem(env.pageserver) / 1024, 'MB')
+            zenbenchmark.record("peak_mem",
+                                zenbenchmark.get_peak_mem(env.pageserver) / 1024,
+                                'MB',
+                                report=MetricReport.LOWER_IS_BETTER)
 
             # Report disk space used by the repository
             timeline_size = zenbenchmark.get_timeline_size(env.repo_dir,
                                                            env.initial_tenant,
                                                            timeline)
-            zenbenchmark.record('size', timeline_size / (1024 * 1024), 'MB')
+            zenbenchmark.record('size',
+                                timeline_size / (1024 * 1024),
+                                'MB',
+                                report=MetricReport.LOWER_IS_BETTER)

test_runner/performance/test_perf_pgbench.py

@@ -1,6 +1,7 @@
-import os
 from contextlib import closing
-from fixtures.zenith_fixtures import ZenithEnv
+from fixtures.zenith_fixtures import PgBin, ZenithEnv
+
+from fixtures.benchmark_fixture import MetricReport, ZenithBenchmarker
 from fixtures.log_helper import log
 
 pytest_plugins = ("fixtures.zenith_fixtures", "fixtures.benchmark_fixture")
@@ -15,7 +16,7 @@ pytest_plugins = ("fixtures.zenith_fixtures", "fixtures.benchmark_fixture")
 # 2. Time to run 5000 pgbench transactions
 # 3. Disk space used
 #
-def test_pgbench(zenith_simple_env: ZenithEnv, pg_bin, zenbenchmark):
+def test_pgbench(zenith_simple_env: ZenithEnv, pg_bin: PgBin, zenbenchmark: ZenithBenchmarker):
     env = zenith_simple_env
     # Create a branch for us
     env.zenith_cli(["branch", "test_pgbench_perf", "empty"])
@@ -55,4 +56,7 @@ def test_pgbench(zenith_simple_env: ZenithEnv, pg_bin, zenbenchmark):
 
     # Report disk space used by the repository
     timeline_size = zenbenchmark.get_timeline_size(env.repo_dir, env.initial_tenant, timeline)
-    zenbenchmark.record('size', timeline_size / (1024 * 1024), 'MB')
+    zenbenchmark.record('size',
+                        timeline_size / (1024 * 1024),
+                        'MB',
+                        report=MetricReport.LOWER_IS_BETTER)

test_runner/performance/test_perf_pgbench_remote.py

@@ -0,0 +1,125 @@
+import dataclasses
+import os
+import subprocess
+from typing import List
+from fixtures.benchmark_fixture import PgBenchRunResult, ZenithBenchmarker
+import pytest
+from datetime import datetime
+import calendar
+import timeit
+import os
+
+pytest_plugins = ("fixtures.benchmark_fixture", )
+
+
+def utc_now_timestamp() -> int:
+    return calendar.timegm(datetime.utcnow().utctimetuple())
+
+
+@dataclasses.dataclass
+class PgBenchRunner:
+    connstr: str
+    scale: int
+    transactions: int
+    pgbench_bin_path: str = "pgbench"
+
+    def invoke(self, args: List[str]) -> 'subprocess.CompletedProcess[str]':
+        return subprocess.run([self.pgbench_bin_path, *args],
+                              check=True,
+                              text=True,
+                              capture_output=True)
+
+    def init(self, vacuum: bool = True) -> 'subprocess.CompletedProcess[str]':
+        args = []
+        if not vacuum:
+            args.append("--no-vacuum")
+        args.extend([f"--scale={self.scale}", "--initialize", self.connstr])
+        return self.invoke(args)
+
+    def run(self, jobs: int = 1, clients: int = 1):
+        return self.invoke([
+            f"--transactions={self.transactions}",
+            f"--jobs={jobs}",
+            f"--client={clients}",
+            "--progress=2",  # print progress every two seconds
+            self.connstr,
+        ])
+
+
+@pytest.fixture
+def connstr():
+    res = os.getenv("BENCHMARK_CONNSTR")
+    if res is None:
+        raise ValueError("no connstr provided, use BENCHMARK_CONNSTR environment variable")
+    return res
+
+
+def get_transactions_matrix():
+    transactions = os.getenv("TEST_PG_BENCH_TRANSACTIONS_MATRIX")
+    if transactions is None:
+        return [10**4, 10**5]
+    return list(map(int, transactions.split(",")))
+
+
+def get_scales_matrix():
+    scales = os.getenv("TEST_PG_BENCH_SCALES_MATRIX")
+    if scales is None:
+        return [10, 20]
+    return list(map(int, scales.split(",")))
+
+
+@pytest.mark.parametrize("scale", get_scales_matrix())
+@pytest.mark.parametrize("transactions", get_transactions_matrix())
+@pytest.mark.remote_cluster
+def test_pg_bench_remote_cluster(zenbenchmark: ZenithBenchmarker,
+                                 connstr: str,
+                                 scale: int,
+                                 transactions: int):
+    """
+    The best way is to run same pack of tests both, for local zenith
+    and against staging, but currently local tests heavily depend on
+    things available only locally e.g. zenith binaries, pageserver api, etc.
+    Also separate test allows to run pgbench workload against vanilla postgres
+    or other systems that support postgres protocol.
+
+    Also now this is more of a liveness test because it stresses pageserver internals,
+    so we clearly see what goes wrong in more "real" environment.
+    """
+    pg_bin = os.getenv("PG_BIN")
+    if pg_bin is not None:
+        pgbench_bin_path = os.path.join(pg_bin, "pgbench")
+    else:
+        pgbench_bin_path = "pgbench"
+
+    runner = PgBenchRunner(
+        connstr=connstr,
+        scale=scale,
+        transactions=transactions,
+        pgbench_bin_path=pgbench_bin_path,
+    )
+    # calculate timestamps and durations separately
+    # timestamp is intended to be used for linking to grafana and logs
+    # duration is actually a metric and uses float instead of int for timestamp
+    init_start_timestamp = utc_now_timestamp()
+    t0 = timeit.default_timer()
+    runner.init()
+    init_duration = timeit.default_timer() - t0
+    init_end_timestamp = utc_now_timestamp()
+
+    run_start_timestamp = utc_now_timestamp()
+    t0 = timeit.default_timer()
+    out = runner.run()  # TODO handle failures
+    run_duration = timeit.default_timer() - t0
+    run_end_timestamp = utc_now_timestamp()
+
+    res = PgBenchRunResult.parse_from_output(
+        out=out,
+        init_duration=init_duration,
+        init_start_timestamp=init_start_timestamp,
+        init_end_timestamp=init_end_timestamp,
+        run_duration=run_duration,
+        run_start_timestamp=run_start_timestamp,
+        run_end_timestamp=run_end_timestamp,
+    )
+
+    zenbenchmark.record_pg_bench_result(res)

test_runner/performance/test_small_seqscans.py

@@ -0,0 +1,43 @@
+# Test sequential scan speed
+#
+# The test table is large enough (3-4 MB) that it doesn't fit in the compute node
+# cache, so the seqscans go to the page server. But small enough that it fits
+# into memory in the page server.
+from contextlib import closing
+from fixtures.zenith_fixtures import ZenithEnv
+from fixtures.log_helper import log
+from fixtures.benchmark_fixture import MetricReport, ZenithBenchmarker
+
+pytest_plugins = ("fixtures.zenith_fixtures", "fixtures.benchmark_fixture")
+
+
+def test_small_seqscans(zenith_simple_env: ZenithEnv, zenbenchmark: ZenithBenchmarker):
+    env = zenith_simple_env
+    # Create a branch for us
+    env.zenith_cli(["branch", "test_small_seqscans", "empty"])
+
+    pg = env.postgres.create_start('test_small_seqscans')
+    log.info("postgres is running on 'test_small_seqscans' branch")
+
+    # Open a connection directly to the page server that we'll use to force
+    # flushing the layers to disk
+    psconn = env.pageserver.connect()
+    pscur = psconn.cursor()
+
+    with closing(pg.connect()) as conn:
+        with conn.cursor() as cur:
+            cur.execute('create table t (i integer);')
+            cur.execute('insert into t values (generate_series(1,100000));')
+
+            # Verify that the table is larger than shared_buffers
+            cur.execute('''
+            select setting::int * pg_size_bytes(unit) as shared_buffers, pg_relation_size('t') as tbl_ize
+            from pg_settings where name = 'shared_buffers'
+            ''')
+            row = cur.fetchone()
+            log.info(f"shared_buffers is {row[0]}, table size {row[1]}")
+            assert int(row[0]) < int(row[1])
+
+            with zenbenchmark.record_duration('run'):
+                for i in range(1000):
+                    cur.execute('select count(*) from t;')

test_runner/performance/test_write_amplification.py

@@ -12,6 +12,7 @@
 # Amplification problem at its finest.
 import os
 from contextlib import closing
+from fixtures.benchmark_fixture import MetricReport
 from fixtures.zenith_fixtures import ZenithEnv
 from fixtures.log_helper import log
 
@@ -76,4 +77,7 @@ def test_write_amplification(zenith_simple_env: ZenithEnv, zenbenchmark):
             timeline_size = zenbenchmark.get_timeline_size(env.repo_dir,
                                                            env.initial_tenant,
                                                            timeline)
-            zenbenchmark.record('size', timeline_size / (1024 * 1024), 'MB')
+            zenbenchmark.record('size',
+                                timeline_size / (1024 * 1024),
+                                'MB',
+                                report=MetricReport.LOWER_IS_BETTER)

test_runner/pytest.ini

@@ -1,4 +1,8 @@
 [pytest]
+addopts =
+    -m 'not remote_cluster'
+markers =
+    remote_cluster
 minversion = 6.0
 log_format = %(asctime)s.%(msecs)-3d %(levelname)s [%(filename)s:%(lineno)d] %(message)s
 log_date_format = %Y-%m-%d %H:%M:%S

walkeeper/src/bin/safekeeper.rs

@@ -10,7 +10,7 @@ use std::env;
 use std::path::{Path, PathBuf};
 use std::thread;
 use zenith_utils::http::endpoint;
-use zenith_utils::{logging, tcp_listener};
+use zenith_utils::{logging, tcp_listener, GIT_VERSION};
 
 use walkeeper::defaults::{DEFAULT_HTTP_LISTEN_ADDR, DEFAULT_PG_LISTEN_ADDR};
 use walkeeper::http;
@@ -22,6 +22,7 @@ fn main() -> Result<()> {
     zenith_metrics::set_common_metrics_prefix("safekeeper");
     let arg_matches = App::new("Zenith safekeeper")
         .about("Store WAL stream to local file system and push it to WAL receivers")
+        .version(GIT_VERSION)
         .arg(
             Arg::with_name("datadir")
                 .short("D")
@@ -131,6 +132,8 @@ fn main() -> Result<()> {
 fn start_safekeeper(conf: SafeKeeperConf) -> Result<()> {
     let log_file = logging::init("safekeeper.log", conf.daemonize)?;
 
+    info!("version: {}", GIT_VERSION);
+
     let http_listener = tcp_listener::bind(conf.listen_http_addr.clone()).map_err(|e| {
         error!("failed to bind to address {}: {}", conf.listen_http_addr, e);
         e

walkeeper/src/receive_wal.rs

@@ -133,9 +133,12 @@ impl<'pg> ReceiveWalConn<'pg> {
             // Add far as replication in postgres is initiated by receiver, we should use callme mechanism
             let conf = swh.conf.clone();
             let timelineid = swh.timeline.get().timelineid;
-            thread::spawn(move || {
-                request_callback(conf, timelineid, tenant_id);
-            });
+            let _ = thread::Builder::new()
+                .name("request_callback thread".into())
+                .spawn(move || {
+                    request_callback(conf, timelineid, tenant_id);
+                })
+                .unwrap();
         }
 
         loop {

walkeeper/src/replication.rs

@@ -130,16 +130,16 @@ impl ReplicationConn {
         Ok(())
     }
 
-    /// Helper function that parses a pair of LSNs.
-    fn parse_start_stop(cmd: &[u8]) -> Result<(Lsn, Lsn)> {
+    /// Helper function that parses a single LSN.
+    fn parse_start(cmd: &[u8]) -> Result<Lsn> {
         let re = Regex::new(r"([[:xdigit:]]+/[[:xdigit:]]+)").unwrap();
         let caps = re.captures_iter(str::from_utf8(cmd)?);
         let mut lsns = caps.map(|cap| cap[1].parse::<Lsn>());
         let start_pos = lsns
             .next()
             .ok_or_else(|| anyhow!("Failed to parse start LSN from command"))??;
-        let stop_pos = lsns.next().transpose()?.unwrap_or(Lsn(0));
-        Ok((start_pos, stop_pos))
+        assert!(lsns.next().is_none());
+        Ok(start_pos)
     }
 
     /// Helper function for opening a wal file.
@@ -173,13 +173,16 @@ impl ReplicationConn {
         let bg_timeline = Arc::clone(swh.timeline.get());
         let bg_stream_in = self.stream_in.take().unwrap();
 
-        thread::spawn(move || {
-            if let Err(err) = Self::background_thread(bg_stream_in, bg_timeline) {
-                error!("Replication background thread failed: {}", err);
-            }
-        });
+        let _ = thread::Builder::new()
+            .name("HotStandbyFeedback thread".into())
+            .spawn(move || {
+                if let Err(err) = Self::background_thread(bg_stream_in, bg_timeline) {
+                    error!("Replication background thread failed: {}", err);
+                }
+            })
+            .unwrap();
 
-        let (mut start_pos, mut stop_pos) = Self::parse_start_stop(cmd)?;
+        let mut start_pos = Self::parse_start(cmd)?;
 
         let mut wal_seg_size: usize;
         loop {
@@ -191,14 +194,8 @@ impl ReplicationConn {
                 break;
             }
         }
-        let (wal_end, timeline) = swh.timeline.get().get_end_of_wal();
-        if start_pos == Lsn(0) {
-            start_pos = wal_end;
-        }
-        if stop_pos == Lsn(0) && swh.appname == Some("wal_proposer_recovery".to_string()) {
-            stop_pos = wal_end;
-        }
-        info!("Start replication from {} till {}", start_pos, stop_pos);
+        let (_, timeline) = swh.timeline.get().get_end_of_wal();
+        info!("Start replication from {}", start_pos);
 
         // switch to copy
         pgb.write_message(&BeMessage::CopyBothResponse)?;
@@ -208,28 +205,17 @@ impl ReplicationConn {
 
         loop {
             /* Wait until we have some data to stream */
-            if stop_pos != Lsn(0) {
-                /* recovery mode: stream up to the specified LSN (VCL) */
-                if start_pos >= stop_pos {
-                    /* recovery finished */
-                    break;
-                }
-                end_pos = stop_pos;
+            if let Some(lsn) = swh.timeline.get().wait_for_lsn(start_pos) {
+                end_pos = lsn
             } else {
-                /* normal mode */
-                let timeline = swh.timeline.get();
-                if let Some(lsn) = timeline.wait_for_lsn(start_pos) {
-                    end_pos = lsn
-                } else {
-                    // timeout expired: request pageserver status
-                    pgb.write_message(&BeMessage::KeepAlive(WalSndKeepAlive {
-                        sent_ptr: end_pos.0,
-                        timestamp: get_current_timestamp(),
-                        request_reply: true,
-                    }))
-                    .context("Failed to send KeepAlive message")?;
-                    continue;
-                }
+                // timeout expired: request pageserver status
+                pgb.write_message(&BeMessage::KeepAlive(WalSndKeepAlive {
+                    sent_ptr: end_pos.0,
+                    timestamp: get_current_timestamp(),
+                    request_reply: true,
+                }))
+                .context("Failed to send KeepAlive message")?;
+                continue;
             }
             if end_pos == END_REPLICATION_MARKER {
                 break;

walkeeper/src/wal_service.rs

@@ -18,11 +18,15 @@ pub fn thread_main(conf: SafeKeeperConf, listener: TcpListener) -> Result<()> {
             Ok((socket, peer_addr)) => {
                 debug!("accepted connection from {}", peer_addr);
                 let conf = conf.clone();
-                thread::spawn(move || {
-                    if let Err(err) = handle_socket(socket, conf) {
-                        error!("connection handler exited: {}", err);
-                    }
-                });
+
+                let _ = thread::Builder::new()
+                    .name("WAL service thread".into())
+                    .spawn(move || {
+                        if let Err(err) = handle_socket(socket, conf) {
+                            error!("connection handler exited: {}", err);
+                        }
+                    })
+                    .unwrap();
             }
             Err(e) => error!("Failed to accept connection: {}", e),
         }

zenith/src/main.rs

@@ -20,6 +20,7 @@ use walkeeper::defaults::{
 use zenith_utils::auth::{Claims, Scope};
 use zenith_utils::postgres_backend::AuthType;
 use zenith_utils::zid::{ZTenantId, ZTimelineId};
+use zenith_utils::GIT_VERSION;
 
 use pageserver::branches::BranchInfo;
 
@@ -103,6 +104,7 @@ fn main() -> Result<()> {
 
     let matches = App::new("Zenith CLI")
         .setting(AppSettings::ArgRequiredElseHelp)
+        .version(GIT_VERSION)
         .subcommand(
             SubCommand::with_name("init")
                 .about("Initialize a new Zenith repository")

zenith_utils/Cargo.toml

@@ -30,6 +30,8 @@ hex = { version = "0.4.3", features = ["serde"] }
 rustls = "0.19.1"
 rustls-split = "0.2.1"
 
+git-version = "0.3.5"
+
 [dev-dependencies]
 hex-literal = "0.3"
 bytes = "1.0"

zenith_utils/build.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("cargo:rerun-if-env-changed=GIT_VERSION");
+}

zenith_utils/src/chunked_buffer.rs

@@ -0,0 +1,173 @@
+use std::convert::TryInto;
+
+const CHUNK_SIZE: usize = 1024;
+
+///
+/// ChunkedBuffer is an expandable byte buffer. You can append to the end of it, and you
+/// can read at any byte position. The advantage over a plain Vec<u8> is that the
+/// buffer consists of smaller chunks, so that a big buffer doesn't require one huge
+/// allocation, and expanding doesn't require copying all the data.
+///
+#[derive(Debug, Default)]
+pub struct ChunkedBuffer {
+    // Clippy considers it unnecessary to have Box here, since the vector is stored
+    // on the heap anyway. But we have our reasons for that: we want each chunk to
+    // be allocated separately, so that we don't require one huge allocation, and so
+    // that expanding the array doesn't require copying all of the data. (Clippy
+    // knows about that and doesn't complain if the array is large enough, 4096
+    // bytes is the default threshold. Our chunk size is smaller than that so that
+    // heuristic doesn't save us.)
+    #[allow(clippy::vec_box)]
+    chunks: Vec<Box<[u8; CHUNK_SIZE]>>,
+
+    length: usize,
+}
+
+#[derive(Debug, PartialEq, Eq)]
+pub struct ChunkToken {
+    start: usize,
+    length: usize,
+}
+
+impl ChunkedBuffer {
+    pub fn read(&self, token: &ChunkToken) -> Vec<u8> {
+        let mut buf = Vec::with_capacity(token.length);
+
+        let chunk_idx = token.start / CHUNK_SIZE;
+        let mut chunk_iter = self.chunks[chunk_idx..].iter();
+        let mut bytes_remaining = token.length;
+        if bytes_remaining > 0 {
+            let start_offset = token.start % CHUNK_SIZE;
+            let chunk_bytes = &chunk_iter.next().unwrap()[start_offset..];
+            let bytes = &chunk_bytes[..chunk_bytes.len().min(bytes_remaining)];
+            buf.extend_from_slice(bytes);
+            bytes_remaining -= bytes.len();
+        }
+
+        while bytes_remaining > 0 {
+            let chunk_bytes = chunk_iter.next().unwrap();
+            let bytes = &chunk_bytes[..chunk_bytes.len().min(bytes_remaining)];
+            buf.extend_from_slice(bytes);
+            bytes_remaining -= bytes.len();
+        }
+
+        debug_assert_eq!(buf.len(), token.length);
+        buf
+    }
+
+    pub fn write(&mut self, mut buf: &[u8]) -> ChunkToken {
+        let token = ChunkToken {
+            start: self.length,
+            length: buf.len(),
+        };
+
+        while !buf.is_empty() {
+            let chunk_idx = self.length / CHUNK_SIZE;
+            let chunk = match self.chunks.get_mut(chunk_idx) {
+                Some(chunk) => chunk,
+                None => {
+                    debug_assert_eq!(self.length % CHUNK_SIZE, 0);
+                    debug_assert_eq!(chunk_idx, self.chunks.len());
+                    self.chunks.push(heap_alloc_chunk());
+                    self.chunks.last_mut().unwrap()
+                }
+            };
+
+            let offset = self.length % CHUNK_SIZE;
+            let length = buf.len().min(CHUNK_SIZE - offset);
+
+            chunk[offset..][..length].copy_from_slice(&buf[..length]);
+            buf = &buf[length..];
+            self.length += length;
+        }
+
+        token
+    }
+}
+
+fn heap_alloc_chunk() -> Box<[u8; CHUNK_SIZE]> {
+    vec![0u8; CHUNK_SIZE].into_boxed_slice().try_into().unwrap()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{ChunkToken, ChunkedBuffer, CHUNK_SIZE};
+
+    fn gen_bytes(len: usize) -> Vec<u8> {
+        let mut buf = vec![0u8; len];
+        for (idx, val) in buf.iter_mut().enumerate() {
+            *val = idx as u8;
+        }
+
+        buf
+    }
+
+    #[test]
+    fn one_item() {
+        fn test(data: &[u8]) {
+            let mut chunked_buffer = ChunkedBuffer::default();
+            let token = chunked_buffer.write(data);
+            assert_eq!(
+                ChunkToken {
+                    start: 0,
+                    length: data.len(),
+                },
+                token
+            );
+
+            let buf = chunked_buffer.read(&token);
+            assert_eq!(data, buf.as_slice());
+        }
+
+        test(b"");
+        test(b"a");
+        test(b"abc");
+
+        test(&gen_bytes(CHUNK_SIZE - 1));
+        test(&gen_bytes(CHUNK_SIZE));
+        test(&gen_bytes(CHUNK_SIZE + 1));
+
+        test(&gen_bytes(2 * CHUNK_SIZE - 1));
+        test(&gen_bytes(2 * CHUNK_SIZE));
+        test(&gen_bytes(2 * CHUNK_SIZE + 1));
+    }
+
+    #[test]
+    fn many_items() {
+        let mut chunked_buffer = ChunkedBuffer::default();
+        let mut start = 0;
+        let mut expected = Vec::new();
+
+        let mut test = |data: &[u8]| {
+            let token = chunked_buffer.write(data);
+            assert_eq!(
+                ChunkToken {
+                    start,
+                    length: data.len(),
+                },
+                token
+            );
+
+            expected.push((token, data.to_vec()));
+
+            for (token, expected_data) in &expected {
+                let buf = chunked_buffer.read(token);
+                assert_eq!(expected_data, buf.as_slice());
+            }
+
+            start += data.len();
+        };
+
+        test(b"abc");
+        test(b"");
+        test(b"a");
+
+        test(&gen_bytes(CHUNK_SIZE - 1));
+        test(&gen_bytes(CHUNK_SIZE));
+        test(&gen_bytes(CHUNK_SIZE + 1));
+
+        test(&gen_bytes(2 * CHUNK_SIZE - 1));
+        test(&gen_bytes(2 * CHUNK_SIZE));
+        test(&gen_bytes(2 * CHUNK_SIZE + 1));
+    }
+}

zenith_utils/src/lib.rs

@@ -11,6 +11,9 @@ pub mod seqwait;
 /// append only ordered map implemented with a Vec
 pub mod vec_map;
 
+/// expandable byte buffer consisting of fixed-size chunks
+pub mod chunked_buffer;
+
 // Async version of SeqWait. Currently unused.
 // pub mod seqwait_async;
 
@@ -43,3 +46,34 @@ pub mod accum;
 
 // Utility for binding TcpListeners with proper socket options.
 pub mod tcp_listener;
+
+// Utility for putting a raw file descriptor into non-blocking mode
+pub mod nonblock;
+
+// This is a shortcut to embed git sha into binaries and avoid copying the same build script to all packages
+//
+// we have several cases:
+// * building locally from git repo
+// * building in CI from git repo
+// * building in docker (either in CI or locally)
+//
+// One thing to note is that .git is not available in docker (and it is bad to include it there).
+// So everything becides docker build is covered by git_version crate.
+// For docker use environment variable to pass git version, which is then retrieved by buildscript (build.rs).
+// It takes variable from build process env and puts it to the rustc env. And then we can retrieve it here by using env! macro.
+// Git version received from environment variable used as a fallback in git_version invokation.
+// And to avoid running buildscript every recompilation, we use rerun-if-env-changed option.
+// So the build script will be run only when GIT_VERSION envvar has changed.
+//
+// Why not to use buildscript to get git commit sha directly without procmacro from different crate?
+// Caching and workspaces complicates that. In case zenith_utils is not
+// recompiled due to caching then version may become outdated.
+// git_version crate handles that case by introducing a dependency on .git internals via include_bytes! macro,
+// so if we changed the index state git_version will pick that up and rerun the macro.
+//
+// Note that with git_version prefix is `git:` and in case of git version from env its `git-env:`.
+use git_version::git_version;
+pub const GIT_VERSION: &str = git_version!(
+    prefix = "git:",
+    fallback = concat!("git-env:", env!("GIT_VERSION"))
+);

zenith_utils/src/nonblock.rs

@@ -0,0 +1,17 @@
+use nix::fcntl::{fcntl, OFlag, F_GETFL, F_SETFL};
+use std::os::unix::io::RawFd;
+
+/// Put a file descriptor into non-blocking mode
+pub fn set_nonblock(fd: RawFd) -> Result<(), std::io::Error> {
+    let bits = fcntl(fd, F_GETFL)?;
+
+    // Safety: If F_GETFL returns some unknown bits, they should be valid
+    // for passing back to F_SETFL, too. If we left them out, the F_SETFL
+    // would effectively clear them, which is not what we want.
+    let mut flags = unsafe { OFlag::from_bits_unchecked(bits) };
+    flags |= OFlag::O_NONBLOCK;
+
+    fcntl(fd, F_SETFL(flags))?;
+
+    Ok(())
+}