Return 404 if timeline is not found in safekeeper HTTP API.

This commit is contained in:
Arseny Sher
2023-03-06 17:07:10 +04:00
committed by Arseny Sher
parent ca85646df4
commit 0acf9ace9a
6 changed files with 30 additions and 25 deletions

View File

@@ -218,7 +218,7 @@ impl SafekeeperPostgresHandler {
/// Handle IDENTIFY_SYSTEM replication command
///
fn handle_identify_system(&mut self, pgb: &mut PostgresBackend) -> Result<(), QueryError> {
let tli = GlobalTimelines::get(self.ttid)?;
let tli = GlobalTimelines::get(self.ttid).map_err(|e| QueryError::Other(e.into()))?;
let lsn = if self.is_walproposer_recovery() {
// walproposer should get all local WAL until flush_lsn

View File

@@ -119,6 +119,12 @@ paths:
$ref: "#/components/responses/ForbiddenError"
default:
$ref: "#/components/responses/GenericError"
"404":
description: Timeline not found
content:
application/json:
schema:
$ref: "#/components/schemas/NotFoundError"
delete:
tags:

View File

@@ -1,6 +1,5 @@
use hyper::{Body, Request, Response, StatusCode, Uri};
use anyhow::Context;
use once_cell::sync::Lazy;
use postgres_ffi::WAL_SEGMENT_SIZE;
use safekeeper_api::models::SkTimelineInfo;
@@ -112,12 +111,7 @@ async fn timeline_status_handler(request: Request<Body>) -> Result<Response<Body
);
check_permission(&request, Some(ttid.tenant_id))?;
let tli = GlobalTimelines::get(ttid)
// FIXME: Currently, the only errors from `GlobalTimelines::get` will be client errors
// because the provided timeline isn't there. However, the method can in theory change and
// fail from internal errors later. Remove this comment once it the method returns
// something other than `anyhow::Result`.
.map_err(ApiError::InternalServerError)?;
let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
let (inmem, state) = tli.get_state();
let flush_lsn = tli.get_flush_lsn();
@@ -253,15 +247,7 @@ async fn record_safekeeper_info(mut request: Request<Body>) -> Result<Response<B
local_start_lsn: sk_info.local_start_lsn.0,
};
let tli = GlobalTimelines::get(ttid)
// `GlobalTimelines::get` returns an error when it can't find the timeline.
.with_context(|| {
format!(
"Couldn't get timeline {} for tenant {}",
ttid.timeline_id, ttid.tenant_id
)
})
.map_err(ApiError::NotFound)?;
let tli = GlobalTimelines::get(ttid).map_err(ApiError::from)?;
tli.record_safekeeper_info(&proto_sk_info)
.await
.map_err(ApiError::InternalServerError)?;

View File

@@ -164,7 +164,7 @@ impl ReplicationConn {
) -> Result<(), QueryError> {
let _enter = info_span!("WAL sender", ttid = %spg.ttid).entered();
let tli = GlobalTimelines::get(spg.ttid)?;
let tli = GlobalTimelines::get(spg.ttid).map_err(|e| QueryError::Other(e.into()))?;
// spawn the background thread which receives HotStandbyFeedback messages.
let bg_timeline = Arc::clone(&tli);

View File

@@ -1,7 +1,7 @@
//! This module implements Timeline lifecycle management and has all neccessary code
//! to glue together SafeKeeper and all other background services.
use anyhow::{bail, Result};
use anyhow::{anyhow, bail, Result};
use parking_lot::{Mutex, MutexGuard};
use postgres_ffi::XLogSegNo;
use pq_proto::ReplicationFeedback;
@@ -13,6 +13,7 @@ use tokio::{
time::Instant,
};
use tracing::*;
use utils::http::error::ApiError;
use utils::{
id::{NodeId, TenantTimelineId},
lsn::Lsn,
@@ -356,6 +357,18 @@ pub enum TimelineError {
UninitialinzedPgVersion(TenantTimelineId),
}
// Convert to HTTP API error.
impl From<TimelineError> for ApiError {
fn from(te: TimelineError) -> ApiError {
match te {
TimelineError::NotFound(ttid) => {
ApiError::NotFound(anyhow!("timeline {} not found", ttid))
}
_ => ApiError::InternalServerError(anyhow!("{}", te)),
}
}
}
/// Timeline struct manages lifecycle (creation, deletion, restore) of a safekeeper timeline.
/// It also holds SharedState and provides mutually exclusive access to it.
pub struct Timeline {

View File

@@ -5,7 +5,7 @@
use crate::safekeeper::ServerInfo;
use crate::timeline::{Timeline, TimelineError};
use crate::SafeKeeperConf;
use anyhow::{anyhow, bail, Context, Result};
use anyhow::{bail, Context, Result};
use once_cell::sync::Lazy;
use serde::Serialize;
use std::collections::HashMap;
@@ -50,11 +50,11 @@ impl GlobalTimelinesState {
}
/// Get timeline from the map. Returns error if timeline doesn't exist.
fn get(&self, ttid: &TenantTimelineId) -> Result<Arc<Timeline>> {
fn get(&self, ttid: &TenantTimelineId) -> Result<Arc<Timeline>, TimelineError> {
self.timelines
.get(ttid)
.cloned()
.ok_or_else(|| anyhow!(TimelineError::NotFound(*ttid)))
.ok_or(TimelineError::NotFound(*ttid))
}
}
@@ -240,17 +240,17 @@ impl GlobalTimelines {
/// Get a timeline from the global map. If it's not present, it doesn't exist on disk,
/// or was corrupted and couldn't be loaded on startup. Returned timeline is always valid,
/// i.e. loaded in memory and not cancelled.
pub fn get(ttid: TenantTimelineId) -> Result<Arc<Timeline>> {
pub fn get(ttid: TenantTimelineId) -> Result<Arc<Timeline>, TimelineError> {
let res = TIMELINES_STATE.lock().unwrap().get(&ttid);
match res {
Ok(tli) => {
if tli.is_cancelled() {
anyhow::bail!(TimelineError::Cancelled(ttid));
return Err(TimelineError::Cancelled(ttid));
}
Ok(tli)
}
Err(e) => Err(e),
_ => res,
}
}