add a flag to avoid non incremental size calculation in pageserver http api

This calculation is not that heavy but it is needed only in tests, and
in case the number of tenants/timelines is high the calculation can take
noticeable time.

Resolves https://github.com/zenithdb/zenith/issues/804

Cargo.lock

@@ -1213,6 +1213,7 @@ dependencies = [
  "tokio",
  "toml",
  "tracing",
+ "url",
  "workspace_hack",
  "zenith_metrics",
  "zenith_utils",

pageserver/Cargo.toml

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

pageserver/src/branches.rs

@@ -35,7 +35,7 @@ pub struct BranchInfo {
     pub ancestor_id: Option<String>,
     pub ancestor_lsn: Option<String>,
     pub current_logical_size: usize,
-    pub current_logical_size_non_incremental: usize,
+    pub current_logical_size_non_incremental: Option<usize>,
 }
 
 impl BranchInfo {
@@ -44,6 +44,7 @@ impl BranchInfo {
         conf: &PageServerConf,
         tenantid: &ZTenantId,
         repo: &Arc<dyn Repository>,
+        include_non_incremental_logical_size: bool,
     ) -> Result<Self> {
         let name = path
             .as_ref()
@@ -78,6 +79,14 @@ impl BranchInfo {
             );
         }
 
+        // non incremental size calculation can be heavy, so let it be optional
+        // needed for tests to check size calculation
+        let current_logical_size_non_incremental = include_non_incremental_logical_size
+            .then(|| {
+                timeline.get_current_logical_size_non_incremental(timeline.get_last_record_lsn())
+            })
+            .transpose()?;
+
         Ok(BranchInfo {
             name,
             timeline_id,
@@ -85,8 +94,7 @@ impl BranchInfo {
             ancestor_id,
             ancestor_lsn,
             current_logical_size: timeline.get_current_logical_size(),
-            current_logical_size_non_incremental: timeline
-                .get_current_logical_size_non_incremental(timeline.get_last_record_lsn())?,
+            current_logical_size_non_incremental,
         })
     }
 }
@@ -248,7 +256,11 @@ fn bootstrap_timeline(
     Ok(())
 }
 
-pub(crate) fn get_branches(conf: &PageServerConf, tenantid: &ZTenantId) -> Result<Vec<BranchInfo>> {
+pub(crate) fn get_branches(
+    conf: &PageServerConf,
+    tenantid: &ZTenantId,
+    include_non_incremental_logical_size: bool,
+) -> Result<Vec<BranchInfo>> {
     let repo = tenant_mgr::get_repository_for_tenant(*tenantid)?;
 
     // Each branch has a corresponding record (text file) in the refs/branches
@@ -258,7 +270,13 @@ pub(crate) fn get_branches(conf: &PageServerConf, tenantid: &ZTenantId) -> Resul
     std::fs::read_dir(&branches_dir)?
         .map(|dir_entry_res| {
             let dir_entry = dir_entry_res?;
-            BranchInfo::from_path(dir_entry.path(), conf, tenantid, &repo)
+            BranchInfo::from_path(
+                dir_entry.path(),
+                conf,
+                tenantid,
+                &repo,
+                include_non_incremental_logical_size,
+            )
         })
         .collect()
 }
@@ -320,7 +338,7 @@ pub(crate) fn create_branch(
         ancestor_id: None,
         ancestor_lsn: None,
         current_logical_size: 0,
-        current_logical_size_non_incremental: 0,
+        current_logical_size_non_incremental: Some(0),
     })
 }
 

pageserver/src/http/openapi_spec.yml

@@ -25,6 +25,11 @@ paths:
         schema:
           type: string
           format: hex
+      - name: include-non-incremental-logical-size
+        in: query
+        schema:
+          type: string
+          description: Controls calculation of current_logical_size_non_incremental
     get:
       description: Get branches for tenant
       responses:
@@ -73,6 +78,11 @@ paths:
         required: true
         schema:
           type: string
+      - name: include-non-incremental-logical-size
+        in: query
+        schema:
+          type: string
+          description: Controls calculation of current_logical_size_non_incremental
     get:
       description: Get branches for tenant
       responses:
