From 5bc09074ea61267cf66b08a378397d753c4f4000 Mon Sep 17 00:00:00 2001 From: Dmitry Rodionov Date: Wed, 27 Oct 2021 00:55:32 +0300 Subject: [PATCH] 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 | 1 + pageserver/Cargo.toml | 1 + pageserver/src/branches.rs | 30 ++++++++++++++++----- pageserver/src/http/openapi_spec.yml | 11 +++++++- pageserver/src/http/routes.rs | 35 ++++++++++++++++++++++--- pageserver/src/page_service.rs | 2 +- test_runner/fixtures/zenith_fixtures.py | 4 ++- 7 files changed, 72 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee83de2118..86e2ac3ba3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1213,6 +1213,7 @@ dependencies = [ "tokio", "toml", "tracing", + "url", "workspace_hack", "zenith_metrics", "zenith_utils", diff --git a/pageserver/Cargo.toml b/pageserver/Cargo.toml index d696f72285..100e6551c2 100644 --- a/pageserver/Cargo.toml +++ b/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" } diff --git a/pageserver/src/branches.rs b/pageserver/src/branches.rs index ba922acdb7..6c0e808d12 100644 --- a/pageserver/src/branches.rs +++ b/pageserver/src/branches.rs @@ -35,7 +35,7 @@ pub struct BranchInfo { pub ancestor_id: Option, pub ancestor_lsn: Option, pub current_logical_size: usize, - pub current_logical_size_non_incremental: usize, + pub current_logical_size_non_incremental: Option, } impl BranchInfo { @@ -44,6 +44,7 @@ impl BranchInfo { conf: &PageServerConf, tenantid: &ZTenantId, repo: &Arc, + include_non_incremental_logical_size: bool, ) -> Result { 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> { +pub(crate) fn get_branches( + conf: &PageServerConf, + tenantid: &ZTenantId, + include_non_incremental_logical_size: bool, +) -> Result> { 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), }) } diff --git a/pageserver/src/http/openapi_spec.yml b/pageserver/src/http/openapi_spec.yml index d3c017d832..cc74f2d057 100644 --- a/pageserver/src/http/openapi_spec.yml +++ b/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 diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 0ff39b1253..01f1514f0c 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -86,31 +86,60 @@ async fn branch_create_handler(mut request: Request) -> Result) -> 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) -> Result, 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) -> Result, 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)??; diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index d78974aebb..d7c0b1b2ee 100644 --- a/pageserver/src/page_service.rs +++ b/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)? diff --git a/test_runner/fixtures/zenith_fixtures.py b/test_runner/fixtures/zenith_fixtures.py index 2fdf2f0c13..4622cf64d4 100644 --- a/test_runner/fixtures/zenith_fixtures.py +++ b/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)