Skip to content

Commit

Permalink
it turns out storage controller needed NEON_REPO_DIR because it does …
Browse files Browse the repository at this point in the history
…LocalEnv::load_config()

fix that by adding a CLI flag to storage controller that adds a
dedicated "neon_local" mode
  • Loading branch information
problame committed Jun 14, 2024
1 parent 06d9f93 commit e14801e
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 8 deletions.
6 changes: 4 additions & 2 deletions control_plane/src/bin/neon_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ fn main() -> Result<()> {
handle_init(sub_args).map(Some)
} else {
// all other commands need an existing config
let mut env = LocalEnv::load_config().context("Error loading config")?;
let mut env =
LocalEnv::load_config(&local_env::base_path()).context("Error loading config")?;
let original_env = env.clone();

let rt = tokio::runtime::Builder::new_current_thread()
Expand Down Expand Up @@ -364,7 +365,8 @@ fn handle_init(init_match: &ArgMatches) -> anyhow::Result<LocalEnv> {

LocalEnv::init(init_conf, force)
.context("materialize initial neon_local environment on disk")?;
Ok(LocalEnv::load_config().expect("freshly written config should be loadable"))
Ok(LocalEnv::load_config(&local_env::base_path())
.expect("freshly written config should be loadable"))
}

/// The default pageserver is the one where CLI tenant/timeline operations are sent by default.
Expand Down
8 changes: 3 additions & 5 deletions control_plane/src/local_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,7 @@ impl LocalEnv {
}

/// Construct `Self` from on-disk state.
pub fn load_config() -> anyhow::Result<Self> {
let repopath = base_path();

pub fn load_config(repopath: &Path) -> anyhow::Result<Self> {
if !repopath.exists() {
bail!(
"Neon config is not found in {}. You need to run 'neon_local init' first",
Expand Down Expand Up @@ -461,7 +459,7 @@ impl LocalEnv {
branch_name_mappings,
} = on_disk_config;
LocalEnv {
base_data_dir: repopath.clone(),
base_data_dir: repopath.to_owned(),
pg_distrib_dir,
neon_distrib_dir,
default_tenant_id,
Expand All @@ -482,7 +480,7 @@ impl LocalEnv {
"we ensure this during deserialization"
);
env.pageservers = {
let iter = std::fs::read_dir(&repopath).context("open dir")?;
let iter = std::fs::read_dir(repopath).context("open dir")?;
let mut pageservers = Vec::new();
for res in iter {
let dentry = res?;
Expand Down
5 changes: 5 additions & 0 deletions control_plane/src/storage_controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ impl StorageController {
args.push(format!("--split-threshold={split_threshold}"))
}

args.push(format!(
"--neon-local-repo-dir={}",
self.env.base_data_dir.display()
));

background_process::start_process(
COMMAND,
&self.env.base_data_dir,
Expand Down
8 changes: 7 additions & 1 deletion storage_controller/src/compute_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,13 @@ impl ComputeHook {
// all calls to this function
let _locked = self.neon_local_lock.lock().await;

let env = match LocalEnv::load_config() {
let Some(repo_dir) = self.config.neon_local_repo_dir.as_deref() else {
tracing::warn!(
"neon_local_repo_dir not set, likely a bug in neon_local; skipping compute update"
);
return Ok(());
};
let env = match LocalEnv::load_config(repo_dir) {
Ok(e) => e,
Err(e) => {
tracing::warn!("Couldn't load neon_local config, skipping compute update ({e})");
Expand Down
8 changes: 8 additions & 0 deletions storage_controller/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use clap::Parser;
use diesel::Connection;
use metrics::launch_timestamp::LaunchTimestamp;
use metrics::BuildInfo;
use std::path::PathBuf;
use std::sync::Arc;
use storage_controller::http::make_router;
use storage_controller::metrics::preinitialize_metrics;
Expand Down Expand Up @@ -77,6 +78,12 @@ struct Cli {
/// How long to wait for the initial database connection to be available.
#[arg(long, default_value = "5s")]
db_connect_timeout: humantime::Duration,

/// `neon_local` sets this to the path of the neon_local repo dir.
/// Only relevant for testing.
// TODO: make `cfg(feature = "testing")`
#[arg(long)]
neon_local_repo_dir: Option<PathBuf>,
}

enum StrictMode {
Expand Down Expand Up @@ -260,6 +267,7 @@ async fn async_main() -> anyhow::Result<()> {
.reconciler_concurrency
.unwrap_or(RECONCILER_CONCURRENCY_DEFAULT),
split_threshold: args.split_threshold,
neon_local_repo_dir: args.neon_local_repo_dir,
};

// After loading secrets & config, but before starting anything else, apply database migrations
Expand Down
4 changes: 4 additions & 0 deletions storage_controller/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::{
borrow::Cow,
cmp::Ordering,
collections::{BTreeMap, HashMap, HashSet},
path::PathBuf,
str::FromStr,
sync::Arc,
time::{Duration, Instant},
Expand Down Expand Up @@ -226,6 +227,9 @@ pub struct Config {
/// How large must a shard grow in bytes before we split it?
/// None disables auto-splitting.
pub split_threshold: Option<u64>,

// TODO: make this cfg(feature = "testing")
pub neon_local_repo_dir: Option<PathBuf>,
}

impl From<DatabaseError> for ApiError {
Expand Down

0 comments on commit e14801e

Please sign in to comment.