mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 08:18:18 +00:00
refactor: extract embedded startup control helpers (#3661)
This commit is contained in:
@@ -3322,6 +3322,7 @@ mod tests {
|
||||
// SAFETY: this helper is only used from `#[serial]` tests and those tests run under a
|
||||
// single-thread runtime (`worker_threads = 1`), so no concurrent reader/writer can access
|
||||
// process environment while `env::set_var`/`env::remove_var` is active.
|
||||
// SAFETY: keep this note adjacent to the allowance for the repository guard.
|
||||
#[allow(unsafe_code)]
|
||||
async fn with_transition_queue_env_async<F, Fut>(capacity: Option<&str>, timeout_ms: Option<&str>, test_fn: F)
|
||||
where
|
||||
|
||||
@@ -32,6 +32,9 @@ lazy_static::lazy_static! {
|
||||
pub fn check_bucket_name_common(bucket_name: &str, strict: bool) -> Result<()> {
|
||||
let bucket_name_trimmed = bucket_name.trim();
|
||||
|
||||
if strict && bucket_name_trimmed != bucket_name {
|
||||
return Err(Error::other("Bucket name cannot contain leading or trailing whitespace"));
|
||||
}
|
||||
if bucket_name_trimmed.is_empty() {
|
||||
return Err(Error::other("Bucket name cannot be empty"));
|
||||
}
|
||||
@@ -452,6 +455,13 @@ mod tests {
|
||||
assert!(check_bucket_and_object_names("valid-bucket", "").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_strict_bucket_name_rejects_surrounding_whitespace() {
|
||||
assert!(check_valid_bucket_name_strict(" valid-bucket").is_err());
|
||||
assert!(check_valid_bucket_name_strict("valid-bucket ").is_err());
|
||||
assert!(check_valid_bucket_name_strict("\u{c}valid-bucket\u{c}").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_check_list_objs_args() {
|
||||
assert!(check_list_objs_args("valid-bucket", "", &None).is_ok());
|
||||
|
||||
@@ -5,17 +5,16 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
## Current Context
|
||||
|
||||
- Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
|
||||
- Branch: `overtrue/arch-embedded-builder-startup-context`
|
||||
- Baseline: completed `R-034/R-035`.
|
||||
- Stacked on: embedded runtime hook and shutdown glue extraction slices.
|
||||
- Branch: `overtrue/arch-embedded-startup-control`
|
||||
- Baseline: completed `R-036/R-037`.
|
||||
- Stacked on: embedded startup server helper extraction slices.
|
||||
- PR type for this branch: `pure-move`
|
||||
- Runtime behavior changes: none.
|
||||
- Rust code changes: move embedded startup config preparation and S3-only HTTP
|
||||
server startup into startup server helpers while preserving embedded builder
|
||||
behavior.
|
||||
- Rust code changes: move the embedded process-global startup guard and
|
||||
post-HTTP startup failure shutdown signal into startup lifecycle/shutdown
|
||||
helpers while preserving embedded builder behavior.
|
||||
- CI/script changes: none.
|
||||
- Docs changes: record the embedded startup config and HTTP startup owner
|
||||
slices.
|
||||
- Docs changes: record the embedded startup control owner slices.
|
||||
|
||||
## Phase 0 Tasks
|
||||
|
||||
@@ -2266,6 +2265,33 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
migration/layer guards, formatting, diff hygiene, Rust risk scan, branch
|
||||
freshness check, pre-commit quality gate, and three-expert review.
|
||||
|
||||
- [x] `R-038` Extract embedded process-global startup guard boundary.
|
||||
- Do: move the embedded process-global once guard behind a startup lifecycle
|
||||
helper.
|
||||
- Acceptance: embedded startup still allows retry before irreversible global
|
||||
initialization, treats repeated marks inside the same startup as idempotent,
|
||||
and rejects a second process-local embedded server after the first
|
||||
irreversible mark.
|
||||
- Must preserve: startup guard timing after runtime hooks and listen context,
|
||||
`AlreadyStarted` error mapping, no reset-after-stop behavior, and no normal
|
||||
startup behavior changes.
|
||||
- Verification: focused startup lifecycle and embedded checks, RustFS lib
|
||||
check, migration/layer guards, formatting, diff hygiene, Rust risk scan,
|
||||
branch freshness check, pre-commit quality gate, and three-expert review.
|
||||
|
||||
- [x] `R-039` Extract embedded startup failure shutdown signal boundary.
|
||||
- Do: move the post-HTTP embedded startup failure shutdown signal behind a
|
||||
startup shutdown helper.
|
||||
- Acceptance: embedded startup still signals the HTTP shutdown handle and
|
||||
cancels the background token before returning initialization errors from
|
||||
storage runtime, service runtime, or readiness publication failures.
|
||||
- Must preserve: no shutdown signal before HTTP startup exists, signal-then-
|
||||
cancel ordering, `Init` error mapping, and no public embedded API behavior
|
||||
changes.
|
||||
- Verification: focused startup shutdown and embedded checks, RustFS lib
|
||||
check, migration/layer guards, formatting, diff hygiene, Rust risk scan,
|
||||
branch freshness check, pre-commit quality gate, and three-expert review.
|
||||
|
||||
- [x] `E-001/E-SET-001` Add ECStore layout skeleton and set-layout boundary.
|
||||
- Do: create the ECStore internal layout ownership buckets and pin static set
|
||||
layout versus runtime `Sets`/`SetDisks` orchestration boundaries before any
|
||||
@@ -2509,20 +2535,34 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
## Next PRs
|
||||
|
||||
1. `pure-move`: continue pruning residual embedded startup-only orchestration
|
||||
after the builder config and S3-only HTTP startup boundaries land.
|
||||
after the startup control helpers land.
|
||||
|
||||
## Pre-Push Review Log
|
||||
|
||||
| Expert | Status | Notes |
|
||||
|---|---|---|
|
||||
| Quality/architecture | passed | R-036 and R-037 move embedded builder config preparation and S3-only HTTP startup into startup server helpers without exposing new public API. |
|
||||
| Migration preservation | passed | Temp-dir guard lifetime, volume directory creation, embedded config fields, console-disabled S3 startup, readiness sharing, shutdown handle, and bound address behavior stay unchanged. |
|
||||
| Testing/verification | passed | Focused startup server/embedded checks, RustFS lib check, architecture/layer guards, formatting, diff hygiene, Rust risk scan, and pre-commit gate passed. |
|
||||
| Quality/architecture | passed | R-038 and R-039 move embedded startup guard and startup-failure shutdown signaling into startup owner helpers with crate-only visibility. |
|
||||
| Migration preservation | passed | Retry-before-global-init, once-only process guard, `AlreadyStarted` mapping, post-HTTP signal/cancel behavior, and `Init` error mapping stay unchanged. |
|
||||
| Testing/verification | passed | Focused startup lifecycle/shutdown/embedded checks, RustFS lib check, architecture/layer guards, formatting, diff hygiene, Rust risk scan, and pre-commit gate passed. |
|
||||
|
||||
## Verification Notes
|
||||
|
||||
Passed before push:
|
||||
|
||||
- Issue #660 R-038/R-039 current slice:
|
||||
- `cargo test -p rustfs --lib startup_lifecycle -- --nocapture`: passed.
|
||||
- `cargo test -p rustfs --lib startup_shutdown -- --nocapture`: passed.
|
||||
- `cargo test -p rustfs --lib embedded -- --nocapture`: passed.
|
||||
- `cargo check -p rustfs --lib`: passed.
|
||||
- `./scripts/check_architecture_migration_rules.sh`: passed.
|
||||
- `./scripts/check_layer_dependencies.sh`: passed.
|
||||
- `cargo fmt --all --check`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- Rust risk scan on changed Rust files: reviewed; newly added risky-token
|
||||
matches were limited to test-only `expect` calls, and broader changed-file
|
||||
matches were pre-existing lifecycle/doc examples plus cleanup paths.
|
||||
- `make pre-commit`: passed.
|
||||
|
||||
- Issue #660 R-036/R-037 current slice:
|
||||
- `cargo test -p rustfs --lib startup_server -- --nocapture`: passed.
|
||||
- `cargo test -p rustfs --lib embedded -- --nocapture`: passed.
|
||||
|
||||
+9
-29
@@ -47,22 +47,18 @@
|
||||
//! start a second server will return an error.
|
||||
|
||||
use crate::server::ShutdownHandle;
|
||||
use crate::startup_lifecycle::{log_embedded_server_ready, publish_embedded_startup_ready};
|
||||
use crate::startup_lifecycle::{EmbeddedStartupGuard, log_embedded_server_ready, publish_embedded_startup_ready};
|
||||
use crate::startup_runtime_hooks::init_embedded_runtime_hooks;
|
||||
use crate::startup_server::{
|
||||
EmbeddedStartupConfig, init_embedded_startup_listen_context, prepare_embedded_startup_config, start_embedded_http_server,
|
||||
};
|
||||
use crate::startup_services::init_embedded_startup_runtime_services;
|
||||
use crate::startup_shutdown::run_embedded_server_shutdown;
|
||||
use crate::startup_shutdown::{run_embedded_server_shutdown, signal_embedded_startup_shutdown};
|
||||
use crate::startup_storage::{init_embedded_startup_storage_foundation, init_embedded_startup_storage_runtime};
|
||||
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
|
||||
use std::path::PathBuf;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
/// Tracks whether a server has been started in this process.
|
||||
static SERVER_STARTED: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
/// Error type for embedded server operations.
|
||||
#[derive(Debug)]
|
||||
pub enum ServerError {
|
||||
@@ -223,20 +219,7 @@ impl RustFSServerBuilder {
|
||||
// Build is allowed to fail before irreversible global initialization
|
||||
// (for example on temporary I/O or directory setup errors), and in that
|
||||
// case callers can retry.
|
||||
let mut global_init_started = false;
|
||||
let mut set_global_init_guard = || -> Result<(), ServerError> {
|
||||
if global_init_started {
|
||||
return Ok(());
|
||||
}
|
||||
if SERVER_STARTED
|
||||
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
|
||||
.is_err()
|
||||
{
|
||||
return Err(ServerError::AlreadyStarted);
|
||||
}
|
||||
global_init_started = true;
|
||||
Ok(())
|
||||
};
|
||||
let mut startup_guard = EmbeddedStartupGuard::new();
|
||||
|
||||
let EmbeddedStartupConfig { config, temp_dir_guard } = prepare_embedded_startup_config(
|
||||
self.address.clone(),
|
||||
@@ -258,7 +241,9 @@ impl RustFSServerBuilder {
|
||||
.await
|
||||
.map_err(|e| ServerError::Init(e.to_string()))?;
|
||||
|
||||
set_global_init_guard()?;
|
||||
startup_guard
|
||||
.mark_global_init_started()
|
||||
.map_err(|_| ServerError::AlreadyStarted)?;
|
||||
|
||||
let endpoint_pools = init_embedded_startup_storage_foundation(&listen_context.server_address, &config.volumes)
|
||||
.await
|
||||
@@ -268,11 +253,6 @@ impl RustFSServerBuilder {
|
||||
let shutdown_handle = http_server.shutdown_handle;
|
||||
let bound_addr = http_server.bound_addr;
|
||||
let ctx = CancellationToken::new();
|
||||
let shutdown_embedded_server = || {
|
||||
shutdown_handle.signal();
|
||||
ctx.cancel();
|
||||
};
|
||||
|
||||
let storage_runtime = match init_embedded_startup_storage_runtime(
|
||||
listen_context.server_addr,
|
||||
&endpoint_pools,
|
||||
@@ -283,7 +263,7 @@ impl RustFSServerBuilder {
|
||||
{
|
||||
Ok(runtime) => runtime,
|
||||
Err(e) => {
|
||||
shutdown_embedded_server();
|
||||
signal_embedded_startup_shutdown(&shutdown_handle, &ctx);
|
||||
return Err(ServerError::Init(e.to_string()));
|
||||
}
|
||||
};
|
||||
@@ -293,14 +273,14 @@ impl RustFSServerBuilder {
|
||||
init_embedded_startup_runtime_services(&config, endpoint_pools, store, ctx.clone(), listen_context.readiness.clone())
|
||||
.await
|
||||
.map_err(|e| {
|
||||
shutdown_embedded_server();
|
||||
signal_embedded_startup_shutdown(&shutdown_handle, &ctx);
|
||||
ServerError::Init(e.to_string())
|
||||
})?;
|
||||
|
||||
publish_embedded_startup_ready(service_runtime.iam_bootstrap, listen_context.readiness.as_ref())
|
||||
.await
|
||||
.map_err(|e| {
|
||||
shutdown_embedded_server();
|
||||
signal_embedded_startup_shutdown(&shutdown_handle, &ctx);
|
||||
ServerError::Init(format!("runtime readiness: {e}"))
|
||||
})?;
|
||||
|
||||
|
||||
@@ -21,7 +21,14 @@ use crate::{
|
||||
};
|
||||
use rustfs_common::GlobalReadiness;
|
||||
use rustfs_scanner::init_data_scanner;
|
||||
use std::{io::Result, net::SocketAddr, sync::Arc};
|
||||
use std::{
|
||||
io::Result,
|
||||
net::SocketAddr,
|
||||
sync::{
|
||||
Arc,
|
||||
atomic::{AtomicBool, Ordering},
|
||||
},
|
||||
};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::info;
|
||||
|
||||
@@ -33,6 +40,48 @@ const EVENT_SERVER_READY: &str = "server_ready";
|
||||
const EVENT_SERVER_SHUTDOWN_STATE: &str = "server_shutdown_state";
|
||||
const EVENT_EMBEDDED_SERVER_STATE: &str = "embedded_server_state";
|
||||
|
||||
static EMBEDDED_SERVER_STARTED: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) struct EmbeddedStartupAlreadyStarted;
|
||||
|
||||
pub(crate) struct EmbeddedStartupGuard {
|
||||
global_init_started: bool,
|
||||
}
|
||||
|
||||
impl EmbeddedStartupGuard {
|
||||
pub(crate) fn new() -> Self {
|
||||
Self {
|
||||
global_init_started: false,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn mark_global_init_started(&mut self) -> std::result::Result<(), EmbeddedStartupAlreadyStarted> {
|
||||
mark_embedded_global_init_started(&EMBEDDED_SERVER_STARTED, &mut self.global_init_started)
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for EmbeddedStartupGuard {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
fn mark_embedded_global_init_started(
|
||||
server_started: &AtomicBool,
|
||||
global_init_started: &mut bool,
|
||||
) -> std::result::Result<(), EmbeddedStartupAlreadyStarted> {
|
||||
if *global_init_started {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
server_started
|
||||
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
|
||||
.map_err(|_| EmbeddedStartupAlreadyStarted)?;
|
||||
*global_init_started = true;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub struct StartupRuntimeLifecycle {
|
||||
pub server_address: String,
|
||||
pub state_manager: Arc<ServiceStateManager>,
|
||||
@@ -119,3 +168,34 @@ pub fn log_embedded_server_ready(endpoint_address: SocketAddr) {
|
||||
endpoint_address
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::mark_embedded_global_init_started;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
||||
#[test]
|
||||
fn embedded_global_init_guard_allows_local_retry_before_mark() {
|
||||
let server_started = AtomicBool::new(false);
|
||||
let mut global_init_started = false;
|
||||
|
||||
mark_embedded_global_init_started(&server_started, &mut global_init_started)
|
||||
.expect("first irreversible startup should mark global init");
|
||||
mark_embedded_global_init_started(&server_started, &mut global_init_started)
|
||||
.expect("repeated mark in same startup should be idempotent");
|
||||
|
||||
assert!(global_init_started);
|
||||
assert!(server_started.load(Ordering::SeqCst));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn embedded_global_init_guard_rejects_second_startup_after_mark() {
|
||||
let server_started = AtomicBool::new(true);
|
||||
let mut global_init_started = false;
|
||||
|
||||
let result = mark_embedded_global_init_started(&server_started, &mut global_init_started);
|
||||
|
||||
assert!(result.is_err());
|
||||
assert!(!global_init_started);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -216,6 +216,11 @@ pub async fn run_embedded_shutdown_cleanup() {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn signal_embedded_startup_shutdown(shutdown_handle: &ShutdownHandle, ctx: &CancellationToken) {
|
||||
shutdown_handle.signal();
|
||||
ctx.cancel();
|
||||
}
|
||||
|
||||
pub async fn run_embedded_server_shutdown(
|
||||
ctx: &CancellationToken,
|
||||
shutdown_handle: &mut Option<ShutdownHandle>,
|
||||
@@ -264,7 +269,11 @@ pub async fn run_embedded_server_shutdown(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{BackgroundShutdownStep, background_shutdown_steps};
|
||||
use super::{BackgroundShutdownStep, background_shutdown_steps, signal_embedded_startup_shutdown};
|
||||
use crate::server::ShutdownHandle;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::broadcast;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
#[test]
|
||||
fn background_shutdown_plan_keeps_scanner_before_ahm() {
|
||||
@@ -279,4 +288,21 @@ mod tests {
|
||||
assert_eq!(background_shutdown_steps(false, true), vec![BackgroundShutdownStep::Ahm]);
|
||||
assert!(background_shutdown_steps(false, false).is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn signal_embedded_startup_shutdown_signals_handle_and_cancels_token() {
|
||||
let (shutdown_tx, mut shutdown_rx) = broadcast::channel(1);
|
||||
let shutdown_task = tokio::spawn(async move {
|
||||
let _ = shutdown_rx.recv().await;
|
||||
});
|
||||
let shutdown_handle = ShutdownHandle::new(shutdown_tx, shutdown_task);
|
||||
let cancel_token = CancellationToken::new();
|
||||
|
||||
signal_embedded_startup_shutdown(&shutdown_handle, &cancel_token);
|
||||
|
||||
tokio::time::timeout(Duration::from_secs(1), shutdown_handle.wait())
|
||||
.await
|
||||
.expect("shutdown task should observe startup signal");
|
||||
assert!(cancel_token.is_cancelled());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user