Skip to content

Commit

Permalink
pageserver: remove AncestorStopping error variants (#7916)
Browse files Browse the repository at this point in the history
## Problem

In all cases, AncestorStopping is equivalent to Cancelled.

This became more obvious in
#7912 (comment)
when updating these error types.

## Summary of changes

- Remove AncestorStopping, always use Cancelled instead
  • Loading branch information
jcsp authored May 31, 2024
1 parent 87afbf6 commit 9fda85b
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pageserver/src/consumption_metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ async fn calculate_and_log(tenant: &Tenant, cancel: &CancellationToken, ctx: &Re
// mean the synthetic size worker should terminate.
let shutting_down = matches!(
e.downcast_ref::<PageReconstructError>(),
Some(PageReconstructError::Cancelled | PageReconstructError::AncestorStopping(_))
Some(PageReconstructError::Cancelled)
);

if !shutting_down {
Expand Down
3 changes: 0 additions & 3 deletions pageserver/src/http/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,6 @@ impl From<PageReconstructError> for ApiError {
PageReconstructError::Cancelled => {
ApiError::InternalServerError(anyhow::anyhow!("request was cancelled"))
}
PageReconstructError::AncestorStopping(_) => {
ApiError::ResourceUnavailable(format!("{pre}").into())
}
PageReconstructError::AncestorLsnTimeout(e) => ApiError::Timeout(format!("{e}").into()),
PageReconstructError::WalRedo(pre) => ApiError::InternalServerError(pre),
}
Expand Down
7 changes: 2 additions & 5 deletions pageserver/src/pgdatadir_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,7 @@ impl From<PageReconstructError> for CollectKeySpaceError {
impl From<PageReconstructError> for CalculateLogicalSizeError {
fn from(pre: PageReconstructError) -> Self {
match pre {
PageReconstructError::AncestorStopping(_) | PageReconstructError::Cancelled => {
Self::Cancelled
}
PageReconstructError::Cancelled => Self::Cancelled,
_ => Self::PageRead(pre),
}
}
Expand Down Expand Up @@ -1614,8 +1612,7 @@ impl<'a> DatadirModification<'a> {
aux_files.dir = Some(dir);
}
Err(
e @ (PageReconstructError::AncestorStopping(_)
| PageReconstructError::Cancelled
e @ (PageReconstructError::Cancelled
| PageReconstructError::AncestorLsnTimeout(_)),
) => {
// Important that we do not interpret a shutdown error as "not found" and thereby
Expand Down
19 changes: 4 additions & 15 deletions pageserver/src/tenant/timeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,6 @@ pub(crate) enum PageReconstructError {
#[error("timeline shutting down")]
Cancelled,

/// The ancestor of this is being stopped
#[error("ancestor timeline {0} is being stopped")]
AncestorStopping(TimelineId),

/// An error happened replaying WAL records
#[error(transparent)]
WalRedo(anyhow::Error),
Expand Down Expand Up @@ -569,7 +565,7 @@ impl PageReconstructError {
match self {
Other(_) => false,
AncestorLsnTimeout(_) => false,
Cancelled | AncestorStopping(_) => true,
Cancelled => true,
WalRedo(_) => false,
MissingKey { .. } => false,
}
Expand Down Expand Up @@ -645,9 +641,6 @@ pub(crate) enum GetVectoredError {

#[derive(thiserror::Error, Debug)]
pub(crate) enum GetReadyAncestorError {
#[error("ancestor timeline {0} is being stopped")]
AncestorStopping(TimelineId),

#[error("Ancestor LSN wait error: {0}")]
AncestorLsnTimeout(#[from] WaitLsnError),

Expand Down Expand Up @@ -757,7 +750,6 @@ impl From<GetReadyAncestorError> for PageReconstructError {
fn from(e: GetReadyAncestorError) -> Self {
use GetReadyAncestorError::*;
match e {
AncestorStopping(tid) => PageReconstructError::AncestorStopping(tid),
AncestorLsnTimeout(wait_err) => PageReconstructError::AncestorLsnTimeout(wait_err),
bad_state @ BadState { .. } => PageReconstructError::Other(anyhow::anyhow!(bad_state)),
Cancelled => PageReconstructError::Cancelled,
Expand Down Expand Up @@ -1192,9 +1184,7 @@ impl Timeline {

use PageReconstructError::*;
match block {
Err(Cancelled | AncestorStopping(_)) => {
return Err(GetVectoredError::Cancelled)
}
Err(Cancelled) => return Err(GetVectoredError::Cancelled),
Err(MissingKey(_))
if NON_INHERITED_RANGE.contains(&key)
|| NON_INHERITED_SPARSE_RANGE.contains(&key) =>
Expand Down Expand Up @@ -3585,9 +3575,8 @@ impl Timeline {
match ancestor.wait_to_become_active(ctx).await {
Ok(()) => {}
Err(TimelineState::Stopping) => {
return Err(GetReadyAncestorError::AncestorStopping(
ancestor.timeline_id,
));
// If an ancestor is stopping, it means the tenant is stopping: handle this the same as if this timeline was stopping.
return Err(GetReadyAncestorError::Cancelled);
}
Err(state) => {
return Err(GetReadyAncestorError::BadState {
Expand Down
2 changes: 1 addition & 1 deletion test_runner/fixtures/pageserver/allowed_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def scan_pageserver_log_for_errors(
".*query handler for 'pagestream.*failed: Timeline .* is not active", # timeline delete in progress
".*task iteration took longer than the configured period.*",
# these can happen anytime we do compactions from background task and shutdown pageserver
r".*ERROR.*ancestor timeline \S+ is being stopped",
".*could not compact.*cancelled.*",
# this is expected given our collaborative shutdown approach for the UploadQueue
".*Compaction failed.*, retrying in .*: Other\\(queue is in state Stopped.*",
".*Compaction failed.*, retrying in .*: ShuttingDown",
Expand Down

1 comment on commit 9fda85b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tests were run or test report is not available

Code coverage* (full report)

  • functions: 31.4% (6503 of 20707 functions)
  • lines: 48.3% (50220 of 103877 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
9fda85b at 2024-05-31T17:22:03.947Z :recycle:

Please sign in to comment.