From 15d1f85552231a5f6d4d500ac75010669dbec540 Mon Sep 17 00:00:00 2001 From: Dmitry Rodionov Date: Thu, 13 Apr 2023 12:11:43 +0300 Subject: [PATCH] Add reason to TenantState::Broken (#3954) Reason and backtrace are added to the Broken state. Backtrace is automatically collected when tenant entered the broken state. The format for API, CLI and metrics is changed and unified to return tenant state name in camel case. Previously snake case was used for metrics and camel case was used for everything else. Now tenant state field in TenantInfo swagger spec is changed to contain state name in "slug" field and other fields (currently only reason and backtrace for Broken variant in "data" field). To allow for this breaking change state was removed from TenantInfo swagger spec because it was not used anywhere. Please note that the tenant's broken reason is not persisted on disk so the reason is lost when pageserver is restarted. Requires changes to grafana dashboard that monitors tenant states. Closes #3001 --------- Co-authored-by: theirix --- Cargo.lock | 2 + libs/pageserver_api/Cargo.toml | 4 +- libs/pageserver_api/src/models.rs | 106 ++++++++++++++---- pageserver/src/http/openapi_spec.yml | 3 - pageserver/src/http/routes.rs | 6 +- pageserver/src/metrics.rs | 14 +-- pageserver/src/tenant.rs | 56 +++++---- pageserver/src/tenant/mgr.rs | 4 +- pageserver/src/tenant/tasks.rs | 2 +- test_runner/fixtures/pageserver/utils.py | 26 +++-- test_runner/regress/test_ondemand_download.py | 6 +- .../regress/test_pageserver_restart.py | 2 +- test_runner/regress/test_tenant_conf.py | 4 +- test_runner/regress/test_tenant_relocation.py | 6 +- test_runner/regress/test_tenant_tasks.py | 15 +-- test_runner/regress/test_tenants.py | 12 +- .../test_tenants_with_remote_storage.py | 6 +- test_runner/regress/test_timeline_size.py | 4 +- test_runner/regress/test_wal_acceptor.py | 2 +- 19 files changed, 181 insertions(+), 99 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 668487a9bd..fc587c57bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2517,6 +2517,8 @@ dependencies = [ "serde", "serde_json", "serde_with", + "strum", + "strum_macros", "utils", "workspace_hack", ] diff --git a/libs/pageserver_api/Cargo.toml b/libs/pageserver_api/Cargo.toml index 7709da1072..f97ec54e91 100644 --- a/libs/pageserver_api/Cargo.toml +++ b/libs/pageserver_api/Cargo.toml @@ -7,6 +7,7 @@ license.workspace = true [dependencies] serde.workspace = true serde_with.workspace = true +serde_json.workspace = true const_format.workspace = true anyhow.workspace = true bytes.workspace = true @@ -14,6 +15,7 @@ byteorder.workspace = true utils.workspace = true postgres_ffi.workspace = true enum-map.workspace = true -serde_json.workspace = true +strum.workspace = true +strum_macros.workspace = true workspace_hack.workspace = true diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 98a4b56858..a351761f4a 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -7,6 +7,7 @@ use std::{ use byteorder::{BigEndian, ReadBytesExt}; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DisplayFromStr}; +use strum_macros; use utils::{ history_buffer::HistoryBufferWithDropCounter, id::{NodeId, TenantId, TimelineId}, @@ -18,11 +19,23 @@ use anyhow::bail; use bytes::{BufMut, Bytes, BytesMut}; /// A state of a tenant in pageserver's memory. -#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[derive( + Clone, + PartialEq, + Eq, + serde::Serialize, + serde::Deserialize, + strum_macros::Display, + strum_macros::EnumString, + strum_macros::EnumVariantNames, + strum_macros::AsRefStr, + strum_macros::IntoStaticStr, +)] +#[serde(tag = "slug", content = "data")] pub enum TenantState { - // This tenant is being loaded from local disk + /// This tenant is being loaded from local disk Loading, - // This tenant is being downloaded from cloud storage. + /// This tenant is being downloaded from cloud storage. Attaching, /// Tenant is fully operational Active, @@ -31,15 +44,7 @@ pub enum TenantState { Stopping, /// A tenant is recognized by the pageserver, but can no longer be used for /// any operations, because it failed to be activated. - Broken, -} - -pub mod state { - pub const LOADING: &str = "loading"; - pub const ATTACHING: &str = "attaching"; - pub const ACTIVE: &str = "active"; - pub const STOPPING: &str = "stopping"; - pub const BROKEN: &str = "broken"; + Broken { reason: String, backtrace: String }, } impl TenantState { @@ -49,17 +54,26 @@ impl TenantState { Self::Attaching => true, Self::Active => false, Self::Stopping => false, - Self::Broken => false, + Self::Broken { .. } => false, } } - pub fn as_str(&self) -> &'static str { + pub fn broken_from_reason(reason: String) -> Self { + let backtrace_str: String = format!("{}", std::backtrace::Backtrace::force_capture()); + Self::Broken { + reason, + backtrace: backtrace_str, + } + } +} + +impl std::fmt::Debug for TenantState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - TenantState::Loading => state::LOADING, - TenantState::Attaching => state::ATTACHING, - TenantState::Active => state::ACTIVE, - TenantState::Stopping => state::STOPPING, - TenantState::Broken => state::BROKEN, + Self::Broken { reason, backtrace } if !reason.is_empty() => { + write!(f, "Broken due to: {reason}. Backtrace:\n{backtrace}") + } + _ => write!(f, "{self}"), } } } @@ -615,6 +629,7 @@ impl PagestreamBeMessage { #[cfg(test)] mod tests { use bytes::Buf; + use serde_json::json; use super::*; @@ -665,4 +680,57 @@ mod tests { assert!(msg == reconstructed); } } + + #[test] + fn test_tenantinfo_serde() { + // Test serialization/deserialization of TenantInfo + let original_active = TenantInfo { + id: TenantId::generate(), + state: TenantState::Active, + current_physical_size: Some(42), + has_in_progress_downloads: Some(false), + }; + let expected_active = json!({ + "id": original_active.id.to_string(), + "state": { + "slug": "Active", + }, + "current_physical_size": 42, + "has_in_progress_downloads": false, + }); + + let original_broken = TenantInfo { + id: TenantId::generate(), + state: TenantState::Broken { + reason: "reason".into(), + backtrace: "backtrace info".into(), + }, + current_physical_size: Some(42), + has_in_progress_downloads: Some(false), + }; + let expected_broken = json!({ + "id": original_broken.id.to_string(), + "state": { + "slug": "Broken", + "data": { + "backtrace": "backtrace info", + "reason": "reason", + } + }, + "current_physical_size": 42, + "has_in_progress_downloads": false, + }); + + assert_eq!( + serde_json::to_value(&original_active).unwrap(), + expected_active + ); + + assert_eq!( + serde_json::to_value(&original_broken).unwrap(), + expected_broken + ); + assert!(format!("{:?}", &original_broken.state).contains("reason")); + assert!(format!("{:?}", &original_broken.state).contains("backtrace info")); + } } diff --git a/pageserver/src/http/openapi_spec.yml b/pageserver/src/http/openapi_spec.yml index 478e9d228a..b0e4e1ca85 100644 --- a/pageserver/src/http/openapi_spec.yml +++ b/pageserver/src/http/openapi_spec.yml @@ -829,12 +829,9 @@ components: type: object required: - id - - state properties: id: type: string - state: - type: string current_physical_size: type: integer has_in_progress_downloads: diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 2db60f557d..e7a86e4822 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -465,7 +465,7 @@ async fn tenant_list_handler(request: Request) -> Result, A .iter() .map(|(id, state)| TenantInfo { id: *id, - state: *state, + state: state.clone(), current_physical_size: None, has_in_progress_downloads: Some(state.has_in_progress_downloads()), }) @@ -490,7 +490,7 @@ async fn tenant_status(request: Request) -> Result, ApiErro let state = tenant.current_state(); Ok(TenantInfo { id: tenant_id, - state, + state: state.clone(), current_physical_size: Some(current_physical_size), has_in_progress_downloads: Some(state.has_in_progress_downloads()), }) @@ -931,7 +931,7 @@ async fn handle_tenant_break(r: Request) -> Result, ApiErro .await .map_err(|_| ApiError::Conflict(String::from("no active tenant found")))?; - tenant.set_broken("broken from test"); + tenant.set_broken("broken from test".to_owned()); json_response(StatusCode::OK, ()) } diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs index 1f31e5a8fb..dfb38387ea 100644 --- a/pageserver/src/metrics.rs +++ b/pageserver/src/metrics.rs @@ -6,7 +6,8 @@ use metrics::{ UIntGauge, UIntGaugeVec, }; use once_cell::sync::Lazy; -use pageserver_api::models::state; +use pageserver_api::models::TenantState; +use strum::VariantNames; use utils::id::{TenantId, TimelineId}; /// Prometheus histogram buckets (in seconds) for operations in the critical @@ -147,15 +148,6 @@ static CURRENT_LOGICAL_SIZE: Lazy = Lazy::new(|| { .expect("failed to define current logical size metric") }); -// Metrics collected on tenant states. -const TENANT_STATE_OPTIONS: &[&str] = &[ - state::LOADING, - state::ATTACHING, - state::ACTIVE, - state::STOPPING, - state::BROKEN, -]; - pub static TENANT_STATE_METRIC: Lazy = Lazy::new(|| { register_uint_gauge_vec!( "pageserver_tenant_states_count", @@ -707,7 +699,7 @@ impl Drop for TimelineMetrics { pub fn remove_tenant_metrics(tenant_id: &TenantId) { let tid = tenant_id.to_string(); let _ = TENANT_SYNTHETIC_SIZE_METRIC.remove_label_values(&[&tid]); - for state in TENANT_STATE_OPTIONS { + for state in TenantState::VARIANTS { let _ = TENANT_STATE_METRIC.remove_label_values(&[&tid, state]); } } diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 7e88a12963..d98aa5c566 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -622,7 +622,7 @@ impl Tenant { match tenant_clone.attach(ctx).await { Ok(_) => {} Err(e) => { - tenant_clone.set_broken(&e.to_string()); + tenant_clone.set_broken(e.to_string()); error!("error attaching tenant: {:?}", e); } } @@ -830,7 +830,10 @@ impl Tenant { pub fn create_broken_tenant(conf: &'static PageServerConf, tenant_id: TenantId) -> Arc { let wal_redo_manager = Arc::new(PostgresRedoManager::new(conf, tenant_id)); Arc::new(Tenant::new( - TenantState::Broken, + TenantState::Broken { + reason: "create_broken_tenant".into(), + backtrace: String::new(), + }, conf, TenantConfOpt::default(), wal_redo_manager, @@ -891,7 +894,7 @@ impl Tenant { match tenant_clone.load(&ctx).await { Ok(()) => {} Err(err) => { - tenant_clone.set_broken(&err.to_string()); + tenant_clone.set_broken(err.to_string()); error!("could not load tenant {tenant_id}: {err:?}"); } } @@ -1443,7 +1446,7 @@ impl Tenant { } pub fn current_state(&self) -> TenantState { - *self.state.borrow() + self.state.borrow().clone() } pub fn is_active(&self) -> bool { @@ -1454,15 +1457,15 @@ impl Tenant { fn activate(&self, ctx: &RequestContext) -> anyhow::Result<()> { let mut result = Ok(()); self.state.send_modify(|current_state| { - match *current_state { + match &*current_state { TenantState::Active => { // activate() was called on an already Active tenant. Shouldn't happen. result = Err(anyhow::anyhow!("Tenant is already active")); } - TenantState::Broken => { + TenantState::Broken { reason, .. } => { // This shouldn't happen either result = Err(anyhow::anyhow!( - "Could not activate tenant because it is in broken state" + "Could not activate tenant because it is in broken state due to: {reason}", )); } TenantState::Stopping => { @@ -1496,7 +1499,10 @@ impl Tenant { timeline.timeline_id, e ); timeline.set_state(TimelineState::Broken); - *current_state = TenantState::Broken; + *current_state = TenantState::broken_from_reason(format!( + "failed to activate timeline {}: {}", + timeline.timeline_id, e + )); } } } @@ -1509,7 +1515,7 @@ impl Tenant { /// Change tenant status to Stopping, to mark that it is being shut down pub fn set_stopping(&self) { self.state.send_modify(|current_state| { - match *current_state { + match current_state { TenantState::Active | TenantState::Loading | TenantState::Attaching => { *current_state = TenantState::Stopping; @@ -1525,8 +1531,8 @@ impl Tenant { timeline.set_state(TimelineState::Stopping); } } - TenantState::Broken => { - info!("Cannot set tenant to Stopping state, it is already in Broken state"); + TenantState::Broken { reason, .. } => { + info!("Cannot set tenant to Stopping state, it is in Broken state due to: {reason}"); } TenantState::Stopping => { // The tenant was detached, or system shutdown was requested, while we were @@ -1537,7 +1543,7 @@ impl Tenant { }); } - pub fn set_broken(&self, reason: &str) { + pub fn set_broken(&self, reason: String) { self.state.send_modify(|current_state| { match *current_state { TenantState::Active => { @@ -1545,24 +1551,24 @@ impl Tenant { // while loading or attaching a tenant. A tenant that has already been // activated should never be marked as broken. We cope with it the best // we can, but it shouldn't happen. - *current_state = TenantState::Broken; warn!("Changing Active tenant to Broken state, reason: {}", reason); + *current_state = TenantState::broken_from_reason(reason); } - TenantState::Broken => { + TenantState::Broken { .. } => { // This shouldn't happen either warn!("Tenant is already in Broken state"); } TenantState::Stopping => { // This shouldn't happen either - *current_state = TenantState::Broken; warn!( "Marking Stopping tenant as Broken state, reason: {}", reason ); + *current_state = TenantState::broken_from_reason(reason); } TenantState::Loading | TenantState::Attaching => { info!("Setting tenant as Broken state, reason: {}", reason); - *current_state = TenantState::Broken; + *current_state = TenantState::broken_from_reason(reason); } } }); @@ -1575,7 +1581,7 @@ impl Tenant { pub async fn wait_to_become_active(&self) -> anyhow::Result<()> { let mut receiver = self.state.subscribe(); loop { - let current_state = *receiver.borrow_and_update(); + let current_state = receiver.borrow_and_update().clone(); match current_state { TenantState::Loading | TenantState::Attaching => { // in these states, there's a chance that we can reach ::Active @@ -1584,12 +1590,12 @@ impl Tenant { TenantState::Active { .. } => { return Ok(()); } - TenantState::Broken | TenantState::Stopping => { + TenantState::Broken { .. } | TenantState::Stopping => { // There's no chance the tenant can transition back into ::Active anyhow::bail!( "Tenant {} will not become active. Current state: {:?}", self.tenant_id, - current_state, + ¤t_state, ); } } @@ -1770,21 +1776,23 @@ impl Tenant { let (state, mut rx) = watch::channel(state); tokio::spawn(async move { - let current_state = *rx.borrow_and_update(); + let mut current_state: &'static str = From::from(&*rx.borrow_and_update()); let tid = tenant_id.to_string(); TENANT_STATE_METRIC - .with_label_values(&[&tid, current_state.as_str()]) + .with_label_values(&[&tid, current_state]) .inc(); loop { match rx.changed().await { Ok(()) => { - let new_state = *rx.borrow(); + let new_state: &'static str = From::from(&*rx.borrow_and_update()); TENANT_STATE_METRIC - .with_label_values(&[&tid, current_state.as_str()]) + .with_label_values(&[&tid, current_state]) .dec(); TENANT_STATE_METRIC - .with_label_values(&[&tid, new_state.as_str()]) + .with_label_values(&[&tid, new_state]) .inc(); + + current_state = new_state; } Err(_sender_dropped_error) => { info!("Tenant dropped the state updates sender, quitting waiting for tenant state change"); diff --git a/pageserver/src/tenant/mgr.rs b/pageserver/src/tenant/mgr.rs index 4971186206..754316b3cd 100644 --- a/pageserver/src/tenant/mgr.rs +++ b/pageserver/src/tenant/mgr.rs @@ -537,7 +537,7 @@ where Some(tenant) => match tenant.current_state() { TenantState::Attaching | TenantState::Loading - | TenantState::Broken + | TenantState::Broken { .. } | TenantState::Active => tenant.set_stopping(), TenantState::Stopping => return Err(TenantStateError::IsStopping(tenant_id)), }, @@ -565,7 +565,7 @@ where let tenants_accessor = TENANTS.read().await; match tenants_accessor.get(&tenant_id) { Some(tenant) => { - tenant.set_broken(&e.to_string()); + tenant.set_broken(e.to_string()); } None => { warn!("Tenant {tenant_id} got removed from memory"); diff --git a/pageserver/src/tenant/tasks.rs b/pageserver/src/tenant/tasks.rs index 8aeacc12f5..7e7dbd3c5c 100644 --- a/pageserver/src/tenant/tasks.rs +++ b/pageserver/src/tenant/tasks.rs @@ -209,7 +209,7 @@ async fn wait_for_active_tenant( loop { match tenant_state_updates.changed().await { Ok(()) => { - let new_state = *tenant_state_updates.borrow(); + let new_state = &*tenant_state_updates.borrow(); match new_state { TenantState::Active => { debug!("Tenant state changed to active, continuing the task loop"); diff --git a/test_runner/fixtures/pageserver/utils.py b/test_runner/fixtures/pageserver/utils.py index 65eda5b636..c060fc8dea 100644 --- a/test_runner/fixtures/pageserver/utils.py +++ b/test_runner/fixtures/pageserver/utils.py @@ -1,16 +1,20 @@ import time +from typing import Optional from fixtures.log_helper import log from fixtures.pageserver.http import PageserverHttpClient from fixtures.types import Lsn, TenantId, TimelineId -def assert_tenant_status( - pageserver_http: PageserverHttpClient, tenant: TenantId, expected_status: str +def assert_tenant_state( + pageserver_http: PageserverHttpClient, + tenant: TenantId, + expected_state: str, + message: Optional[str] = None, ): tenant_status = pageserver_http.tenant_status(tenant) log.info(f"tenant_status: {tenant_status}") - assert tenant_status["state"] == expected_status, tenant_status + assert tenant_status["state"]["slug"] == expected_state, message or tenant_status def tenant_exists(pageserver_http: PageserverHttpClient, tenant_id: TenantId): @@ -68,6 +72,7 @@ def wait_until_tenant_state( tenant_id: TenantId, expected_state: str, iterations: int, + period: float = 1.0, ) -> bool: """ Does not use `wait_until` for debugging purposes @@ -76,21 +81,28 @@ def wait_until_tenant_state( try: tenant = pageserver_http.tenant_status(tenant_id=tenant_id) log.debug(f"Tenant {tenant_id} data: {tenant}") - if tenant["state"] == expected_state: + if tenant["state"]["slug"] == expected_state: return True except Exception as e: log.debug(f"Tenant {tenant_id} state retrieval failure: {e}") - time.sleep(1) + time.sleep(period) raise Exception(f"Tenant {tenant_id} did not become {expected_state} in {iterations} seconds") def wait_until_tenant_active( - pageserver_http: PageserverHttpClient, tenant_id: TenantId, iterations: int = 30 + pageserver_http: PageserverHttpClient, + tenant_id: TenantId, + iterations: int = 30, + period: float = 1.0, ): wait_until_tenant_state( - pageserver_http, tenant_id, expected_state="Active", iterations=iterations + pageserver_http, + tenant_id, + expected_state="Active", + iterations=iterations, + period=period, ) diff --git a/test_runner/regress/test_ondemand_download.py b/test_runner/regress/test_ondemand_download.py index 90ab8e68d8..07410b64df 100644 --- a/test_runner/regress/test_ondemand_download.py +++ b/test_runner/regress/test_ondemand_download.py @@ -17,7 +17,7 @@ from fixtures.neon_fixtures import ( ) from fixtures.pageserver.http import PageserverApiException, PageserverHttpClient from fixtures.pageserver.utils import ( - assert_tenant_status, + assert_tenant_state, wait_for_last_record_lsn, wait_for_upload, wait_until_tenant_state, @@ -239,7 +239,7 @@ def test_ondemand_download_timetravel( ##### Second start, restore the data and ensure it's the same env.pageserver.start() - wait_until(10, 0.2, lambda: assert_tenant_status(client, tenant_id, "Active")) + wait_until(10, 0.2, lambda: assert_tenant_state(client, tenant_id, "Active")) # The current_physical_size reports the sum of layers loaded in the layer # map, regardless of where the layer files are located. So even though we @@ -392,7 +392,7 @@ def test_download_remote_layers_api( ] ) - wait_until(10, 0.2, lambda: assert_tenant_status(client, tenant_id, "Active")) + wait_until(10, 0.2, lambda: assert_tenant_state(client, tenant_id, "Active")) ###### Phase 1: exercise download error code path assert ( diff --git a/test_runner/regress/test_pageserver_restart.py b/test_runner/regress/test_pageserver_restart.py index 453ddec0d4..77db729880 100644 --- a/test_runner/regress/test_pageserver_restart.py +++ b/test_runner/regress/test_pageserver_restart.py @@ -59,7 +59,7 @@ def test_pageserver_restart(neon_env_builder: NeonEnvBuilder): client = env.pageserver.http_client() tenant_status = client.tenant_status(env.initial_tenant) log.info("Tenant status : %s", tenant_status) - assert tenant_status["state"] == "Loading" + assert tenant_status["state"]["slug"] == "Loading" # Try to read. This waits until the loading finishes, and then return normally. cur.execute("SELECT count(*) FROM foo") diff --git a/test_runner/regress/test_tenant_conf.py b/test_runner/regress/test_tenant_conf.py index 67aba227e5..80d4b99504 100644 --- a/test_runner/regress/test_tenant_conf.py +++ b/test_runner/regress/test_tenant_conf.py @@ -7,7 +7,7 @@ from fixtures.neon_fixtures import ( NeonEnvBuilder, RemoteStorageKind, ) -from fixtures.pageserver.utils import assert_tenant_status, wait_for_upload +from fixtures.pageserver.utils import assert_tenant_state, wait_for_upload from fixtures.types import Lsn from fixtures.utils import wait_until @@ -278,7 +278,7 @@ def test_creating_tenant_conf_after_attach(neon_env_builder: NeonEnvBuilder): wait_until( number_of_iterations=5, interval=1, - func=lambda: assert_tenant_status(http_client, tenant_id, "Active"), + func=lambda: assert_tenant_state(http_client, tenant_id, "Active"), ) env.neon_cli.config_tenant(tenant_id, {"gc_horizon": "1000000"}) diff --git a/test_runner/regress/test_tenant_relocation.py b/test_runner/regress/test_tenant_relocation.py index 8ad4bd1c11..3569ab0c53 100644 --- a/test_runner/regress/test_tenant_relocation.py +++ b/test_runner/regress/test_tenant_relocation.py @@ -15,7 +15,7 @@ from fixtures.neon_fixtures import ( ) from fixtures.pageserver.http import PageserverHttpClient from fixtures.pageserver.utils import ( - assert_tenant_status, + assert_tenant_state, tenant_exists, wait_for_last_record_lsn, wait_for_upload, @@ -416,11 +416,11 @@ def test_tenant_relocation( # wait for tenant to finish attaching tenant_status = new_pageserver_http.tenant_status(tenant_id=tenant_id) - assert tenant_status["state"] in ["Attaching", "Active"] + assert tenant_status["state"]["slug"] in ["Attaching", "Active"] wait_until( number_of_iterations=10, interval=1, - func=lambda: assert_tenant_status(new_pageserver_http, tenant_id, "Active"), + func=lambda: assert_tenant_state(new_pageserver_http, tenant_id, "Active"), ) check_timeline_attached( diff --git a/test_runner/regress/test_tenant_tasks.py b/test_runner/regress/test_tenant_tasks.py index 24b211e368..8c89100745 100644 --- a/test_runner/regress/test_tenant_tasks.py +++ b/test_runner/regress/test_tenant_tasks.py @@ -1,5 +1,6 @@ from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnvBuilder +from fixtures.pageserver.utils import assert_tenant_state, wait_until_tenant_active from fixtures.types import TenantId, TimelineId from fixtures.utils import wait_until @@ -25,16 +26,16 @@ def test_tenant_tasks(neon_env_builder: NeonEnvBuilder): for t in timelines: client.timeline_delete(tenant, t) - def assert_active(tenant): - assert get_state(tenant) == "Active" - # Create tenant, start compute tenant, _ = env.neon_cli.create_tenant() env.neon_cli.create_timeline(name, tenant_id=tenant) pg = env.postgres.create_start(name, tenant_id=tenant) - assert ( - get_state(tenant) == "Active" - ), "Pageserver should activate a tenant and start background jobs if timelines are loaded" + assert_tenant_state( + client, + tenant, + expected_state="Active", + message="Pageserver should activate a tenant and start background jobs if timelines are loaded", + ) # Stop compute pg.stop() @@ -47,7 +48,7 @@ def test_tenant_tasks(neon_env_builder: NeonEnvBuilder): for tenant_info in client.tenant_list(): tenant_id = TenantId(tenant_info["id"]) delete_all_timelines(tenant_id) - wait_until(10, 0.2, lambda: assert_active(tenant_id)) + wait_until_tenant_active(client, tenant_id, iterations=10, period=0.2) # Assert that all tasks finish quickly after tenant is detached task_starts = client.get_metric_value("pageserver_tenant_task_events_total", {"event": "start"}) diff --git a/test_runner/regress/test_tenants.py b/test_runner/regress/test_tenants.py index 8021bf9914..2162520217 100644 --- a/test_runner/regress/test_tenants.py +++ b/test_runner/regress/test_tenants.py @@ -332,24 +332,24 @@ def test_pageserver_with_empty_tenants( [broken_tenant] = [t for t in tenants if t["id"] == str(tenant_without_timelines_dir)] assert ( - broken_tenant["state"] == "Broken" + broken_tenant["state"]["slug"] == "Broken" ), f"Tenant {tenant_without_timelines_dir} without timelines dir should be broken" broken_tenant_status = client.tenant_status(tenant_without_timelines_dir) assert ( - broken_tenant_status["state"] == "Broken" + broken_tenant_status["state"]["slug"] == "Broken" ), f"Tenant {tenant_without_timelines_dir} without timelines dir should be broken" assert env.pageserver.log_contains(".*Setting tenant as Broken state, reason:.*") [loaded_tenant] = [t for t in tenants if t["id"] == str(tenant_with_empty_timelines_dir)] assert ( - loaded_tenant["state"] == "Active" + loaded_tenant["state"]["slug"] == "Active" ), "Tenant {tenant_with_empty_timelines_dir} with empty timelines dir should be active and ready for timeline creation" loaded_tenant_status = client.tenant_status(tenant_with_empty_timelines_dir) assert ( - loaded_tenant_status["state"] == "Active" + loaded_tenant_status["state"]["slug"] == "Active" ), f"Tenant {tenant_with_empty_timelines_dir} without timelines dir should be active" time.sleep(1) # to allow metrics propagation @@ -357,11 +357,11 @@ def test_pageserver_with_empty_tenants( ps_metrics = client.get_metrics() broken_tenants_metric_filter = { "tenant_id": str(tenant_without_timelines_dir), - "state": "broken", + "state": "Broken", } active_tenants_metric_filter = { "tenant_id": str(tenant_with_empty_timelines_dir), - "state": "active", + "state": "Active", } tenant_active_count = int( diff --git a/test_runner/regress/test_tenants_with_remote_storage.py b/test_runner/regress/test_tenants_with_remote_storage.py index ec1c12a0d8..514e2b6fa0 100644 --- a/test_runner/regress/test_tenants_with_remote_storage.py +++ b/test_runner/regress/test_tenants_with_remote_storage.py @@ -24,7 +24,7 @@ from fixtures.neon_fixtures import ( wait_for_sk_commit_lsn_to_reach_remote_storage, ) from fixtures.pageserver.utils import ( - assert_tenant_status, + assert_tenant_state, wait_for_last_record_lsn, wait_for_upload, ) @@ -202,7 +202,7 @@ def test_tenants_attached_after_download( wait_until( number_of_iterations=5, interval=1, - func=lambda: assert_tenant_status(client, tenant_id, "Active"), + func=lambda: assert_tenant_state(client, tenant_id, "Active"), ) restored_timelines = client.timeline_list(tenant_id) @@ -286,7 +286,7 @@ def test_tenant_redownloads_truncated_file_on_startup( wait_until( number_of_iterations=5, interval=1, - func=lambda: assert_tenant_status(client, tenant_id, "Active"), + func=lambda: assert_tenant_state(client, tenant_id, "Active"), ) restored_timelines = client.timeline_list(tenant_id) diff --git a/test_runner/regress/test_timeline_size.py b/test_runner/regress/test_timeline_size.py index 7c77e1fe59..28da3c5a48 100644 --- a/test_runner/regress/test_timeline_size.py +++ b/test_runner/regress/test_timeline_size.py @@ -23,7 +23,7 @@ from fixtures.neon_fixtures import ( ) from fixtures.pageserver.http import PageserverApiException, PageserverHttpClient from fixtures.pageserver.utils import ( - assert_tenant_status, + assert_tenant_state, wait_for_upload_queue_empty, wait_until_tenant_active, ) @@ -333,7 +333,7 @@ def test_timeline_physical_size_init( wait_until( number_of_iterations=5, interval=1, - func=lambda: assert_tenant_status(client, env.initial_tenant, "Active"), + func=lambda: assert_tenant_state(client, env.initial_tenant, "Active"), ) assert_physical_size_invariants( diff --git a/test_runner/regress/test_wal_acceptor.py b/test_runner/regress/test_wal_acceptor.py index 306c492e8f..c24c77bb95 100644 --- a/test_runner/regress/test_wal_acceptor.py +++ b/test_runner/regress/test_wal_acceptor.py @@ -587,7 +587,7 @@ def test_s3_wal_replay(neon_env_builder: NeonEnvBuilder, remote_storage_kind: Re raise RuntimeError("Timed out waiting for WAL redo") tenant_status = ps_cli.tenant_status(tenant_id) - if tenant_status["state"] == "Loading": + if tenant_status["state"]["slug"] == "Loading": log.debug(f"Tenant {tenant_id} is still loading, retrying") else: pageserver_lsn = Lsn(