From b63ba61da1580a94c97a7c9074e4f8e56871ebda Mon Sep 17 00:00:00 2001 From: John Spray Date: Thu, 7 Dec 2023 14:57:15 +0000 Subject: [PATCH] pageserver: refactor creation API (add ShardParams) --- control_plane/src/bin/attachment_service.rs | 18 +++++++++++ control_plane/src/pageserver.rs | 3 +- libs/pageserver_api/src/models.rs | 36 ++++++++++++++++++++- libs/pageserver_api/src/shard.rs | 16 ++++++++- pageserver/src/http/routes.rs | 3 +- pageserver/src/tenant.rs | 5 ++- pageserver/src/tenant/config.rs | 11 ++++--- pageserver/src/tenant/mgr.rs | 31 +++++++++++++++--- 8 files changed, 110 insertions(+), 13 deletions(-) diff --git a/control_plane/src/bin/attachment_service.rs b/control_plane/src/bin/attachment_service.rs index e50c8fbba0..9db36ebd55 100644 --- a/control_plane/src/bin/attachment_service.rs +++ b/control_plane/src/bin/attachment_service.rs @@ -288,6 +288,21 @@ async fn handle_inspect(mut req: Request) -> Result, ApiErr ) } +async fn handle_tenant_create(mut req: Request) -> Result, ApiError> { + let inspect_req = json_request::(&mut req).await?; + + let state = get_state(&req).inner.clone(); + let locked = state.write().await; + let tenant_state = locked.tenants.get(&inspect_req.tenant_shard_id); + + json_response( + StatusCode::OK, + InspectResponse { + attachment: tenant_state.and_then(|s| s.pageserver.map(|ps| (s.generation, ps))), + }, + ) +} + fn make_router(persistent_state: PersistentState) -> RouterBuilder { endpoint::make_router() .data(Arc::new(State::new(persistent_state))) @@ -295,6 +310,9 @@ fn make_router(persistent_state: PersistentState) -> RouterBuilder, } +/// Parameters that apply to all shards in a tenant. Used during tenant creation. +#[derive(Serialize, Deserialize, Debug)] +#[serde(deny_unknown_fields)] +pub struct ShardParameters { + pub count: ShardCount, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub stripe_size: Option, +} + +impl ShardParameters { + pub fn is_unsharded(&self) -> bool { + self.count == ShardCount(0) + } +} + +impl Default for ShardParameters { + fn default() -> Self { + Self { + count: ShardCount(0), + stripe_size: None, + } + } +} + #[derive(Serialize, Deserialize, Debug)] #[serde(deny_unknown_fields)] pub struct TenantCreateRequest { @@ -195,6 +223,12 @@ pub struct TenantCreateRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub generation: Option, + + // If omitted, create a single shard with TenantShardId::unsharded() + #[serde(default)] + #[serde(skip_serializing_if = "ShardParameters::is_unsharded")] + pub shard_parameters: ShardParameters, + #[serde(flatten)] pub config: TenantConfig, // as we have a flattened field, we should reject all unknown fields in it } diff --git a/libs/pageserver_api/src/shard.rs b/libs/pageserver_api/src/shard.rs index 18ef2be523..4f1188ba1d 100644 --- a/libs/pageserver_api/src/shard.rs +++ b/libs/pageserver_api/src/shard.rs @@ -1,6 +1,9 @@ use std::{ops::RangeInclusive, str::FromStr}; -use crate::key::{is_rel_block_key, Key}; +use crate::{ + key::{is_rel_block_key, Key}, + models::ShardParameters, +}; use hex::FromHex; use serde::{Deserialize, Serialize}; use thiserror; @@ -403,6 +406,17 @@ impl ShardIdentity { } } + /// For use when creating ShardIdentity instances for new shards, where a creation request + /// specifies the ShardParameters that apply to all shards. + pub fn from_params(number: ShardNumber, params: &ShardParameters) -> Self { + Self { + number, + count: params.count, + layout: LAYOUT_V1, + stripe_size: params.stripe_size.unwrap_or(DEFAULT_STRIPE_SIZE), + } + } + fn is_broken(&self) -> bool { self.layout == LAYOUT_BROKEN } diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 5c7747d353..ec2fd9e09f 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -258,7 +258,7 @@ impl From for ApiError { SetNewTenantConfigError::GetTenant(tid) => { ApiError::NotFound(anyhow!("tenant {}", tid).into()) } - e @ SetNewTenantConfigError::Persist(_) => { + e @ (SetNewTenantConfigError::Persist(_) | SetNewTenantConfigError::Other(_)) => { ApiError::InternalServerError(anyhow::Error::new(e)) } } @@ -1152,6 +1152,7 @@ async fn tenant_create_handler( state.conf, tenant_conf, target_tenant_id, + request_data.shard_parameters, generation, state.tenant_resources(), &ctx, diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index e7aa6f4206..55eb6110ad 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -17,6 +17,7 @@ use enumset::EnumSet; use futures::stream::FuturesUnordered; use futures::FutureExt; use futures::StreamExt; +use pageserver_api::models::ShardParameters; use pageserver_api::models::TimelineState; use pageserver_api::shard::ShardIdentity; use pageserver_api::shard::TenantShardId; @@ -2655,10 +2656,11 @@ impl Tenant { } } - // Legacy configs are implicitly in attached state + // Legacy configs are implicitly in attached state, and do not support sharding Ok(LocationConf::attached_single( tenant_conf, Generation::none(), + &ShardParameters::default(), )) } else { // FIXME If the config file is not found, assume that we're attaching @@ -4068,6 +4070,7 @@ pub(crate) mod harness { AttachedTenantConf::try_from(LocationConf::attached_single( TenantConfOpt::from(self.tenant_conf), self.generation, + &ShardParameters::default(), )) .unwrap(), // This is a legacy/test code path: sharding isn't supported here. diff --git a/pageserver/src/tenant/config.rs b/pageserver/src/tenant/config.rs index e856047607..65f12388c3 100644 --- a/pageserver/src/tenant/config.rs +++ b/pageserver/src/tenant/config.rs @@ -9,7 +9,7 @@ //! may lead to a data loss. //! use anyhow::bail; -use pageserver_api::models::{self, EvictionPolicy}; +use pageserver_api::models::{self, EvictionPolicy, ShardParameters}; use pageserver_api::shard::{ShardCount, ShardIdentity, ShardNumber, ShardStripeSize}; use serde::de::IntoDeserializer; use serde::{Deserialize, Serialize}; @@ -167,14 +167,17 @@ impl LocationConf { /// For use when loading from a legacy configuration: presence of a tenant /// implies it is in AttachmentMode::Single, which used to be the only /// possible state. This function should eventually be removed. - pub(crate) fn attached_single(tenant_conf: TenantConfOpt, generation: Generation) -> Self { + pub(crate) fn attached_single( + tenant_conf: TenantConfOpt, + generation: Generation, + shard_params: &ShardParameters, + ) -> Self { Self { mode: LocationMode::Attached(AttachedLocationConfig { generation, attach_mode: AttachmentMode::Single, }), - // Legacy configuration loads are always from tenants created before sharding existed. - shard: ShardIdentity::unsharded(), + shard: ShardIdentity::from_params(ShardNumber(0), shard_params), tenant_conf, } } diff --git a/pageserver/src/tenant/mgr.rs b/pageserver/src/tenant/mgr.rs index 70b41b7b1f..d363d16055 100644 --- a/pageserver/src/tenant/mgr.rs +++ b/pageserver/src/tenant/mgr.rs @@ -3,7 +3,8 @@ use camino::{Utf8DirEntry, Utf8Path, Utf8PathBuf}; use pageserver_api::key::Key; -use pageserver_api::shard::{ShardIdentity, ShardNumber, TenantShardId}; +use pageserver_api::models::ShardParameters; +use pageserver_api::shard::{ShardCount, ShardIdentity, ShardNumber, TenantShardId}; use rand::{distributions::Alphanumeric, Rng}; use std::borrow::Cow; use std::collections::{BTreeMap, HashMap}; @@ -758,13 +759,21 @@ pub(crate) async fn create_tenant( conf: &'static PageServerConf, tenant_conf: TenantConfOpt, tenant_shard_id: TenantShardId, + shard_params: ShardParameters, generation: Generation, resources: TenantSharedResources, ctx: &RequestContext, ) -> Result, TenantMapInsertError> { - let location_conf = LocationConf::attached_single(tenant_conf, generation); + let location_conf = LocationConf::attached_single(tenant_conf, generation, &shard_params); + info!("Creating tenant at location {location_conf:?}"); + if shard_params.count.0 > 1 { + return Err(TenantMapInsertError::Other(anyhow::anyhow!( + "Only single-shard tenant creations may be serviced directly by a pageserver" + ))); + } + let slot_guard = tenant_map_acquire_slot(&tenant_shard_id, TenantSlotAcquireMode::MustNotExist)?; let tenant_path = super::create_tenant_files(conf, &location_conf, &tenant_shard_id).await?; @@ -799,6 +808,8 @@ pub(crate) enum SetNewTenantConfigError { GetTenant(#[from] GetTenantError), #[error(transparent)] Persist(anyhow::Error), + #[error(transparent)] + Other(anyhow::Error), } pub(crate) async fn set_new_tenant_config( @@ -812,10 +823,21 @@ pub(crate) async fn set_new_tenant_config( info!("configuring tenant {tenant_id}"); let tenant = get_tenant(tenant_shard_id, true)?; + if tenant.tenant_shard_id().shard_count > ShardCount(0) { + // Note that we use ShardParameters::default below. + return Err(SetNewTenantConfigError::Other(anyhow::anyhow!( + "This API may only be used on single-sharded tenants, use the /location_config API for sharded tenants" + ))); + } + // This is a legacy API that only operates on attached tenants: the preferred // API to use is the location_config/ endpoint, which lets the caller provide // the full LocationConf. - let location_conf = LocationConf::attached_single(new_tenant_conf, tenant.generation); + let location_conf = LocationConf::attached_single( + new_tenant_conf, + tenant.generation, + &ShardParameters::default(), + ); Tenant::persist_tenant_config(conf, &tenant_shard_id, &location_conf) .await @@ -1662,10 +1684,11 @@ pub(crate) async fn attach_tenant( ) -> Result<(), TenantMapInsertError> { // This is a legacy API (replaced by `/location_conf`). It does not support sharding let tenant_shard_id = TenantShardId::unsharded(tenant_id); + let shard_params = ShardParameters::default(); let slot_guard = tenant_map_acquire_slot(&tenant_shard_id, TenantSlotAcquireMode::MustNotExist)?; - let location_conf = LocationConf::attached_single(tenant_conf, generation); + let location_conf = LocationConf::attached_single(tenant_conf, generation, &shard_params); let tenant_dir = create_tenant_files(conf, &location_conf, &tenant_shard_id).await?; // TODO: tenant directory remains on disk if we bail out from here on. // See https://github.com/neondatabase/neon/issues/4233