mirror of
https://github.com/neondatabase/neon.git
synced 2026-01-17 02:12:56 +00:00
c9b1657e4c
## Problem For #6423, creating a reproducer turned out to be very easy, as an extension to test_ondemand_activation. However, before I had diagnosed the issue, I was starting with a more brute force approach of running creation API calls in the background while restarting a pageserver, and that shows up a bunch of other interesting issues. In this PR: - Add the reproducer for #6423 by extending `test_ondemand_activation` (confirmed that this test fails if I revert the fix from https://github.com/neondatabase/neon/pull/6430) - In timeline creation, return 503 responses when we get an error and the tenant's cancellation token is set: this covers the cases where we get an anyhow::Error from something during timeline creation as a result of shutdown. - While waiting for tenants to become active during creation, don't .map_err() the result to a 500: instead let the `From` impl map the result to something appropriate (this includes mapping shutdown to 503) - During tenant creation, we were calling `Tenant::load_local` because no Preload object is provided. This is usually harmless because the tenant dir is empty, but if there are some half-created timelines in there, bad things can happen. Propagate the SpawnMode into Tenant::attach, so that it can properly skip _any_ attempt to load timelines if creating. - When we call upsert_location, there's a SpawnMode that tells us whether to load from remote storage or not. But if the operation is a retry and we already have the tenant, it is not correct to skip loading from remote storage: there might be a timeline there. This isn't strictly a correctness issue as long as the caller behaves correctly (does not assume that any timelines are persistent until the creation is acked), but it's a more defensive position. - If we shut down while the task in Tenant::attach is running, it can end up spawning rogue tasks. Fix this by holding a GateGuard through here, and in upsert_location shutting down a tenant after calling tenant_spawn if we can't insert it into tenants_map. This fixes the expected behavior that after shutdown_all_tenants returns, no tenant tasks are running. - Add `test_create_churn_during_restart`, which runs tenant & timeline creations across pageserver restarts. - Update a couple of tests that covered cancellation, to reflect the cleaner errors we now return.
141 lines
4.9 KiB
Rust
141 lines
4.9 KiB
Rust
use hyper::{header, Body, Response, StatusCode};
|
|
use serde::{Deserialize, Serialize};
|
|
use std::borrow::Cow;
|
|
use std::error::Error as StdError;
|
|
use thiserror::Error;
|
|
use tracing::{error, info, warn};
|
|
|
|
#[derive(Debug, Error)]
|
|
pub enum ApiError {
|
|
#[error("Bad request: {0:#?}")]
|
|
BadRequest(anyhow::Error),
|
|
|
|
#[error("Forbidden: {0}")]
|
|
Forbidden(String),
|
|
|
|
#[error("Unauthorized: {0}")]
|
|
Unauthorized(String),
|
|
|
|
#[error("NotFound: {0}")]
|
|
NotFound(Box<dyn StdError + Send + Sync + 'static>),
|
|
|
|
#[error("Conflict: {0}")]
|
|
Conflict(String),
|
|
|
|
#[error("Precondition failed: {0}")]
|
|
PreconditionFailed(Box<str>),
|
|
|
|
#[error("Resource temporarily unavailable: {0}")]
|
|
ResourceUnavailable(Cow<'static, str>),
|
|
|
|
#[error("Shutting down")]
|
|
ShuttingDown,
|
|
|
|
#[error("Timeout")]
|
|
Timeout(Cow<'static, str>),
|
|
|
|
#[error(transparent)]
|
|
InternalServerError(anyhow::Error),
|
|
}
|
|
|
|
impl ApiError {
|
|
pub fn into_response(self) -> Response<Body> {
|
|
match self {
|
|
ApiError::BadRequest(err) => HttpErrorBody::response_from_msg_and_status(
|
|
format!("{err:#?}"), // use debug printing so that we give the cause
|
|
StatusCode::BAD_REQUEST,
|
|
),
|
|
ApiError::Forbidden(_) => {
|
|
HttpErrorBody::response_from_msg_and_status(self.to_string(), StatusCode::FORBIDDEN)
|
|
}
|
|
ApiError::Unauthorized(_) => HttpErrorBody::response_from_msg_and_status(
|
|
self.to_string(),
|
|
StatusCode::UNAUTHORIZED,
|
|
),
|
|
ApiError::NotFound(_) => {
|
|
HttpErrorBody::response_from_msg_and_status(self.to_string(), StatusCode::NOT_FOUND)
|
|
}
|
|
ApiError::Conflict(_) => {
|
|
HttpErrorBody::response_from_msg_and_status(self.to_string(), StatusCode::CONFLICT)
|
|
}
|
|
ApiError::PreconditionFailed(_) => HttpErrorBody::response_from_msg_and_status(
|
|
self.to_string(),
|
|
StatusCode::PRECONDITION_FAILED,
|
|
),
|
|
ApiError::ShuttingDown => HttpErrorBody::response_from_msg_and_status(
|
|
"Shutting down".to_string(),
|
|
StatusCode::SERVICE_UNAVAILABLE,
|
|
),
|
|
ApiError::ResourceUnavailable(err) => HttpErrorBody::response_from_msg_and_status(
|
|
err.to_string(),
|
|
StatusCode::SERVICE_UNAVAILABLE,
|
|
),
|
|
ApiError::Timeout(err) => HttpErrorBody::response_from_msg_and_status(
|
|
err.to_string(),
|
|
StatusCode::REQUEST_TIMEOUT,
|
|
),
|
|
ApiError::InternalServerError(err) => HttpErrorBody::response_from_msg_and_status(
|
|
err.to_string(),
|
|
StatusCode::INTERNAL_SERVER_ERROR,
|
|
),
|
|
}
|
|
}
|
|
}
|
|
|
|
#[derive(Serialize, Deserialize)]
|
|
pub struct HttpErrorBody {
|
|
pub msg: String,
|
|
}
|
|
|
|
impl HttpErrorBody {
|
|
pub fn from_msg(msg: String) -> Self {
|
|
HttpErrorBody { msg }
|
|
}
|
|
|
|
pub fn response_from_msg_and_status(msg: String, status: StatusCode) -> Response<Body> {
|
|
HttpErrorBody { msg }.to_response(status)
|
|
}
|
|
|
|
pub fn to_response(&self, status: StatusCode) -> Response<Body> {
|
|
Response::builder()
|
|
.status(status)
|
|
.header(header::CONTENT_TYPE, "application/json")
|
|
// we do not have nested maps with non string keys so serialization shouldn't fail
|
|
.body(Body::from(serde_json::to_string(self).unwrap()))
|
|
.unwrap()
|
|
}
|
|
}
|
|
|
|
pub async fn route_error_handler(err: routerify::RouteError) -> Response<Body> {
|
|
match err.downcast::<ApiError>() {
|
|
Ok(api_error) => api_error_handler(*api_error),
|
|
Err(other_error) => {
|
|
// We expect all the request handlers to return an ApiError, so this should
|
|
// not be reached. But just in case.
|
|
error!("Error processing HTTP request: {other_error:?}");
|
|
HttpErrorBody::response_from_msg_and_status(
|
|
other_error.to_string(),
|
|
StatusCode::INTERNAL_SERVER_ERROR,
|
|
)
|
|
}
|
|
}
|
|
}
|
|
|
|
pub fn api_error_handler(api_error: ApiError) -> Response<Body> {
|
|
// Print a stack trace for Internal Server errors
|
|
|
|
match api_error {
|
|
ApiError::Forbidden(_) | ApiError::Unauthorized(_) => {
|
|
warn!("Error processing HTTP request: {api_error:#}")
|
|
}
|
|
ApiError::ResourceUnavailable(_) => info!("Error processing HTTP request: {api_error:#}"),
|
|
ApiError::NotFound(_) => info!("Error processing HTTP request: {api_error:#}"),
|
|
ApiError::InternalServerError(_) => error!("Error processing HTTP request: {api_error:?}"),
|
|
ApiError::ShuttingDown => info!("Shut down while processing HTTP request"),
|
|
ApiError::Timeout(_) => info!("Timeout while processing HTTP request: {api_error:#}"),
|
|
_ => info!("Error processing HTTP request: {api_error:#}"),
|
|
}
|
|
|
|
api_error.into_response()
|
|
}
|