diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 93c88749e5..86b9be640e 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -23,13 +23,28 @@ pub enum TenantState { Active { background_jobs_running: bool, }, - /// A tenant is recognized by pageserver, but not yet ready to operate: - /// e.g. not present locally and being downloaded or being read into memory from the file system. + /// A tenant is recognized by pageserver, but it is being detached or the system is being + /// shut down. Paused, - /// A tenant is recognized by the pageserver, but no longer used for any operations, as failed to get activated. + /// A tenant is recognized by the pageserver, but can no longer used for any operations, + /// because it failed to get activated. Broken, } +impl TenantState { + pub fn has_in_progress_downloads(&self) -> bool { + match self { + Self::Loading => true, + Self::Attaching => true, + Self::Active { + background_jobs_running: _, + } => false, + Self::Paused => false, + Self::Broken => false, + } + } +} + /// A state of a timeline in pageserver's memory. #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub enum TimelineState { diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 3f0a1aab01..e0a74ba2f0 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -396,7 +396,7 @@ async fn tenant_list_handler(request: Request) -> Result, A id: *id, state: *state, current_physical_size: None, - has_in_progress_downloads: Some(state == &TenantState::Attaching), + has_in_progress_downloads: Some(state.has_in_progress_downloads()), }) .collect::>() }) @@ -425,7 +425,7 @@ async fn tenant_status(request: Request) -> Result, ApiErro id: tenant_id, state, current_physical_size: Some(current_physical_size), - has_in_progress_downloads: Some(state == TenantState::Attaching), + has_in_progress_downloads: Some(state.has_in_progress_downloads()), }; Ok::<_, anyhow::Error>(tenant_info) diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index 5de11017c6..b829f05395 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -940,15 +940,10 @@ impl postgres_backend_async::Handler for PageServerHandler { /// all tenants are still loading. async fn get_active_tenant_with_timeout(tenant_id: TenantId) -> Result> { let tenant = tenant_mgr::get_tenant(tenant_id, false)?; - - match tokio::time::timeout(Duration::from_secs(30), tenant.wait_until_loaded()).await { - Ok(result) => { - let state = result?; - if !matches!(state, pageserver_api::models::TenantState::Active { .. }) { - anyhow::bail!("Tenant {tenant_id} is not active. Current state: {state:?}"); - } - Ok(tenant) - } + match tokio::time::timeout(Duration::from_secs(30), tenant.wait_to_become_active()).await { + Ok(wait_result) => wait_result + // no .context(), the error message is good enough and some tests depend on it + .map(move |()| tenant), Err(_) => anyhow::bail!("Timeout waiting for tenant {tenant_id} to become Active"), } } diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 5056c9ddc2..58844a62a4 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -1298,14 +1298,27 @@ impl Tenant { self.state.subscribe() } - pub async fn wait_until_loaded(&self) -> anyhow::Result { + pub async fn wait_to_become_active(&self) -> anyhow::Result<()> { let mut receiver = self.state.subscribe(); loop { let current_state = *receiver.borrow_and_update(); - if current_state != TenantState::Loading { - break Ok(current_state); + match current_state { + TenantState::Loading | TenantState::Attaching => { + // in these states, there's a chance that we can reach ::Active + receiver.changed().await?; + } + TenantState::Active { .. } => { + return Ok(()); + } + TenantState::Broken | TenantState::Paused => { + // 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, + ); + } } - receiver.changed().await?; } } } diff --git a/test_runner/regress/test_broken_timeline.py b/test_runner/regress/test_broken_timeline.py index 421dcf3140..08945e6795 100644 --- a/test_runner/regress/test_broken_timeline.py +++ b/test_runner/regress/test_broken_timeline.py @@ -18,6 +18,7 @@ def test_broken_timeline(neon_env_builder: NeonEnvBuilder): ".*Failed to load delta layer.*", ".*could not find data for key.*", ".*is not active. Current state: Broken.*", + ".*will not become active. Current state: Broken.*", ".*failed to load metadata.*", ] ) @@ -78,7 +79,7 @@ def test_broken_timeline(neon_env_builder: NeonEnvBuilder): # First timeline would not get loaded into pageserver due to corrupt metadata file with pytest.raises( - Exception, match=f"Tenant {tenant1} is not active. Current state: Broken" + Exception, match=f"Tenant {tenant1} will not become active. Current state: Broken" ) as err: pg1.start() log.info(