@@ -260,7 +270,6 @@ components:
         - timeline_id
         - latest_valid_lsn
         - current_logical_size
-        - current_logical_size_non_incremental
       properties:
         name:
           type: string

pageserver/src/http/routes.rs

@@ -86,31 +86,60 @@ async fn branch_create_handler(mut request: Request<Body>) -> Result<Response<Bo
     Ok(json_response(StatusCode::CREATED, response_data)?)
 }
 
+// Gate non incremental logical size calculation behind a flag
+// after pgbench -i -s100 calculation took 28ms so if multiplied by the number of timelines
+// and tenants it can take noticeable amount of time. Also the value currently used only in tests
+fn get_include_non_incremental_logical_size(request: &Request<Body>) -> bool {
+    request
+        .uri()
+        .query()
+        .map(|v| {
+            url::form_urlencoded::parse(v.as_bytes())
+                .into_owned()
+                .position(|(param, _)| param == "include-non-incremental-logical-size")
+        })
+        .flatten()
+        .is_some()
+}
+
 async fn branch_list_handler(request: Request<Body>) -> Result<Response<Body>, ApiError> {
     let tenantid: ZTenantId = parse_request_param(&request, "tenant_id")?;
 
+    let include_non_incremental_logical_size = get_include_non_incremental_logical_size(&request);
+
     check_permission(&request, Some(tenantid))?;
 
     let response_data = tokio::task::spawn_blocking(move || {
         let _enter = info_span!("branch_list", tenant = %tenantid).entered();
-        crate::branches::get_branches(get_config(&request), &tenantid)
+        crate::branches::get_branches(
+            get_config(&request),
+            &tenantid,
+            include_non_incremental_logical_size,
+        )
     })
     .await
     .map_err(ApiError::from_err)??;
     Ok(json_response(StatusCode::OK, response_data)?)
 }
 
-// TODO add to swagger
 async fn branch_detail_handler(request: Request<Body>) -> Result<Response<Body>, ApiError> {
     let tenantid: ZTenantId = parse_request_param(&request, "tenant_id")?;
     let branch_name: String = get_request_param(&request, "branch_name")?.to_string();
     let conf = get_state(&request).conf;
     let path = conf.branch_path(&branch_name, &tenantid);
 
+    let include_non_incremental_logical_size = get_include_non_incremental_logical_size(&request);
+
     let response_data = tokio::task::spawn_blocking(move || {
         let _enter = info_span!("branch_detail", tenant = %tenantid, branch=%branch_name).entered();
         let repo = tenant_mgr::get_repository_for_tenant(tenantid)?;
-        BranchInfo::from_path(path, conf, &tenantid, &repo)
+        BranchInfo::from_path(
+            path,
+            conf,
+            &tenantid,
+            &repo,
+            include_non_incremental_logical_size,
+        )
     })
     .await
     .map_err(ApiError::from_err)??;

pageserver/src/page_service.rs

@@ -630,7 +630,7 @@ impl postgres_backend::Handler for PageServerHandler {
 
             let tenantid = ZTenantId::from_str(caps.get(1).unwrap().as_str())?;
 
-            let branches = crate::branches::get_branches(self.conf, &tenantid)?;
+            let branches = crate::branches::get_branches(self.conf, &tenantid, true)?;
             let branches_buf = serde_json::to_vec(&branches)?;
 
             pgb.write_message_noflush(&SINGLE_COL_ROWDESC)?

test_runner/fixtures/zenith_fixtures.py

@@ -610,7 +610,9 @@ class ZenithPageserverHttpClient(requests.Session):
         return res_json
 
     def branch_detail(self, tenant_id: uuid.UUID, name: str) -> Dict[Any, Any]:
-        res = self.get(f"http://localhost:{self.port}/v1/branch/{tenant_id.hex}/{name}", )
+        res = self.get(
+            f"http://localhost:{self.port}/v1/branch/{tenant_id.hex}/{name}?include-non-incremental-logical-size=1",
+        )
         res.raise_for_status()
         res_json = res.json()
         assert isinstance(res_json, dict)