Compare commits

..

5 Commits

Author SHA1 Message Date
houseme 4b7a1ac050 Merge branch 'main' into fix/b5-t1-a1-replication-deny-edit 2026-08-23 12:28:13 +08:00
唐小鸭 74171bd673 fix(replication): pass site peer ids into the bucket usecase from the interface layer
The review fix made the bucket usecase read the site-replication peer set
through the admin handlers, an app->interface import the layer guard
rejects. The S3 handlers (interface) now read the peer set and pass it in,
so the usecase stays a pure function of its inputs; a state-read failure
still fails the edit closed, just one layer up.
2026-08-23 10:13:59 +08:00
唐小鸭 a42d81b26a fix(replication): keep operator rule priorities across site rule merges
Merging stored site-replication rules into a PutBucketReplication body
renumbered every rule 1..n in list order, rewriting the submitted policy:
overlapping same-target rules submitted as priority 5 then 1 became 1
then 2, so the delete-marker-disabled rule won the replication decision.
The reconciler and the peer-removal prune renumbered the same way.

Operator priorities now stay verbatim everywhere; only the reconciler's
derived rules move, to the lowest priorities no operator rule uses, via
one pure helper shared by the S3 edit merge, the peer ingestion merge,
the reconciler pass and the prune. Being a pure function of the rule
list it is idempotent, so the reconciler's no-op check still holds after
a merged write, and an on-disk config in the historical layout (operator
rules 1..k, site rules k+1..n) yields the same bytes, so nothing is
rewritten on upgrade.
2026-08-23 00:58:29 +08:00
唐小鸭 a3733c1a1c fix(replication): scope site-owned rule detection to reconciler-derived rules
The `site-repl-*` prefix alone classified any rule as site-owned, so on a
bucket outside site replication an owner's `site-repl-user` rule survived
DeleteBucketReplication (rule and target kept, success returned). Rule ids
do not reserve that namespace.

A rule is reconciler-owned only when it matches what the reconciler
derives: id `site-repl-<deployment id>` for a current remote site
replication peer and a destination ARN naming that same deployment id.
The S3 put/delete path reads the remote peer set (empty when site
replication is disabled) and keeps exactly those rules; everything else
is operator state the request replaces or deletes. An incoming rule that
claims a current peer's id is dropped so the reconciler rule's id stays
unique. The peer ingestion path and the reconciler keep their prefix
predicate unchanged.
2026-08-23 00:53:38 +08:00
唐小鸭 ce9b69d811 fix(replication): deny non-owner replication config edits under site replication
Under site replication a user holding only bucket-scoped
s3:PutReplicationConfiguration could rewrite or erase the operator-managed
site-repl-* rules, with the change broadcast to every peer (backlog#1948,
audit A1/P2-17).

- Gate PutBucketReplication/DeleteBucketReplication in the S3 handlers:
  when site replication is enabled and the requester is not the owner,
  return MinIO-parity XMinioReplicationDenyEdit (HTTP 400). The gate runs
  after policy authorization and only on the external S3 path; the
  reconciler and peer bucket-meta ingestion are unaffected.
- Defense in depth in the bucket usecase: PUT merges the incoming config
  with the stored site-repl-* rules (same merge as peer ingestion) instead
  of overwriting verbatim; DELETE keeps the site-repl-* rules and never
  garbage-collects a bucket target a surviving site-replication rule still
  references.
- Move is_site_replication_rule / merge_incoming_replication_config /
  replication_target_arn_deployment_id from the admin site-replication
  handler down to rustfs-replication so the app layer can reuse them
  without new layering violations.
2026-08-21 19:17:34 +08:00
35 changed files with 1083 additions and 4206 deletions
+6 -3
View File
@@ -39,10 +39,11 @@ jobs:
env:
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
steps:
- name: Checkout repository
- name: Checkout main branch
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
with:
persist-credentials: false
ref: main
- name: Setup Rust environment
uses: ./.github/actions/setup
@@ -88,10 +89,11 @@ jobs:
# either casing.
NO_PROXY: 127.0.0.1,localhost
steps:
- name: Checkout repository
- name: Checkout main branch
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
with:
persist-credentials: false
ref: main
- name: Setup Rust environment
uses: ./.github/actions/setup
@@ -176,10 +178,11 @@ jobs:
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
NO_PROXY: 127.0.0.1,localhost
steps:
- name: Checkout repository
- name: Checkout main branch
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
with:
persist-credentials: false
ref: main
- name: Setup Rust environment
uses: ./.github/actions/setup
+10 -15
View File
@@ -196,15 +196,16 @@ pub mod bucket {
ReplicationOperation, ReplicationPoolTrait, ReplicationPriority, ReplicationQueueAdmission, ReplicationScannerBridge,
ReplicationState, ReplicationStats, ReplicationStatusType, ReplicationStorage, ReplicationTargetValidationError,
ReplicationType, ResyncOpts, ResyncStatusType, RuntimeReplicationTargetBacklog, TargetReplicationResyncStatus,
VersionPurgeStatusType, XferStats, commit_force_delete_intent, complete_force_delete_intent,
delete_replication_state_from_config, delete_replication_version_id, get_global_replication_pool,
get_global_replication_stats, get_proxy_targets, init_background_replication,
invalid_replication_config_status_field, persist_force_delete_intent, read_durable_mrf_backlog,
replication_state_to_filemeta, replication_status_to_filemeta, replication_statuses_map, replication_target_arns,
resync_start_conflict_id, should_remove_replication_target, should_schedule_delete_replication,
should_use_existing_delete_replication_info, should_use_existing_delete_replication_source,
unsupported_replication_config_field, validate_replication_config_structure, validate_replication_config_target_arns,
version_purge_status_to_filemeta,
VersionPurgeStatusType, XferStats, assign_site_replication_rule_priorities, commit_force_delete_intent,
complete_force_delete_intent, delete_replication_state_from_config, delete_replication_version_id,
get_global_replication_pool, get_global_replication_stats, get_proxy_targets, init_background_replication,
invalid_replication_config_status_field, is_site_replication_rule, merge_incoming_replication_config,
merge_user_replication_config, persist_force_delete_intent, read_durable_mrf_backlog, replication_state_to_filemeta,
replication_status_to_filemeta, replication_statuses_map, replication_target_arn_deployment_id,
replication_target_arns, resync_start_conflict_id, should_remove_replication_target,
should_schedule_delete_replication, should_use_existing_delete_replication_info,
should_use_existing_delete_replication_source, unsupported_replication_config_field,
validate_replication_config_structure, validate_replication_config_target_arns, version_purge_status_to_filemeta,
};
}
@@ -440,12 +441,6 @@ pub mod rebalance {
RebalanceMeta, RebalanceStats, RebalanceStopPropagationRecord, decode_rebalance_stop_propagation_record,
encode_rebalance_stop_propagation_record,
};
#[cfg(feature = "test-util")]
pub mod test_util {
pub use crate::services::rebalance::PausedRebalanceEntryTestFixture;
pub use crate::services::rebalance::test_store_with_persisted_rebalance_meta;
}
}
pub mod rio {
@@ -4324,16 +4324,6 @@ pub async fn expire_transitioned_object(
lc_event: &lifecycle::Event,
_src: &LcEventSrc,
bucket_incarnation_id: Uuid,
) -> Result<ObjectInfo, std::io::Error> {
expire_transitioned_object_with_lock_lost_signal(api, oi, lc_event, bucket_incarnation_id, None).await
}
async fn expire_transitioned_object_with_lock_lost_signal(
api: Arc<ECStore>,
oi: &ObjectInfo,
lc_event: &lifecycle::Event,
bucket_incarnation_id: Uuid,
lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
) -> Result<ObjectInfo, std::io::Error> {
let publication_guard = lifecycle_expiry_publication_guard(&api, oi, bucket_incarnation_id)
.await
@@ -4345,9 +4335,6 @@ async fn expire_transitioned_object_with_lock_lost_signal(
let mut opts = transitioned_object_delete_opts(oi, lc_event.action, versioned, version_suspended, bucket_incarnation_id)
.map_err(std::io::Error::other)?;
opts.add_namespace_lock_guard(&publication_guard);
if let Some(signal) = lock_lost_signal {
opts.add_namespace_lock_lost_signal(signal);
}
opts.delete_replication_config_snapshot = Some(Arc::new(snapshot));
//let tags = LcAuditEvent::new(src, lcEvent).Tags();
if lc_event.action.delete_restored() {
@@ -5006,32 +4993,12 @@ pub async fn apply_expiry_on_transitioned_object(
lc_event: &lifecycle::Event,
src: &LcEventSrc,
bucket_incarnation_id: Uuid,
) -> bool {
apply_expiry_on_transitioned_object_with_lock_lost_signal(api, oi, lc_event, src, bucket_incarnation_id, None).await
}
async fn apply_expiry_on_transitioned_object_with_lock_lost_signal(
api: Arc<ECStore>,
oi: &ObjectInfo,
lc_event: &lifecycle::Event,
_src: &LcEventSrc,
bucket_incarnation_id: Uuid,
lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
) -> bool {
if lc_event.action.delete_all() {
return apply_expiry_on_non_transitioned_objects_with_lock_lost_signal(
api,
oi,
lc_event,
bucket_incarnation_id,
lock_lost_signal,
)
.await;
return apply_expiry_on_non_transitioned_objects(api, oi, lc_event, src, bucket_incarnation_id).await;
}
let time_ilm = Metrics::time_ilm(lc_event.action);
if let Err(_err) =
expire_transitioned_object_with_lock_lost_signal(api, oi, lc_event, bucket_incarnation_id, lock_lost_signal).await
{
if let Err(_err) = expire_transitioned_object(api, oi, lc_event, src, bucket_incarnation_id).await {
return false;
}
time_ilm(1)();
@@ -5045,16 +5012,6 @@ pub async fn apply_expiry_on_non_transitioned_objects(
lc_event: &lifecycle::Event,
_src: &LcEventSrc,
bucket_incarnation_id: Uuid,
) -> bool {
apply_expiry_on_non_transitioned_objects_with_lock_lost_signal(api, oi, lc_event, bucket_incarnation_id, None).await
}
async fn apply_expiry_on_non_transitioned_objects_with_lock_lost_signal(
api: Arc<ECStore>,
oi: &ObjectInfo,
lc_event: &lifecycle::Event,
bucket_incarnation_id: Uuid,
lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
) -> bool {
let Some(publication_guard) = lifecycle_expiry_publication_guard(&api, oi, bucket_incarnation_id).await else {
return false;
@@ -5085,9 +5042,6 @@ async fn apply_expiry_on_non_transitioned_objects_with_lock_lost_signal(
..Default::default()
};
opts.add_namespace_lock_guard(&publication_guard);
if let Some(signal) = lock_lost_signal {
opts.add_namespace_lock_lost_signal(signal);
}
if lc_event.action.delete_versioned() {
opts.version_id = oi.version_id.map(|v| v.to_string());
@@ -5169,61 +5123,6 @@ async fn enqueue_expiry_rule_with_incarnation(
expiry_state.enqueue_by_days(oi, event, src, bucket_incarnation_id)
}
fn lifecycle_expiry_object_matches(current: &ObjectInfo, expected: &ObjectInfo) -> bool {
current.version_id == expected.version_id
&& current.data_dir == expected.data_dir
&& current.mod_time == expected.mod_time
&& current.etag == expected.etag
&& current.delete_marker == expected.delete_marker
&& current.transitioned_object.name == expected.transitioned_object.name
&& current.transitioned_object.version_id == expected.transitioned_object.version_id
&& current.transitioned_object.tier == expected.transitioned_object.tier
&& current.transitioned_object.status == expected.transitioned_object.status
&& current.restore_expires == expected.restore_expires
}
pub(crate) async fn apply_expiry_rule_for_data_movement(
api: Arc<ECStore>,
event: &lifecycle::Event,
src: &LcEventSrc,
oi: &ObjectInfo,
lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
) -> bool {
let Ok(_lifecycle_guard) = api.acquire_bucket_lifecycle_read_lock(&oi.bucket).await else {
return false;
};
let Ok(bucket_incarnation_id) = api.bucket_incarnation_id_from_disk(&oi.bucket).await else {
return false;
};
let current = match api
.get_object_info(
&oi.bucket,
&oi.name,
&ObjectOptions {
version_id: oi.version_id.map(|version_id| version_id.to_string()),
versioned: oi.version_id.is_some(),
expected_bucket_incarnation_id: Some(bucket_incarnation_id),
..Default::default()
},
)
.await
{
Ok(current) => current,
Err(_) => return false,
};
if !lifecycle_expiry_object_matches(&current, oi) {
return false;
}
if oi.transitioned_object.status.is_empty() {
apply_expiry_on_non_transitioned_objects_with_lock_lost_signal(api, oi, event, bucket_incarnation_id, lock_lost_signal)
.await
} else {
apply_expiry_on_transitioned_object_with_lock_lost_signal(api, oi, event, src, bucket_incarnation_id, lock_lost_signal)
.await
}
}
pub(crate) async fn apply_expiry_rule_in(api: Arc<ECStore>, event: &lifecycle::Event, src: &LcEventSrc, oi: &ObjectInfo) -> bool {
let Ok(_lifecycle_guard) = api.acquire_bucket_lifecycle_read_lock(&oi.bucket).await else {
return false;
@@ -5247,7 +5146,17 @@ pub(crate) async fn apply_expiry_rule_in(api: Arc<ECStore>, event: &lifecycle::E
Ok(current) => current,
Err(_) => return false,
};
if !lifecycle_expiry_object_matches(&current, oi) {
if current.version_id != oi.version_id
|| current.data_dir != oi.data_dir
|| current.mod_time != oi.mod_time
|| current.etag != oi.etag
|| current.delete_marker != oi.delete_marker
|| current.transitioned_object.name != oi.transitioned_object.name
|| current.transitioned_object.version_id != oi.transitioned_object.version_id
|| current.transitioned_object.tier != oi.transitioned_object.tier
|| current.transitioned_object.status != oi.transitioned_object.status
|| current.restore_expires != oi.restore_expires
{
return false;
}
enqueue_expiry_rule_with_incarnation(event, src, oi, bucket_incarnation_id).await
+4 -2
View File
@@ -47,8 +47,10 @@ pub use replication_config_boundary::{
ObjectOpts, REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION, REMOTE_TARGET_UNSUPPORTED_FIELDS, REMOTE_TARGET_WRITABLE_FIELDS,
REPLICATION_CAPABILITY_CONTRACT_VERSION, REPLICATION_READ_ONLY_HISTORICAL_FIELDS, REPLICATION_WRITABLE_FIELDS,
ReplicationConfigStructureError, ReplicationConfigurationExt, ReplicationTargetValidationError,
invalid_replication_config_status_field, replication_target_arns, should_remove_replication_target,
unsupported_replication_config_field, validate_replication_config_structure, validate_replication_config_target_arns,
assign_site_replication_rule_priorities, invalid_replication_config_status_field, is_site_replication_rule,
merge_incoming_replication_config, merge_user_replication_config, replication_target_arn_deployment_id,
replication_target_arns, should_remove_replication_target, unsupported_replication_config_field,
validate_replication_config_structure, validate_replication_config_target_arns,
};
pub(crate) use replication_filemeta_boundary::version_purge_statuses_map;
pub use replication_filemeta_boundary::{
@@ -16,6 +16,8 @@ pub use rustfs_replication::{
ObjectOpts, REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION, REMOTE_TARGET_UNSUPPORTED_FIELDS, REMOTE_TARGET_WRITABLE_FIELDS,
REPLICATION_CAPABILITY_CONTRACT_VERSION, REPLICATION_READ_ONLY_HISTORICAL_FIELDS, REPLICATION_WRITABLE_FIELDS,
ReplicationConfigStructureError, ReplicationConfigurationExt, ReplicationRuleExt, ReplicationTargetValidationError,
invalid_replication_config_status_field, replication_target_arns, should_remove_replication_target,
unsupported_replication_config_field, validate_replication_config_structure, validate_replication_config_target_arns,
assign_site_replication_rule_priorities, invalid_replication_config_status_field, is_site_replication_rule,
merge_incoming_replication_config, merge_user_replication_config, replication_target_arn_deployment_id,
replication_target_arns, should_remove_replication_target, unsupported_replication_config_field,
validate_replication_config_structure, validate_replication_config_target_arns,
};
+30 -607
View File
@@ -19,8 +19,8 @@ use crate::bucket::{
LifecycleExpiryConfigs,
bucket_lifecycle_audit::LcEventSrc,
bucket_lifecycle_ops::{
LifecycleOps, apply_expiry_on_transitioned_object, apply_expiry_rule_for_data_movement, apply_expiry_rule_in,
eval_action_from_lifecycle, lifecycle_delete_all_versions_blocked_by_replication,
LifecycleOps, apply_expiry_on_transitioned_object, apply_expiry_rule_in, eval_action_from_lifecycle,
lifecycle_delete_all_versions_blocked_by_replication,
},
get_expiry_configs,
lifecycle::IlmAction,
@@ -28,9 +28,7 @@ use crate::bucket::{
metadata_sys,
};
use crate::cache_value::metacache_set::{ListPathRawOptions, list_path_raw};
use crate::config::com::{
CONFIG_PREFIX, read_config, read_config_no_lock, save_config, save_config_with_opts, save_config_with_opts_quiet,
};
use crate::config::com::{CONFIG_PREFIX, read_config, read_config_no_lock, save_config, save_config_with_opts};
use crate::data_movement;
use crate::data_movement::backpressure::{self, DataMovementOperation};
use crate::data_usage::DATA_USAGE_CACHE_NAME;
@@ -50,6 +48,7 @@ use crate::storage_api_contracts::{
admin::StorageAdminApi,
bucket::{BucketOperations, BucketOptions, MakeBucketOptions},
heal::HealOperations as _,
namespace::NamespaceLocking as _,
object::{EcstoreObjectIO, ObjectIO as _, ObjectOperations as _},
};
use crate::{core::sets::Sets, store::ECStore};
@@ -1087,7 +1086,7 @@ fn resolve_start_decommission_pool_meta_reload_result(result: Result<()>) -> Res
resolve_decommission_pool_meta_reload_result(result, "start_decommission")
}
fn activation_rebalance_meta_lock_error(err: rustfs_lock::LockError) -> Error {
fn decommission_rebalance_meta_lock_error(err: rustfs_lock::LockError) -> Error {
match err {
rustfs_lock::LockError::QuorumNotReached { required, achieved } => Error::NamespaceLockQuorumUnavailable {
mode: "write",
@@ -1097,12 +1096,12 @@ fn activation_rebalance_meta_lock_error(err: rustfs_lock::LockError) -> Error {
achieved,
},
other => Error::other(format!(
"failed to acquire rebalance activation lock on {RUSTFS_META_BUCKET}/{REBAL_META_NAME}: {other}"
"failed to acquire rebalance metadata write lock before decommission start on {RUSTFS_META_BUCKET}/{REBAL_META_NAME}: {other}"
)),
}
}
fn activation_pool_meta_lock_error(err: rustfs_lock::LockError) -> Error {
fn decommission_pool_meta_lock_error(err: rustfs_lock::LockError) -> Error {
match err {
rustfs_lock::LockError::QuorumNotReached { required, achieved } => Error::NamespaceLockQuorumUnavailable {
mode: "write",
@@ -1112,227 +1111,11 @@ fn activation_pool_meta_lock_error(err: rustfs_lock::LockError) -> Error {
achieved,
},
other => Error::other(format!(
"failed to acquire pool activation lock on {RUSTFS_META_BUCKET}/{POOL_META_NAME}: {other}"
"failed to acquire pool metadata write lock before decommission start on {RUSTFS_META_BUCKET}/{POOL_META_NAME}: {other}"
)),
}
}
pub(crate) struct PoolRebalanceActivationFence {
pool_meta_guard: rustfs_lock::NamespaceLockGuard,
rebalance_meta_guard: rustfs_lock::NamespaceLockGuard,
#[cfg(test)]
forced_lost: Arc<AtomicBool>,
}
impl PoolRebalanceActivationFence {
pub(crate) fn ensure_held(&self) -> Result<()> {
#[cfg(test)]
let forced_lost = self.forced_lost.load(Ordering::Acquire);
#[cfg(not(test))]
let forced_lost = false;
if forced_lost || self.pool_meta_guard.is_lock_lost() || self.rebalance_meta_guard.is_lock_lost() {
return Err(Error::other("activation lock lost before metadata commit or worker admission"));
}
Ok(())
}
pub(crate) fn add_namespace_lock_fence(&self, opts: &mut ObjectOptions) {
opts.add_namespace_lock_guard(&self.pool_meta_guard);
opts.add_namespace_lock_guard(&self.rebalance_meta_guard);
}
#[cfg(test)]
fn force_lost_for_test(&self) {
self.forced_lost.store(true, Ordering::Release);
}
}
pub(crate) async fn acquire_pool_rebalance_activation_locks<S>(pool: Arc<S>) -> Result<PoolRebalanceActivationFence>
where
S: crate::storage_api_contracts::namespace::NamespaceLocking<
Error = Error,
NamespaceLock = rustfs_lock::NamespaceLockWrapper,
>,
{
// Activation lock order is always pool.bin -> rebalance.bin.
let pool_meta_lock = pool.new_ns_lock(RUSTFS_META_BUCKET, POOL_META_NAME).await?;
let pool_meta_guard = pool_meta_lock
.get_write_lock(get_lock_acquire_timeout())
.await
.map_err(activation_pool_meta_lock_error)?;
let rebalance_meta_lock = pool.new_ns_lock(RUSTFS_META_BUCKET, REBAL_META_NAME).await?;
let rebalance_meta_guard = rebalance_meta_lock
.get_write_lock(get_lock_acquire_timeout())
.await
.map_err(activation_rebalance_meta_lock_error)?;
Ok(PoolRebalanceActivationFence {
pool_meta_guard,
rebalance_meta_guard,
#[cfg(test)]
forced_lost: Arc::new(AtomicBool::new(false)),
})
}
#[cfg(test)]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub(crate) enum PoolActivationStartKind {
Rebalance,
Decommission,
}
#[cfg(test)]
struct PoolActivationDurableSaveBarrierState {
pool_key: usize,
arrived: tokio::sync::Notify,
release: tokio::sync::Notify,
}
#[cfg(test)]
static POOL_ACTIVATION_DURABLE_SAVE_BARRIER: std::sync::OnceLock<
std::sync::Mutex<Option<Arc<PoolActivationDurableSaveBarrierState>>>,
> = std::sync::OnceLock::new();
#[cfg(test)]
pub(crate) struct PoolActivationDurableSaveBarrier {
state: Arc<PoolActivationDurableSaveBarrierState>,
}
#[cfg(test)]
fn pool_activation_test_pool_key<S>(pool: &Arc<S>) -> usize {
Arc::as_ptr(pool).cast::<()>() as usize
}
#[cfg(test)]
impl PoolActivationDurableSaveBarrier {
pub(crate) fn install<S>(pool: &Arc<S>) -> Self {
let state = Arc::new(PoolActivationDurableSaveBarrierState {
pool_key: pool_activation_test_pool_key(pool),
arrived: tokio::sync::Notify::new(),
release: tokio::sync::Notify::new(),
});
let mut barrier = POOL_ACTIVATION_DURABLE_SAVE_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("pool activation durable save barrier should not be poisoned");
assert!(barrier.is_none(), "pool activation durable save barrier must be unique");
*barrier = Some(Arc::clone(&state));
Self { state }
}
pub(crate) async fn wait_until_paused(&self) {
tokio::time::timeout(std::time::Duration::from_secs(30), self.state.arrived.notified())
.await
.expect("activation should reach the post-durable-save barrier");
}
pub(crate) fn release_after_fence_loss(&self) {
self.state.release.notify_one();
}
}
#[cfg(test)]
impl Drop for PoolActivationDurableSaveBarrier {
fn drop(&mut self) {
self.state.release.notify_one();
let mut barrier = POOL_ACTIVATION_DURABLE_SAVE_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("pool activation durable save barrier should not be poisoned");
if barrier.as_ref().is_some_and(|state| Arc::ptr_eq(state, &self.state)) {
*barrier = None;
}
}
}
#[cfg(test)]
pub(crate) async fn pause_pool_activation_after_durable_save<S>(pool: &Arc<S>, fence: &PoolRebalanceActivationFence) {
let pool_key = pool_activation_test_pool_key(pool);
let barrier = {
let mut barrier = POOL_ACTIVATION_DURABLE_SAVE_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("pool activation durable save barrier should not be poisoned");
if barrier.as_ref().is_some_and(|state| state.pool_key == pool_key) {
barrier.take()
} else {
None
}
};
if let Some(barrier) = barrier {
barrier.arrived.notify_one();
barrier.release.notified().await;
fence.force_lost_for_test();
}
}
#[cfg(test)]
struct PoolActivationStartProbeState {
kind: PoolActivationStartKind,
attempted: std::sync::atomic::AtomicBool,
notify: tokio::sync::Notify,
}
#[cfg(test)]
static POOL_ACTIVATION_START_PROBES: std::sync::OnceLock<std::sync::Mutex<Vec<Arc<PoolActivationStartProbeState>>>> =
std::sync::OnceLock::new();
#[cfg(test)]
pub(crate) struct PoolActivationStartProbe {
state: Arc<PoolActivationStartProbeState>,
}
#[cfg(test)]
impl PoolActivationStartProbe {
pub(crate) fn install(kind: PoolActivationStartKind) -> Self {
let state = Arc::new(PoolActivationStartProbeState {
kind,
attempted: std::sync::atomic::AtomicBool::new(false),
notify: tokio::sync::Notify::new(),
});
POOL_ACTIVATION_START_PROBES
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
.lock()
.expect("pool activation start probe should not be poisoned")
.push(Arc::clone(&state));
Self { state }
}
pub(crate) async fn wait_until_attempted(&self) {
while !self.state.attempted.load(Ordering::Acquire) {
self.state.notify.notified().await;
}
}
}
#[cfg(test)]
impl Drop for PoolActivationStartProbe {
fn drop(&mut self) {
let mut probes = POOL_ACTIVATION_START_PROBES
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
.lock()
.expect("pool activation start probe should not be poisoned");
probes.retain(|state| !Arc::ptr_eq(state, &self.state));
}
}
#[cfg(test)]
pub(crate) fn observe_pool_activation_start_attempt(kind: PoolActivationStartKind) {
let probes = POOL_ACTIVATION_START_PROBES
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
.lock()
.expect("pool activation start probe should not be poisoned")
.iter()
.filter(|state| state.kind == kind)
.cloned()
.collect::<Vec<_>>();
for state in probes {
state.attempted.store(true, Ordering::Release);
state.notify.notify_one();
}
}
fn rollback_decommission_pool_meta(pool_meta: &mut PoolMeta, previous_pool_meta: PoolMeta) {
*pool_meta = previous_pool_meta;
}
@@ -2317,67 +2100,6 @@ impl PoolMeta {
Ok(())
}
async fn save_no_lock_with_activation_fence<S>(
&self,
pools: Vec<Arc<S>>,
activation_fence: PoolRebalanceActivationFence,
) -> Result<()>
where
S: EcstoreObjectIO,
{
let data = self.encode_config_data()?;
if data.is_empty() {
return Ok(());
}
let mut pools = pools.into_iter();
let Some(canonical_pool) = pools.next() else {
return Ok(());
};
let mut opts = ObjectOptions {
max_parity: true,
no_lock: true,
..Default::default()
};
activation_fence.add_namespace_lock_fence(&mut opts);
activation_fence.ensure_held()?;
#[cfg(test)]
let barrier_pool = canonical_pool.clone();
save_config_with_opts(canonical_pool, POOL_META_NAME, data.clone(), &opts).await?;
#[cfg(test)]
pause_pool_activation_after_durable_save(&barrier_pool, &activation_fence).await;
// Pool zero is canonical. Once its save succeeds, later writes only
// replicate committed state and must not reuse the admission fence.
// Failed replicas remain repairable by the next full pool metadata save.
drop(activation_fence);
for (pool_index, pool) in pools.enumerate() {
if let Err(err) = save_config_with_opts_quiet(
pool,
POOL_META_NAME,
data.clone(),
&ObjectOptions {
max_parity: true,
no_lock: true,
..Default::default()
},
)
.await
{
warn!(
event = EVENT_DECOMMISSION_STATE,
component = LOG_COMPONENT_ECSTORE,
subsystem = LOG_SUBSYSTEM_POOLS,
pool_index = pool_index + 1,
state = "activation_replica_repair_pending",
error = %err,
"Decommission activation replica repair pending"
);
}
}
Ok(())
}
pub fn decommission_cancel(&mut self, idx: usize) -> bool {
if let Some(stats) = self.pools.get_mut(idx) {
if let Some(d) = &stats.decommission {
@@ -2975,85 +2697,6 @@ fn lifecycle_action_skips_heal_version(action: IlmAction) -> bool {
action.delete()
}
#[cfg(test)]
struct LifecycleDataMovementMutationBarrierState {
bucket: String,
object: String,
arrived: tokio::sync::Notify,
release: tokio::sync::Notify,
}
#[cfg(test)]
pub(crate) struct LifecycleDataMovementMutationBarrier {
state: Arc<LifecycleDataMovementMutationBarrierState>,
}
#[cfg(test)]
static LIFECYCLE_DATA_MOVEMENT_MUTATION_BARRIER: std::sync::OnceLock<
std::sync::Mutex<Option<Arc<LifecycleDataMovementMutationBarrierState>>>,
> = std::sync::OnceLock::new();
#[cfg(test)]
impl LifecycleDataMovementMutationBarrier {
pub(crate) fn install(bucket: &str, object: &str) -> Self {
let state = Arc::new(LifecycleDataMovementMutationBarrierState {
bucket: bucket.to_string(),
object: object.to_string(),
arrived: tokio::sync::Notify::new(),
release: tokio::sync::Notify::new(),
});
let mut slot = LIFECYCLE_DATA_MOVEMENT_MUTATION_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("lifecycle data movement mutation barrier should not be poisoned");
assert!(slot.is_none(), "lifecycle data movement mutation barrier must be unique");
*slot = Some(Arc::clone(&state));
Self { state }
}
pub(crate) async fn wait_until_paused(&self) {
tokio::time::timeout(std::time::Duration::from_secs(30), self.state.arrived.notified())
.await
.expect("lifecycle data movement should reach its mutation boundary");
}
pub(crate) fn release(&self) {
self.state.release.notify_one();
}
}
#[cfg(test)]
impl Drop for LifecycleDataMovementMutationBarrier {
fn drop(&mut self) {
self.state.release.notify_one();
let mut slot = LIFECYCLE_DATA_MOVEMENT_MUTATION_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("lifecycle data movement mutation barrier should not be poisoned");
if slot.as_ref().is_some_and(|state| Arc::ptr_eq(state, &self.state)) {
*slot = None;
}
}
}
#[cfg(test)]
async fn pause_lifecycle_data_movement_mutation(bucket: &str, object: &str, has_run_fence_signal: bool) {
if !has_run_fence_signal {
return;
}
let barrier = LIFECYCLE_DATA_MOVEMENT_MUTATION_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("lifecycle data movement mutation barrier should not be poisoned")
.as_ref()
.filter(|barrier| barrier.bucket == bucket && barrier.object == object)
.cloned();
if let Some(barrier) = barrier {
barrier.arrived.notify_one();
barrier.release.notified().await;
}
}
fn resolve_data_movement_lifecycle_expiry_result(action: IlmAction, apply_actions: bool, applied: bool) -> Result<bool> {
if !apply_actions || applied {
return Ok(true);
@@ -3073,7 +2716,6 @@ pub(crate) async fn should_skip_lifecycle_for_data_movement(
object_lock_config: Option<&ObjectLockConfiguration>,
apply_actions: bool,
event_source: &LcEventSrc,
lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
) -> Result<bool> {
let Some(lifecycle_config) = lifecycle_config else {
return Ok(false);
@@ -3089,31 +2731,16 @@ pub(crate) async fn should_skip_lifecycle_for_data_movement(
let Ok(bucket_incarnation_id) = store.bucket_incarnation_id_from_disk(bucket).await else {
return Ok(false);
};
let _ = match lock_lost_signal {
Some(signal) => {
apply_expiry_rule_for_data_movement(store, &event, event_source, &object_info, Some(signal)).await
}
None => {
apply_expiry_on_transitioned_object(store, &object_info, &event, event_source, bucket_incarnation_id)
.await
}
};
let _ =
apply_expiry_on_transitioned_object(store, &object_info, &event, event_source, bucket_incarnation_id).await;
}
Ok(false)
}
action if lifecycle_action_removes_data_movement_version(action) => {
#[cfg(test)]
pause_lifecycle_data_movement_mutation(bucket, &version.name, lock_lost_signal.is_some()).await;
if lifecycle_delete_all_versions_blocked_by_replication(store.clone(), bucket, &object_info.name, action).await? {
return Ok(false);
}
let applied = !apply_actions
|| match lock_lost_signal {
Some(signal) => {
apply_expiry_rule_for_data_movement(store, &event, event_source, &object_info, Some(signal)).await
}
None => apply_expiry_rule_in(store, &event, event_source, &object_info).await,
};
let applied = !apply_actions || apply_expiry_rule_in(store, &event, event_source, &object_info).await;
resolve_data_movement_lifecycle_expiry_result(action, apply_actions, applied)
}
_ => Ok(false),
@@ -3250,9 +2877,16 @@ impl ECStore {
.first()
.cloned()
.ok_or_else(|| Error::other("decommission start rebalance metadata load failed: no storage pools available"))?;
#[cfg(test)]
observe_pool_activation_start_attempt(PoolActivationStartKind::Decommission);
let activation_fence = acquire_pool_rebalance_activation_locks(rebalance_pool.clone()).await?;
let pool_meta_lock = rebalance_pool.new_ns_lock(RUSTFS_META_BUCKET, POOL_META_NAME).await?;
let _pool_meta_guard = pool_meta_lock
.get_write_lock(get_lock_acquire_timeout())
.await
.map_err(decommission_pool_meta_lock_error)?;
let ns_lock = rebalance_pool.new_ns_lock(RUSTFS_META_BUCKET, REBAL_META_NAME).await?;
let _guard = ns_lock
.get_write_lock(get_lock_acquire_timeout())
.await
.map_err(decommission_rebalance_meta_lock_error)?;
let mut rebalance_meta = RebalanceMeta::new();
match rebalance_meta
@@ -3297,10 +2931,7 @@ impl ECStore {
latest_pool_meta.queue_buckets(idx, decom_buckets.clone());
}
activation_fence.ensure_held()?;
latest_pool_meta
.save_no_lock_with_activation_fence(self.pools.clone(), activation_fence)
.await?;
latest_pool_meta.save_no_lock(self.pools.clone()).await?;
{
let mut pool_meta = self.pool_meta.write().await;
*pool_meta = latest_pool_meta;
@@ -3314,11 +2945,6 @@ impl ECStore {
ensure_decommission_not_rebalancing(self.is_rebalance_conflicting_with_decommission().await)
}
async fn ensure_decommission_rebalance_idle_after_refresh_under_start_gate(&self) -> Result<()> {
self.load_rebalance_meta_under_start_gate().await?;
ensure_decommission_not_rebalancing(self.is_rebalance_conflicting_with_decommission().await)
}
pub async fn status(&self, idx: usize) -> Result<PoolStatus> {
let space_info = self.get_decommission_pool_space_info(idx).await?;
@@ -4213,7 +3839,6 @@ impl ECStore {
object_lock_config.as_ref(),
true,
&LcEventSrc::Decom,
None,
)
.await
})
@@ -4544,7 +4169,6 @@ impl ECStore {
lifecycle_guard: bucket_incarnation_fence
.as_ref()
.and_then(|guard| guard.namespace_lock_guard()),
namespace_lock_lost_signal: None,
object_mutation_fence: Some(&source_cleanup_mutation_fence),
},
"decommission",
@@ -5361,11 +4985,7 @@ impl ECStore {
validate_start_decommission_request(&indices, self.single_pool())?;
self.ensure_decommission_rebalance_idle_after_refresh().await?;
#[cfg(test)]
let endpoints = self.instance_endpoints().unwrap_or_else(|| self.endpoints());
#[cfg(not(test))]
let endpoints = self.endpoints();
ensure_decommission_start_local_leader(&endpoints, &indices)?;
ensure_decommission_start_local_leader(&self.endpoints(), &indices)?;
for idx in indices.iter().copied() {
ensure_valid_decommission_pool_index(self.pools.len(), idx)?;
@@ -5400,8 +5020,7 @@ impl ECStore {
}
let _start_guard = self.start_gate.lock().await;
self.ensure_decommission_rebalance_idle_after_refresh_under_start_gate()
.await?;
self.ensure_decommission_rebalance_idle_after_refresh().await?;
let all_space_infos = self.get_decommission_all_pool_space_infos().await?;
self.cancel_decommission_routines_and_wait(&indices).await;
@@ -5595,7 +5214,6 @@ impl ECStore {
object_lock_config.as_ref(),
false,
&LcEventSrc::Decom,
None,
)
.await
{
@@ -5676,128 +5294,6 @@ mod tests {
use crate::bucket::replication::{ReplicationState, ReplicationStatusType};
use serde::Serialize;
#[tokio::test]
#[serial_test::serial]
async fn decommission_activation_replicates_commit_after_post_save_fence_loss() {
let (_temp_dirs, store, _other_store) = crate::services::rebalance::test_two_pool_stores(None).await;
let barrier = PoolActivationDurableSaveBarrier::install(&store.pools[0]);
let start_store = Arc::clone(&store);
let start_task = tokio::spawn(async move {
start_store
.save_current_pool_meta_for_decommission_start(
&[0],
vec![(
0,
PoolSpaceInfo {
free: 50,
total: 100,
used: 50,
},
)],
Vec::new(),
)
.await
});
barrier.wait_until_paused().await;
barrier.release_after_fence_loss();
tokio::time::timeout(std::time::Duration::from_secs(30), start_task)
.await
.expect("decommission activation should finish after its canonical commit")
.expect("decommission activation task should not panic")
.expect("post-commit fence loss must not report the committed activation as failed");
let local = store.pool_meta.read().await;
assert!(pool_meta_has_active_decommission(&local));
drop(local);
for pool in &store.pools {
let mut persisted = PoolMeta::default();
persisted
.load_no_lock(pool.clone())
.await
.expect("every pool should retain readable committed decommission metadata");
assert!(
pool_meta_has_active_decommission(&persisted),
"every pool must adopt the canonical committed activation"
);
}
}
#[tokio::test]
#[serial_test::serial]
async fn decommission_activation_adopts_canonical_commit_after_replica_failure() {
let (_temp_dirs, store, _other_store) = crate::services::rebalance::test_two_pool_stores(None).await;
let barrier = PoolActivationDurableSaveBarrier::install(&store.pools[0]);
let start_store = Arc::clone(&store);
let start_task = tokio::spawn(async move {
start_store
.save_current_pool_meta_for_decommission_start(
&[0],
vec![(
0,
PoolSpaceInfo {
free: 50,
total: 100,
used: 50,
},
)],
Vec::new(),
)
.await
});
barrier.wait_until_paused().await;
let mut replica_disks = Vec::new();
for set in &store.pools[1].disk_set {
let mut disks = set.disks.write().await;
let saved = std::mem::take(&mut *disks);
*disks = vec![None; saved.len()];
replica_disks.push(saved);
}
barrier.release_after_fence_loss();
tokio::time::timeout(std::time::Duration::from_secs(30), start_task)
.await
.expect("decommission activation should finish after its canonical commit")
.expect("decommission activation task should not panic")
.expect("a replica save failure must not report the committed activation as failed");
for (set, disks) in store.pools[1].disk_set.iter().zip(replica_disks) {
*set.disks.write().await = disks;
}
let local = store.pool_meta.read().await;
assert!(pool_meta_has_active_decommission(&local));
drop(local);
let mut canonical = PoolMeta::default();
canonical
.load_no_lock(store.pools[0].clone())
.await
.expect("the canonical committed decommission metadata should remain readable");
assert!(pool_meta_has_active_decommission(&canonical));
let mut replica = PoolMeta::default();
replica
.load_no_lock(store.pools[1].clone())
.await
.expect("the stale replica metadata should remain readable after disks recover");
assert!(!pool_meta_has_active_decommission(&replica));
let worker_cancel = CancellationToken::new();
store
.spawn_decommission_routines(Arc::clone(&store), worker_cancel.clone(), vec![0])
.await
.expect("the committed activation should admit its decommission worker");
let admitted_cancel = store.decommission_cancelers.read().await[0]
.clone()
.expect("the admitted decommission worker should have a cancellation token");
assert!(!admitted_cancel.is_cancelled());
worker_cancel.cancel();
assert!(admitted_cancel.is_cancelled());
}
#[test]
fn ensure_pool_not_left_in_cmdline_after_decommission_allows_active_pool() {
assert!(ensure_pool_not_left_in_cmdline_after_decommission(0, "http://node{1...4}/disk{1...4}", false).is_ok());
@@ -6855,12 +6351,11 @@ mod pools_tests {
use super::{
DECOMMISSION_ENTRY_CONCURRENCY_DEFAULT_CAP, DECOMMISSION_ENTRY_CONCURRENCY_HARD_CAP, DECOMMISSION_ENTRY_QUEUE_HARD_CAP,
DECOMMISSION_PROGRESS_SAVE_INTERVAL, DECOMMISSION_PROGRESS_SAVE_ITEM_THRESHOLD, DecomBucketInfo, DecommissionCanceler,
DecommissionEntryEnqueueResult, DecommissionStartPoolState, DecommissionTerminalState, ListCallback, POOL_META_NAME,
PoolDecommissionInfo, PoolMeta, PoolSpaceInfo, PoolStatus, QueuedDecommissionEntry, REBAL_META_NAME,
acquire_pool_rebalance_activation_locks, apply_decommission_status_space_info, await_decommission_worker,
bind_decommission_cancelers, bind_missing_decommission_cancelers, cancel_decommission_canceler,
clamp_decommission_entry_concurrency, classify_decommission_terminal_state, count_decommission_item,
decommission_cancel_signal_result, decommission_entry_queue_capacity, decommission_item_size,
DecommissionEntryEnqueueResult, DecommissionStartPoolState, DecommissionTerminalState, ListCallback,
PoolDecommissionInfo, PoolMeta, PoolSpaceInfo, PoolStatus, QueuedDecommissionEntry, apply_decommission_status_space_info,
await_decommission_worker, bind_decommission_cancelers, bind_missing_decommission_cancelers,
cancel_decommission_canceler, clamp_decommission_entry_concurrency, classify_decommission_terminal_state,
count_decommission_item, decommission_cancel_signal_result, decommission_entry_queue_capacity, decommission_item_size,
decommission_meta_bucket_options, decommission_start_pool_state, dedup_indices, default_decommission_bucket_concurrency,
default_decommission_entry_concurrency, drain_decommission_entry_queue, enqueue_decommission_entry,
ensure_decommission_cancel_allowed, ensure_decommission_clear_allowed, ensure_decommission_generation,
@@ -6901,45 +6396,15 @@ mod pools_tests {
use rustfs_filemeta::{FileInfo, FileInfoVersions, MetaCacheEntry, ObjectPartInfo};
use rustfs_filemeta::{MetaCacheEntries, MetadataResolutionParams};
use rustfs_rio::Index;
use std::future::Future;
use std::sync::{
Arc, Mutex,
Arc,
atomic::{AtomicBool, AtomicUsize, Ordering},
};
use std::task::{Context, Poll};
use std::time::Duration as StdDuration;
use time::{Duration, OffsetDateTime};
use tokio::sync::Semaphore;
use tokio_util::sync::CancellationToken;
#[derive(Debug)]
struct ActivationLockRecorder {
lock_manager: Arc<rustfs_lock::GlobalLockManager>,
owner: &'static str,
resources: Mutex<Vec<String>>,
}
#[async_trait::async_trait]
impl crate::storage_api_contracts::namespace::NamespaceLocking for ActivationLockRecorder {
type Error = Error;
type NamespaceLock = rustfs_lock::NamespaceLockWrapper;
async fn new_ns_lock(&self, bucket: &str, object: &str) -> crate::error::Result<Self::NamespaceLock> {
self.resources
.lock()
.expect("activation lock recorder should not be poisoned")
.push(object.to_string());
Ok(rustfs_lock::NamespaceLockWrapper::new(
rustfs_lock::NamespaceLock::with_local_manager(
"activation-lock-test".to_string(),
Arc::clone(&self.lock_manager),
),
rustfs_lock::ObjectKey::new(bucket, object),
self.owner.to_string(),
))
}
}
fn noop_decommission_list_callback() -> ListCallback {
Arc::new(|_| Box::pin(async {}))
}
@@ -6988,48 +6453,6 @@ mod pools_tests {
}
}
#[tokio::test]
async fn test_activation_fence_uses_one_lock_order_and_serializes_callers() {
let manager = Arc::new(rustfs_lock::GlobalLockManager::new());
let first = Arc::new(ActivationLockRecorder {
lock_manager: Arc::clone(&manager),
owner: "first",
resources: Mutex::new(Vec::new()),
});
let second = Arc::new(ActivationLockRecorder {
lock_manager: manager,
owner: "second",
resources: Mutex::new(Vec::new()),
});
let first_guards = acquire_pool_rebalance_activation_locks(first.clone())
.await
.expect("first activation should acquire both locks");
assert_eq!(
*first
.resources
.lock()
.expect("activation lock recorder should not be poisoned"),
vec![POOL_META_NAME.to_string(), REBAL_META_NAME.to_string()]
);
let mut second_acquire = Box::pin(acquire_pool_rebalance_activation_locks(second.clone()));
let mut context = Context::from_waker(futures::task::noop_waker_ref());
assert!(matches!(second_acquire.as_mut().poll(&mut context), Poll::Pending));
drop(first_guards);
second_acquire
.await
.expect("second activation should acquire both locks after the first releases them");
assert_eq!(
*second
.resources
.lock()
.expect("activation lock recorder should not be poisoned"),
vec![POOL_META_NAME.to_string(), REBAL_META_NAME.to_string()]
);
}
#[test]
fn test_apply_decommission_status_space_info_adds_idle_pool_usage() {
let status = apply_decommission_status_space_info(
+4 -12
View File
@@ -1241,16 +1241,8 @@ pub(crate) async fn make_local_two_set_sets() -> (Vec<tempfile::TempDir>, Arc<Se
make_local_two_set_sets_with_ctx(bootstrap_ctx()).await
}
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
pub(crate) async fn make_local_two_set_sets_with_ctx(ctx: Arc<InstanceContext>) -> (Vec<tempfile::TempDir>, Arc<Sets>) {
make_local_two_set_sets_for_pool_with_ctx(ctx, 0).await
}
#[cfg(any(test, feature = "test-util"))]
pub(crate) async fn make_local_two_set_sets_for_pool_with_ctx(
ctx: Arc<InstanceContext>,
pool_idx: usize,
) -> (Vec<tempfile::TempDir>, Arc<Sets>) {
use crate::layout::endpoint::Endpoint;
use rustfs_lock::client::local::LocalClient;
@@ -1266,7 +1258,7 @@ pub(crate) async fn make_local_two_set_sets_for_pool_with_ctx(
let temp_dir = tempfile::tempdir().expect("tempdir should be created");
let mut endpoint = Endpoint::try_from(temp_dir.path().to_str().expect("tempdir path should be utf8"))
.expect("endpoint should parse");
endpoint.set_pool_index(pool_idx);
endpoint.set_pool_index(0);
endpoint.set_set_index(set_index);
endpoint.set_disk_index(disk_index);
let disk = new_disk(
@@ -1302,7 +1294,7 @@ pub(crate) async fn make_local_two_set_sets_for_pool_with_ctx(
2,
1,
set_index,
pool_idx,
0,
endpoints,
format.clone(),
lockers,
@@ -1315,7 +1307,7 @@ pub(crate) async fn make_local_two_set_sets_for_pool_with_ctx(
let sets = Arc::new(Sets {
id: format.id,
disk_set: disk_sets,
pool_idx,
pool_idx: 0,
endpoints: PoolEndpoints {
legacy: false,
set_count: 2,
+26 -69
View File
@@ -1023,11 +1023,10 @@ pub(crate) enum SourceCleanupError {
Storage(#[from] Error),
}
#[derive(Clone, Default)]
#[derive(Clone, Copy, Default)]
pub(crate) struct SourceCleanupBucketFence<'a> {
pub(crate) expected_incarnation_id: Option<uuid::Uuid>,
pub(crate) lifecycle_guard: Option<&'a rustfs_lock::NamespaceLockGuard>,
pub(crate) namespace_lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
pub(crate) object_mutation_fence: Option<&'a SourceCleanupMutationFence>,
}
@@ -1062,7 +1061,7 @@ pub(crate) async fn ensure_source_cleanup_versions_unchanged(
ensure_source_cleanup_versions_match(expected, &current, allowed_missing)
}
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
struct SourceCleanupDeleteBarrierState {
bucket: String,
object: String,
@@ -1072,7 +1071,7 @@ struct SourceCleanupDeleteBarrierState {
release: tokio::sync::Notify,
}
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
#[allow(
dead_code,
reason = "installed by set_disk object tests behind `--features test-util` (backlog#1823)"
@@ -1081,11 +1080,11 @@ pub(crate) struct SourceCleanupDeleteBarrier {
state: Arc<SourceCleanupDeleteBarrierState>,
}
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
static SOURCE_CLEANUP_DELETE_BARRIERS: std::sync::OnceLock<std::sync::Mutex<Vec<Arc<SourceCleanupDeleteBarrierState>>>> =
std::sync::OnceLock::new();
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
#[allow(
dead_code,
reason = "installed by set_disk object tests behind `--features test-util` (backlog#1823)"
@@ -1149,7 +1148,7 @@ pub(crate) fn notify_source_cleanup_mutation_fence_pending(bucket: &str, object:
}
}
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
impl Drop for SourceCleanupDeleteBarrier {
fn drop(&mut self) {
self.state.release.notify_one();
@@ -1161,7 +1160,7 @@ impl Drop for SourceCleanupDeleteBarrier {
}
}
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
async fn pause_source_cleanup_before_delete(bucket: &str, object: &str) {
let barrier = SOURCE_CLEANUP_DELETE_BARRIERS
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
@@ -1221,7 +1220,7 @@ pub(crate) async fn cleanup_source_entry_if_unchanged(
ensure_source_cleanup_versions_unchanged(set.clone(), bucket, object, expected, allowed_missing, op_label).await?;
#[cfg(any(test, feature = "test-util"))]
#[cfg(test)]
pause_source_cleanup_before_delete(bucket, object).await;
let mut opts = ObjectOptions {
@@ -1241,9 +1240,6 @@ pub(crate) async fn cleanup_source_entry_if_unchanged(
if let Some(bucket_lifecycle_guard) = bucket_fence.lifecycle_guard {
opts.add_bucket_lifecycle_lock_guard(bucket_lifecycle_guard);
}
if let Some(signal) = bucket_fence.namespace_lock_lost_signal {
opts.add_namespace_lock_lost_signal(signal);
}
let result = set.delete_object(bucket, cleanup_key.as_str(), opts).await;
if result.is_ok() {
crate::store::list_objects::observe_scanner_namespace_mutations(bucket, 1);
@@ -1414,13 +1410,11 @@ pub(crate) async fn migrate_decommission_object(
rd,
source_bucket_incarnation_id,
op_label,
None,
Some(&_mutation_fence),
)
.await
}
#[cfg(test)]
pub(crate) async fn migrate_object(
store: Arc<ECStore>,
pool_idx: usize,
@@ -1429,33 +1423,9 @@ pub(crate) async fn migrate_object(
source_bucket_incarnation_id: Option<uuid::Uuid>,
op_label: &str,
) -> Result<()> {
migrate_object_with_lock_lost_signal(store, pool_idx, bucket, rd, source_bucket_incarnation_id, op_label, None).await
migrate_object_inner(store, pool_idx, bucket, rd, source_bucket_incarnation_id, op_label, None).await
}
#[allow(clippy::too_many_arguments)]
pub(crate) async fn migrate_object_with_lock_lost_signal(
store: Arc<ECStore>,
pool_idx: usize,
bucket: String,
rd: GetObjectReader,
source_bucket_incarnation_id: Option<uuid::Uuid>,
op_label: &str,
lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
) -> Result<()> {
migrate_object_inner(
store,
pool_idx,
bucket,
rd,
source_bucket_incarnation_id,
op_label,
lock_lost_signal,
None,
)
.await
}
#[allow(clippy::too_many_arguments)]
async fn migrate_object_inner(
store: Arc<ECStore>,
pool_idx: usize,
@@ -1463,7 +1433,6 @@ async fn migrate_object_inner(
rd: GetObjectReader,
source_bucket_incarnation_id: Option<uuid::Uuid>,
op_label: &str,
lock_lost_signal: Option<Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
mutation_fence: Option<&ObjectLockDiagGuard>,
) -> Result<()> {
let object_info = rd.object_info.clone();
@@ -1477,9 +1446,6 @@ async fn migrate_object_inner(
if should_use_multipart_data_movement(&object_info, has_part_checksums) {
let mut new_multipart_opts = data_movement_new_multipart_opts(&object_info, pool_idx);
new_multipart_opts.expected_bucket_incarnation_id = source_bucket_incarnation_id;
if let Some(signal) = lock_lost_signal.as_ref() {
new_multipart_opts.add_namespace_lock_lost_signal(Arc::clone(signal));
}
let (res, target_pool_idx, expected_bucket_incarnation_id) = match store
.handle_new_multipart_upload_with_pool_idx(&bucket, &object_info.name, &new_multipart_opts, mutation_fence)
.await
@@ -1524,7 +1490,7 @@ async fn migrate_object_inner(
err,
)
})?;
let mut part_opts = ObjectOptions {
let part_opts = ObjectOptions {
part_number: Some(part.number),
preserve_etag: Some(part.etag.clone()),
data_movement: true,
@@ -1532,9 +1498,6 @@ async fn migrate_object_inner(
expected_bucket_incarnation_id,
..Default::default()
};
if let Some(signal) = lock_lost_signal.as_ref() {
part_opts.add_namespace_lock_lost_signal(Arc::clone(signal));
}
let pi = match store
.put_object_part_for_data_movement(
target_pool_idx,
@@ -1579,9 +1542,6 @@ async fn migrate_object_inner(
)
})?;
complete_multipart_opts.expected_bucket_incarnation_id = expected_bucket_incarnation_id;
if let Some(signal) = lock_lost_signal.as_ref() {
complete_multipart_opts.add_namespace_lock_lost_signal(Arc::clone(signal));
}
if let Err(err) = store
.clone()
.complete_multipart_upload_for_data_movement(
@@ -1630,18 +1590,18 @@ async fn migrate_object_inner(
if multipart_result.is_ok() && should_abort_multipart_upload(&abort_multipart_flag) {
let abort_result = store
.abort_multipart_upload_for_data_movement(target_pool_idx, &bucket, &object_info.name, &res.upload_id, &{
let mut opts = ObjectOptions {
.abort_multipart_upload_for_data_movement(
target_pool_idx,
&bucket,
&object_info.name,
&res.upload_id,
&ObjectOptions {
data_movement: true,
src_pool_idx: pool_idx,
expected_bucket_incarnation_id,
..Default::default()
};
if let Some(signal) = lock_lost_signal.as_ref() {
opts.add_namespace_lock_lost_signal(Arc::clone(signal));
}
opts
})
},
)
.await;
match abort_result {
Ok(()) => return Ok(()),
@@ -1699,18 +1659,18 @@ async fn migrate_object_inner(
if let Err(primary_err) = multipart_result {
if should_abort_multipart_upload(&abort_multipart_flag) {
return match store
.abort_multipart_upload_for_data_movement(target_pool_idx, &bucket, &object_info.name, &res.upload_id, &{
let mut opts = ObjectOptions {
.abort_multipart_upload_for_data_movement(
target_pool_idx,
&bucket,
&object_info.name,
&res.upload_id,
&ObjectOptions {
data_movement: true,
src_pool_idx: pool_idx,
expected_bucket_incarnation_id,
..Default::default()
};
if let Some(signal) = lock_lost_signal.as_ref() {
opts.add_namespace_lock_lost_signal(Arc::clone(signal));
}
opts
})
},
)
.await
{
Ok(()) => Err(primary_err),
@@ -1745,9 +1705,6 @@ async fn migrate_object_inner(
let mut put_opts = data_movement_put_object_opts(&object_info, pool_idx);
put_opts.expected_bucket_incarnation_id = source_bucket_incarnation_id;
if let Some(signal) = lock_lost_signal {
put_opts.add_namespace_lock_lost_signal(signal);
}
let (target_pool_idx, put_result) = store
.put_object_for_data_movement(&bucket, &object_info.name, &mut data, &put_opts, mutation_fence)
.await
+6 -38
View File
@@ -784,24 +784,6 @@ pub(crate) fn create_deferred_bitrot_reader_with_stripe_handle(
///
/// # Returns
/// A Result containing the BitrotWriterWrapper or an error
/// Size hint handed to `DiskAPI::create_file` for a bitrot-wrapped shard.
///
/// A known length is grown by one checksum per shard so the on-disk file size
/// matches what the bitrot writer emits. A negative length is the
/// unknown-size sentinel (`HashReader::SIZE_PRESERVE_LAYER`, used by SSE and
/// compression) and must be preserved: `RemoteDisk::create_file` forwards it
/// in the `put_file_stream` query, and the receiver only treats `size > 0` as
/// a fixed body length when locating the authenticated trailer. Clamping it
/// to `0` would claim an empty body and misframe the stream. `0` stays `0`
/// because a genuinely empty object still means an empty body.
fn bitrot_create_file_size(length: i64, shard_size: usize, checksum_algo: &HashAlgorithm) -> i64 {
if length <= 0 {
return length;
}
let length = length as usize;
(length.div_ceil(shard_size) * checksum_algo.size() + length) as i64
}
pub async fn create_bitrot_writer(
is_inline_buffer: bool,
disk: Option<&DiskStore>,
@@ -814,7 +796,12 @@ pub async fn create_bitrot_writer(
let writer = if is_inline_buffer {
CustomWriter::new_inline_buffer()
} else if let Some(disk) = disk {
let length = bitrot_create_file_size(length, shard_size, &checksum_algo);
let length = if length > 0 {
let length = length as usize;
(length.div_ceil(shard_size) * checksum_algo.size() + length) as i64
} else {
0
};
let file = disk.create_file("", volume, path, length).await?;
#[cfg(feature = "hotpath")]
@@ -833,25 +820,6 @@ mod tests {
use rustfs_rio::ChunkReader;
use std::collections::VecDeque;
#[test]
fn bitrot_create_file_size_grows_known_length_by_checksums() {
// 10 bytes over 4-byte shards = 3 shards, each followed by a 32-byte hash.
assert_eq!(bitrot_create_file_size(10, 4, &HashAlgorithm::HighwayHash256), 10 + 3 * 32);
assert_eq!(bitrot_create_file_size(10, 4, &HashAlgorithm::None), 10);
}
#[test]
fn bitrot_create_file_size_keeps_empty_and_unknown_distinct() {
assert_eq!(bitrot_create_file_size(0, 4, &HashAlgorithm::HighwayHash256), 0);
// SSE/compression streams advertise SIZE_PRESERVE_LAYER (-1); the remote
// put_file_stream receiver relies on a non-positive size to parse the auth
// trailer from the stream tail, so the sentinel must survive untouched.
assert_eq!(
bitrot_create_file_size(rustfs_rio::HashReader::SIZE_PRESERVE_LAYER, 4, &HashAlgorithm::HighwayHash256),
rustfs_rio::HashReader::SIZE_PRESERVE_LAYER
);
}
struct TestChunkReader {
chunks: VecDeque<Bytes>,
}
-69
View File
@@ -84,61 +84,6 @@ impl NamespaceLockFence {
}
}
#[cfg(test)]
static NAMESPACE_LOCK_SIGNAL_TEST_FENCES: std::sync::OnceLock<std::sync::Mutex<Vec<(usize, NamespaceLockFence)>>> =
std::sync::OnceLock::new();
#[cfg(test)]
pub(crate) struct NamespaceLockSignalTestFence {
signal_key: usize,
}
#[cfg(test)]
impl NamespaceLockSignalTestFence {
pub(crate) fn install_with_loss_handle(
signal: &Arc<rustfs_lock::distributed_lock::LockLostSignal>,
loss_handle: Arc<std::sync::atomic::AtomicBool>,
) -> Self {
let fence = NamespaceLockFence {
signals: Arc::default(),
forced_lost: Arc::new(vec![loss_handle]),
};
let signal_key = Arc::as_ptr(signal) as usize;
let mut fences = NAMESPACE_LOCK_SIGNAL_TEST_FENCES
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
.lock()
.expect("namespace lock signal test fence should not be poisoned");
assert!(
!fences.iter().any(|(key, _)| *key == signal_key),
"namespace lock signal test fence must be unique"
);
fences.push((signal_key, fence));
Self { signal_key }
}
}
#[cfg(test)]
impl Drop for NamespaceLockSignalTestFence {
fn drop(&mut self) {
let mut fences = NAMESPACE_LOCK_SIGNAL_TEST_FENCES
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
.lock()
.expect("namespace lock signal test fence should not be poisoned");
fences.retain(|(key, _)| *key != self.signal_key);
}
}
#[cfg(test)]
pub(crate) fn namespace_lock_signal_test_fence_is_lost(signal: &Arc<rustfs_lock::distributed_lock::LockLostSignal>) -> bool {
NAMESPACE_LOCK_SIGNAL_TEST_FENCES
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
.lock()
.expect("namespace lock signal test fence should not be poisoned")
.iter()
.find(|(key, _)| *key == Arc::as_ptr(signal) as usize)
.is_some_and(|(_, fence)| fence.is_lock_lost())
}
#[derive(Debug)]
pub struct ObjectLockConfigSnapshot {
store_id: Option<Uuid>,
@@ -460,23 +405,9 @@ impl ObjectOptions {
}
pub(crate) fn add_namespace_lock_lost_signal(&mut self, signal: Arc<rustfs_lock::distributed_lock::LockLostSignal>) {
#[cfg(test)]
let test_fence = NAMESPACE_LOCK_SIGNAL_TEST_FENCES
.get_or_init(|| std::sync::Mutex::new(Vec::new()))
.lock()
.expect("namespace lock signal test fence should not be poisoned")
.iter()
.find(|(key, _)| *key == Arc::as_ptr(&signal) as usize)
.map(|(_, fence)| fence.clone());
self.namespace_lock_fence
.get_or_insert_with(NamespaceLockFence::new)
.add_signal(signal);
#[cfg(test)]
if let Some(test_fence) = test_fence {
self.namespace_lock_fence
.get_or_insert_with(NamespaceLockFence::new)
.extend(&test_fence);
}
}
pub(crate) fn ensure_namespace_lock_fence(&mut self) {
+2 -2
View File
@@ -119,8 +119,8 @@ pub(crate) fn endpoint_erasure_set_count() -> Option<usize> {
endpoint_pools().map(|endpoints| endpoints.es_count())
}
pub(crate) fn endpoint_pool_is_local(endpoints: &EndpointServerPools, pool_index: usize) -> bool {
endpoints
pub(crate) fn endpoint_pool_is_local(pool_index: usize) -> bool {
get_global_endpoints()
.as_ref()
.get(pool_index)
.is_some_and(|pool| pool.endpoints.as_ref().first().is_some_and(|endpoint| endpoint.is_local))
@@ -1121,21 +1121,9 @@ impl NotificationSys {
}
}
let local_rebalance_id = match expected_rebalance_id {
Some(expected_id) => Some(expected_id.to_owned()),
None => store.current_rebalance_id().await,
};
match store.stop_rebalance_for_id(local_rebalance_id.as_deref()).await {
match store.stop_rebalance_for_id(expected_rebalance_id).await {
Ok(_) => {
let save_result = match local_rebalance_id.as_deref() {
Some(expected_id) => {
store
.save_rebalance_stats_for_id(usize::MAX, RebalSaveOpt::StoppedAt, expected_id)
.await
}
None => Ok(()),
};
if let Err(err) = save_result {
if let Err(err) = store.save_rebalance_stats(usize::MAX, RebalSaveOpt::StoppedAt).await {
error!(
event = EVENT_NOTIFICATION_PEER_PROPAGATION,
component = LOG_COMPONENT_ECSTORE,
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
@@ -102,20 +102,11 @@ pub(crate) trait MigrationBackend: Send + Sync {
pub(crate) struct RebalanceMigrationBackend<'a> {
source: &'a SetDisks,
store: &'a ECStore,
lock_lost_signal: Option<std::sync::Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
}
impl<'a> RebalanceMigrationBackend<'a> {
pub(crate) fn new(
source: &'a SetDisks,
store: &'a ECStore,
lock_lost_signal: Option<std::sync::Arc<rustfs_lock::distributed_lock::LockLostSignal>>,
) -> Self {
Self {
source,
store,
lock_lost_signal,
}
pub(crate) fn new(source: &'a SetDisks, store: &'a ECStore) -> Self {
Self { source, store }
}
}
@@ -139,11 +130,7 @@ impl MigrationBackend for RebalanceMigrationBackend<'_> {
fi: &FileInfo,
opts: &ObjectOptions,
) -> Result<()> {
let mut opts = opts.clone();
if let Some(signal) = self.lock_lost_signal.as_ref() {
opts.add_namespace_lock_lost_signal(std::sync::Arc::clone(signal));
}
self.store.decommission_tiered_object(bucket, object, fi, &opts).await
self.store.decommission_tiered_object(bucket, object, fi, opts).await
}
}
@@ -12,8 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#[cfg(test)]
use crate::disk::DiskAPI;
use crate::error::{Error, Result};
use crate::object_api::{GetObjectReader, ObjectInfo, ObjectOptions, PutObjReader};
use tokio::time::Duration;
@@ -47,8 +45,6 @@ mod runtime;
mod types;
mod worker;
#[cfg(feature = "test-util")]
pub use entry::test_util::PausedRebalanceEntryTestFixture;
pub(crate) use meta::is_rebalance_conflicting_with_decommission;
pub use meta::{decode_rebalance_stop_propagation_record, encode_rebalance_stop_propagation_record};
pub use types::{
@@ -57,91 +53,5 @@ pub use types::{
};
use types::{RebalanceBucketConfigs, RebalanceBucketOutcome, RebalanceEntryOutcome};
#[cfg(any(test, feature = "test-util"))]
pub async fn test_store_with_persisted_rebalance_meta(
meta: RebalanceMeta,
) -> (Vec<tempfile::TempDir>, std::sync::Arc<crate::store::ECStore>) {
let ctx = std::sync::Arc::new(crate::runtime::instance::InstanceContext::new());
let (temp_dirs, pool) = crate::core::sets::make_local_two_set_sets_with_ctx(ctx.clone()).await;
meta.save(pool.clone())
.await
.expect("rebalance test metadata should be persisted");
let endpoint_pools: crate::layout::endpoints::EndpointServerPools = vec![pool.endpoints.clone()].into();
let store = std::sync::Arc::new(crate::store::ECStore {
id: uuid::Uuid::new_v4(),
disk_map: std::collections::HashMap::new(),
pools: vec![pool],
peer_sys: crate::cluster::rpc::S3PeerSys::new_with_instance_ctx(&endpoint_pools, ctx.clone()),
pool_meta: tokio::sync::RwLock::new(crate::core::pools::PoolMeta::default()),
rebalance_meta: tokio::sync::RwLock::new(Some(meta)),
decommission_cancelers: tokio::sync::RwLock::new(vec![None]),
start_gate: tokio::sync::Mutex::new(()),
pool_meta_save_gate: tokio::sync::Mutex::new(()),
ctx,
bucket_fence_registry: std::sync::Arc::default(),
});
(temp_dirs, store)
}
#[cfg(test)]
pub(crate) async fn test_two_pool_stores(
rebalance_meta: Option<RebalanceMeta>,
) -> (
Vec<tempfile::TempDir>,
std::sync::Arc<crate::store::ECStore>,
std::sync::Arc<crate::store::ECStore>,
) {
use crate::core::pools::PoolMeta;
use crate::layout::endpoints::{EndpointServerPools, SetupType};
let ctx = std::sync::Arc::new(crate::runtime::instance::InstanceContext::new());
ctx.update_erasure_type(SetupType::DistErasure).await;
let (mut temp_dirs, first_pool) =
crate::core::sets::make_local_two_set_sets_for_pool_with_ctx(std::sync::Arc::clone(&ctx), 0).await;
let (second_temp_dirs, second_pool) =
crate::core::sets::make_local_two_set_sets_for_pool_with_ctx(std::sync::Arc::clone(&ctx), 1).await;
temp_dirs.extend(second_temp_dirs);
let pools = vec![first_pool, second_pool];
{
let local_disk_map = ctx.local_disk_map();
let mut local_disk_map = local_disk_map.write().await;
for pool in &pools {
for set in &pool.disk_set {
for disk in set.disks.read().await.iter().flatten() {
local_disk_map.insert(disk.endpoint().to_string(), Some(disk.clone()));
}
}
}
}
let pool_meta = PoolMeta::new(&pools, &PoolMeta::default());
pool_meta
.save(pools.clone())
.await
.expect("baseline pool metadata should be persisted");
if let Some(meta) = rebalance_meta.as_ref() {
meta.save(pools[0].clone())
.await
.expect("active rebalance metadata should be persisted");
}
let endpoint_pools: EndpointServerPools = pools.iter().map(|pool| pool.endpoints.clone()).collect::<Vec<_>>().into();
ctx.set_endpoints(endpoint_pools.clone());
let make_store = || {
std::sync::Arc::new(crate::store::ECStore {
id: uuid::Uuid::new_v4(),
disk_map: std::collections::HashMap::new(),
pools: pools.clone(),
peer_sys: crate::cluster::rpc::S3PeerSys::new_with_instance_ctx(&endpoint_pools, std::sync::Arc::clone(&ctx)),
pool_meta: tokio::sync::RwLock::new(pool_meta.clone()),
rebalance_meta: tokio::sync::RwLock::new(rebalance_meta.clone()),
decommission_cancelers: tokio::sync::RwLock::new(vec![None, None]),
start_gate: tokio::sync::Mutex::new(()),
pool_meta_save_gate: tokio::sync::Mutex::new(()),
ctx: std::sync::Arc::clone(&ctx),
bucket_fence_registry: std::sync::Arc::default(),
})
};
(temp_dirs, make_store(), make_store())
}
#[cfg(test)]
mod rebalance_unit_tests;
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use super::control::{fail_next_rebalance_activation_save_for_test, validate_rebalance_disk_stats_coverage};
use super::control::validate_rebalance_disk_stats_coverage;
use super::meta::{
RebalanceMetaMergeOutcome, RebalanceTerminalEvent, apply_rebalance_save_option, apply_rebalance_terminal_event,
apply_stopped_at, classify_rebalance_terminal_event, clone_arc_by_index, clone_first_arc, clone_rebalance_pool_stats,
@@ -32,11 +32,7 @@ use super::migration::{
MigrationBackend, MigrationVersionResult, migrate_entry_version, migrate_entry_version_with_retry_wait,
rebalance_delete_marker_opts,
};
use super::runtime::{
RebalanceLocalActivationOutcome, commit_local_rebalance_worker_activation,
commit_local_rebalance_worker_activation_candidate, should_fail_repeated_rebalance_bucket_defer,
source_cleanup_defer_attempt, stage_local_rebalance_worker_activation,
};
use super::runtime::{should_fail_repeated_rebalance_bucket_defer, source_cleanup_defer_attempt};
use super::worker::{
RebalanceEntryCleanupResult, ensure_rebalance_listing_disks_available, is_transient_rebalance_error,
parse_rebalance_max_attempts, rebalance_listing_retry_delay, rebalance_migration_retry_delay,
@@ -52,7 +48,6 @@ use super::worker::{
use super::{
DiskStat, GetObjectReader, ObjectInfo, ObjectOptions, RebalSaveOpt, RebalStatus, RebalanceBucketConfigs,
RebalanceBucketOutcome, RebalanceCleanupWarnings, RebalanceEntryOutcome, RebalanceInfo, RebalanceMeta, RebalanceStats,
RebalanceStopPropagationRecord,
};
use super::{REBALANCE_DEFERRED_ENTRY_ERROR_PREFIX, REBALANCE_SOURCE_CLEANUP_DEFERRED_ERROR_PREFIX};
use crate::bucket::replication::{ReplicationState, ReplicationStatusType, replication_state_to_filemeta};
@@ -2713,281 +2708,6 @@ async fn test_start_rebalance_for_id_rejects_stopped_metadata() {
assert!(err.to_string().contains("was stopped before start"));
}
#[test]
fn test_stopped_activation_state_prevents_worker_token_commit() {
let mut meta = RebalanceMeta {
id: "rebalance-a".to_string(),
stopped_at: Some(OffsetDateTime::now_utc()),
pool_stats: vec![RebalanceStats {
participating: true,
info: RebalanceInfo {
status: RebalStatus::Started,
..Default::default()
},
..Default::default()
}],
..Default::default()
};
let outcome = commit_local_rebalance_worker_activation(&mut meta, "rebalance-a", tokio_util::sync::CancellationToken::new())
.expect("stopped metadata should produce a non-start outcome");
assert_eq!(outcome, RebalanceLocalActivationOutcome::NotStartedTerminal);
assert!(meta.cancel.is_none(), "stopped rebalance must not receive a worker token");
}
#[test]
fn test_rebalance_activation_candidate_does_not_clobber_replacement_token() {
let mut local = RebalanceMeta {
id: "rebalance-a".to_string(),
pool_stats: vec![RebalanceStats {
participating: true,
buckets: vec!["bucket-a".to_string()],
info: RebalanceInfo {
status: RebalStatus::Started,
..Default::default()
},
..Default::default()
}],
..Default::default()
};
let (candidate, outcome, must_persist) =
stage_local_rebalance_worker_activation(&local, "rebalance-a", CancellationToken::new(), OffsetDateTime::UNIX_EPOCH)
.expect("active activation candidate should be staged");
assert_eq!(outcome, RebalanceLocalActivationOutcome::Started);
assert!(!must_persist);
let replacement = CancellationToken::new();
local.cancel = Some(replacement.clone());
let err = commit_local_rebalance_worker_activation_candidate(&mut local, "rebalance-a", None, candidate)
.expect_err("a replacement token must reject the stale activation candidate");
assert!(err.to_string().contains("worker token changed"));
assert_eq!(local.cancel.as_ref(), Some(&replacement));
}
#[tokio::test]
#[serial_test::serial]
async fn test_rebalance_start_save_failure_retries_persisted_completed_state() {
let active = RebalanceMeta {
id: "rebalance-real-save-completed".to_string(),
percent_free_goal: 0.5,
pool_stats: vec![RebalanceStats {
participating: true,
init_free_space: 400,
init_capacity: 1_000,
bytes: 100,
info: RebalanceInfo {
status: RebalStatus::Started,
..Default::default()
},
..Default::default()
}],
..Default::default()
};
let (_temp_dirs, store) = super::test_store_with_persisted_rebalance_meta(active).await;
fail_next_rebalance_activation_save_for_test("rebalance-real-save-completed");
let err = store
.start_rebalance_under_gate()
.await
.expect_err("the injected first activation save must fail through the real start path");
assert!(err.to_string().contains("injected rebalance activation save failure"));
{
let local = store.rebalance_meta.read().await;
let local = local.as_ref().expect("local rebalance metadata should remain present");
assert_eq!(local.pool_stats[0].info.status, RebalStatus::Started);
assert!(local.cancel.is_none(), "failed persistence must not publish a worker token");
}
let mut after_failure = RebalanceMeta::new();
after_failure
.load(store.pools[0].clone())
.await
.expect("active metadata should remain readable after the failed save");
assert_eq!(after_failure.pool_stats[0].info.status, RebalStatus::Started);
store
.start_rebalance_under_gate()
.await
.expect("the real start path must retry and persist the terminal candidate");
let mut persisted = RebalanceMeta::new();
persisted
.load(store.pools[0].clone())
.await
.expect("retry-persisted completed metadata should be readable");
assert_eq!(persisted.pool_stats[0].info.status, RebalStatus::Completed);
let local = store.rebalance_meta.read().await;
let local = local.as_ref().expect("local rebalance metadata should remain present");
assert_eq!(local.pool_stats[0].info.status, RebalStatus::Completed);
assert!(local.cancel.is_none());
}
#[tokio::test]
#[serial_test::serial]
async fn test_rebalance_start_save_failure_retries_persisted_stopped_state() {
let active = RebalanceMeta {
id: "rebalance-real-save-stopped".to_string(),
pool_stats: vec![RebalanceStats {
participating: true,
info: RebalanceInfo {
status: RebalStatus::Started,
..Default::default()
},
..Default::default()
}],
..Default::default()
};
let (_temp_dirs, store) = super::test_store_with_persisted_rebalance_meta(active).await;
let stopped_at = OffsetDateTime::from_unix_timestamp(1_000).expect("test timestamp should be valid");
{
let mut local = store.rebalance_meta.write().await;
let local = local.as_mut().expect("local rebalance metadata should remain present");
local.stopped_at = Some(stopped_at);
local.pool_stats[0].info.status = RebalStatus::Stopped;
local.pool_stats[0].info.end_time = Some(stopped_at);
}
fail_next_rebalance_activation_save_for_test("rebalance-real-save-stopped");
let err = store
.start_rebalance_under_gate()
.await
.expect_err("the injected first stopped-state save must fail through the real start path");
assert!(err.to_string().contains("injected rebalance activation save failure"));
let mut after_failure = RebalanceMeta::new();
after_failure
.load(store.pools[0].clone())
.await
.expect("active metadata should remain readable after the failed save");
assert_eq!(after_failure.pool_stats[0].info.status, RebalStatus::Started);
{
let local = store.rebalance_meta.read().await;
let local = local.as_ref().expect("local rebalance metadata should remain present");
assert_eq!(local.pool_stats[0].info.status, RebalStatus::Stopped);
assert!(local.cancel.is_none(), "failed persistence must not publish a worker token");
}
store
.start_rebalance_under_gate()
.await
.expect("the real start path must retry and persist the stopped candidate");
let mut persisted = RebalanceMeta::new();
persisted
.load(store.pools[0].clone())
.await
.expect("retry-persisted stopped metadata should be readable");
assert_eq!(persisted.stopped_at, Some(stopped_at));
assert_eq!(persisted.pool_stats[0].info.status, RebalStatus::Stopped);
let local = store.rebalance_meta.read().await;
let local = local.as_ref().expect("local rebalance metadata should remain present");
assert_eq!(local.pool_stats[0].info.status, RebalStatus::Stopped);
assert!(local.cancel.is_none());
}
#[tokio::test]
async fn test_old_worker_cannot_mutate_replacement_rebalance_state() {
let meta = RebalanceMeta {
id: "rebalance-b".to_string(),
pool_stats: vec![RebalanceStats {
participating: true,
buckets: vec!["bucket-a".to_string()],
info: RebalanceInfo {
status: RebalStatus::Started,
..Default::default()
},
..Default::default()
}],
..Default::default()
};
let store = test_store_with_rebalance_meta(meta);
let fi = FileInfo {
size: 128,
..Default::default()
};
for err in [
store
.next_rebal_bucket(0, "rebalance-a")
.await
.expect_err("old worker must not read replacement work"),
store
.bucket_rebalance_done(0, "bucket-a".to_string(), "rebalance-a")
.await
.expect_err("old worker must not complete replacement bucket"),
store
.update_pool_stats_batch_for_rebalance(0, "bucket-a".to_string(), &[&fi], "rebalance-a")
.await
.expect_err("old worker must not update replacement stats"),
store
.check_if_rebalance_done(0, "rebalance-a")
.await
.expect_err("old worker must not complete replacement pool"),
store
.save_rebalance_stats_for_id(0, RebalSaveOpt::Stats, "rebalance-a")
.await
.expect_err("old save task must not persist replacement metadata"),
store
.save_rebalance_stats_for_id(usize::MAX, RebalSaveOpt::StoppedAt, "rebalance-a")
.await
.expect_err("old stop path must not persist replacement metadata"),
] {
assert!(err.to_string().contains("stale rebalance worker rejected"));
}
let meta = store.rebalance_meta.read().await;
let meta = meta.as_ref().expect("replacement metadata should remain present");
assert_eq!(meta.id, "rebalance-b");
assert!(meta.pool_stats[0].rebalanced_buckets.is_empty());
assert_eq!(meta.pool_stats[0].bytes, 0);
assert_eq!(meta.pool_stats[0].info.status, RebalStatus::Started);
assert!(meta.stopped_at.is_none());
}
#[tokio::test]
async fn test_rebalance_metadata_reload_under_start_gate_does_not_reacquire_gate() {
let store = test_store_with_rebalance_meta(RebalanceMeta::default());
let _start_guard = store.start_gate.lock().await;
let err = store
.load_rebalance_meta_under_start_gate()
.await
.expect_err("empty test store should reach the metadata load without waiting on start_gate again");
assert!(err.to_string().contains("no pools available"));
}
#[tokio::test]
async fn test_stale_stop_propagation_cannot_mutate_replacement_rebalance() {
let meta = RebalanceMeta {
id: "rebalance-b".to_string(),
pool_stats: vec![RebalanceStats {
participating: true,
info: RebalanceInfo {
status: RebalStatus::Started,
..Default::default()
},
..Default::default()
}],
..Default::default()
};
let store = test_store_with_rebalance_meta(meta);
let record = RebalanceStopPropagationRecord {
stop_failures: vec!["old rebalance stop failed".to_string()],
..Default::default()
};
let err = store
.record_rebalance_stop_propagation("rebalance-a", record)
.await
.expect_err("old propagation failure must not mutate replacement metadata");
assert!(err.to_string().contains("stale rebalance worker rejected"));
let meta = store.rebalance_meta.read().await;
let meta = meta.as_ref().expect("replacement metadata should remain present");
assert_eq!(meta.id, "rebalance-b");
assert!(meta.last_refreshed_at.is_none());
assert!(meta.pool_stats[0].info.last_error.is_none());
}
fn test_store_with_rebalance_meta(meta: RebalanceMeta) -> Arc<crate::store::ECStore> {
let endpoint_pools: crate::layout::endpoints::EndpointServerPools = Vec::new().into();
Arc::new(crate::store::ECStore {
+36 -221
View File
@@ -1,4 +1,3 @@
use super::control::RebalanceWorkerActivationFence;
use super::meta::{
apply_rebalance_save_option, apply_rebalance_terminal_event, classify_rebalance_terminal_event, clone_first_arc,
complete_rebalance_pools_at_goal, complete_rebalance_pools_with_empty_queue, ensure_valid_rebalance_pool_index,
@@ -40,97 +39,9 @@ pub(super) fn source_cleanup_defer_attempt(deferred_attempts: &mut HashMap<Strin
*attempts
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(super) enum RebalanceLocalActivationOutcome {
Started,
NotStartedTerminal,
}
pub(super) fn commit_local_rebalance_worker_activation(
meta: &mut super::RebalanceMeta,
expected_id: &str,
cancel: CancellationToken,
) -> Result<RebalanceLocalActivationOutcome> {
if meta.id != expected_id {
return Err(Error::other(format!(
"rebalance metadata changed before local worker activation: expected {expected_id}, found {}",
meta.id
)));
}
if meta.stopped_at.is_some() || !is_rebalance_in_progress(meta) {
return Ok(RebalanceLocalActivationOutcome::NotStartedTerminal);
}
meta.cancel = Some(cancel);
Ok(RebalanceLocalActivationOutcome::Started)
}
pub(super) fn stage_local_rebalance_worker_activation(
meta: &super::RebalanceMeta,
expected_id: &str,
cancel: CancellationToken,
now: OffsetDateTime,
) -> Result<(super::RebalanceMeta, RebalanceLocalActivationOutcome, bool)> {
let mut candidate = meta.clone();
let completed_at_goal = complete_rebalance_pools_at_goal(&mut candidate, now);
let completed_empty_queue = complete_rebalance_pools_with_empty_queue(&mut candidate, now);
let outcome = commit_local_rebalance_worker_activation(&mut candidate, expected_id, cancel)?;
let must_persist =
completed_at_goal || completed_empty_queue || outcome == RebalanceLocalActivationOutcome::NotStartedTerminal;
Ok((candidate, outcome, must_persist))
}
pub(super) fn commit_local_rebalance_worker_activation_candidate(
current: &mut super::RebalanceMeta,
expected_id: &str,
expected_cancel: Option<&CancellationToken>,
candidate: super::RebalanceMeta,
) -> Result<()> {
if current.id != expected_id || candidate.id != expected_id {
return Err(Error::other(format!(
"rebalance metadata changed before local worker activation commit: expected {expected_id}, found {}",
current.id
)));
}
if !Arc::ptr_eq(&current.activation_gate, &candidate.activation_gate) {
return Err(Error::other(format!(
"rebalance activation gate changed before local worker activation commit: {expected_id}"
)));
}
if current.cancel.as_ref() != expected_cancel {
return Err(Error::other(format!(
"rebalance worker token changed before local worker activation commit: {expected_id}"
)));
}
*current = candidate;
Ok(())
}
pub(super) fn rollback_local_rebalance_worker_activation(
meta: Option<&mut super::RebalanceMeta>,
expected_id: &str,
activation_token: &CancellationToken,
) -> bool {
let Some(meta) = meta else {
return false;
};
if meta.id != expected_id || meta.cancel.as_ref() != Some(activation_token) {
return false;
}
if let Some(cancel) = meta.cancel.take() {
cancel.cancel();
return true;
}
false
}
impl ECStore {
#[tracing::instrument(skip_all)]
pub async fn start_rebalance(self: &Arc<Self>) -> Result<()> {
let _start_guard = self.start_gate.lock().await;
self.start_rebalance_under_gate().await
}
pub(super) async fn start_rebalance_under_gate(self: &Arc<Self>) -> Result<()> {
info!(
event = EVENT_REBALANCE_STATE,
component = LOG_COMPONENT_ECSTORE,
@@ -138,27 +49,12 @@ impl ECStore {
state = "starting",
"Starting rebalance"
);
let expected_id: Arc<str> = {
let rebalance_meta = self.rebalance_meta.read().await;
Arc::from(rebalance_meta.as_ref().ok_or(Error::ConfigNotFound)?.id.as_str())
};
let pool = clone_first_arc(self.pools.as_slice(), "start_rebalance: no pools available")?;
let activation_fence = match self
.fence_rebalance_worker_activation(pool.clone(), expected_id.as_ref())
.await?
{
RebalanceWorkerActivationFence::Ready(fence) => fence,
RebalanceWorkerActivationFence::NotStartedTerminal => return Ok(()),
};
let decommission_running = self.is_decommission_running().await;
// let rebalance_meta = self.rebalance_meta.read().await;
let cancel_tx = CancellationToken::new();
let rx = cancel_tx.clone();
let activation_outcome;
let candidate;
let expected_cancel;
let must_persist;
let mut meta_to_save = None;
{
let mut rebalance_meta = self.rebalance_meta.write().await;
@@ -178,70 +74,25 @@ impl ECStore {
);
return Ok(());
}
expected_cancel = meta.cancel.clone();
(candidate, activation_outcome, must_persist) = stage_local_rebalance_worker_activation(
meta,
expected_id.as_ref(),
cancel_tx.clone(),
OffsetDateTime::now_utc(),
let now = OffsetDateTime::now_utc();
if complete_rebalance_pools_at_goal(meta, now) {
meta_to_save = Some(meta.clone());
}
if complete_rebalance_pools_with_empty_queue(meta, now) {
meta_to_save = Some(meta.clone());
}
meta.cancel = Some(cancel_tx);
drop(rebalance_meta);
}
if let Some(meta) = meta_to_save {
let pool = clone_first_arc(self.pools.as_slice(), "start_rebalance: no pools available")?;
resolve_rebalance_meta_save_result(
self.save_rebalance_meta_with_merge(pool, &meta, "start_rebalance complete pools at goal")
.await,
"start_rebalance complete pools at goal",
)?;
if let Err(err) = activation_fence.ensure_held() {
cancel_tx.cancel();
return Err(err);
}
if !must_persist
&& let Err(err) = commit_local_rebalance_worker_activation_candidate(
meta,
expected_id.as_ref(),
expected_cancel.as_ref(),
candidate.clone(),
)
{
cancel_tx.cancel();
return Err(err);
}
}
if must_persist {
let save_result = resolve_rebalance_meta_save_result(
self.save_rebalance_meta_under_activation_fence(
pool,
&candidate,
"start_rebalance persist activation candidate",
activation_fence.as_ref(),
expected_id.as_ref(),
)
.await,
"start_rebalance persist activation candidate",
);
if let Err(err) = save_result {
cancel_tx.cancel();
return Err(err);
}
let mut rebalance_meta = self.rebalance_meta.write().await;
let Some(meta) = rebalance_meta.as_mut() else {
cancel_tx.cancel();
return Err(Error::ConfigNotFound);
};
if let Err(err) = commit_local_rebalance_worker_activation_candidate(
meta,
expected_id.as_ref(),
expected_cancel.as_ref(),
candidate,
) {
cancel_tx.cancel();
return Err(err);
}
}
if !must_persist && let Err(err) = activation_fence.ensure_held() {
let mut rebalance_meta = self.rebalance_meta.write().await;
rollback_local_rebalance_worker_activation(rebalance_meta.as_mut(), expected_id.as_ref(), &rx);
return Err(err);
}
drop(activation_fence);
if activation_outcome != RebalanceLocalActivationOutcome::Started {
return Ok(());
}
let participants = if let Some(ref meta) = *self.rebalance_meta.read().await {
@@ -259,8 +110,6 @@ impl ECStore {
};
if !participants.iter().any(|participating| *participating) {
let mut rebalance_meta = self.rebalance_meta.write().await;
rollback_local_rebalance_worker_activation(rebalance_meta.as_mut(), expected_id.as_ref(), &rx);
debug!(
event = EVENT_REBALANCE_STATE,
component = LOG_COMPONENT_ECSTORE,
@@ -272,11 +121,6 @@ impl ECStore {
return Ok(());
}
#[cfg(test)]
let endpoints = self.instance_endpoints().unwrap_or_else(|| self.endpoints());
#[cfg(not(test))]
let endpoints = self.endpoints();
let mut workers_started = 0usize;
for (idx, participating) in participants.iter().enumerate() {
if !*participating {
@@ -292,7 +136,7 @@ impl ECStore {
continue;
}
if !runtime_sources::endpoint_pool_is_local(&endpoints, idx) {
if !runtime_sources::endpoint_pool_is_local(idx) {
debug!(
event = EVENT_REBALANCE_STATE,
component = LOG_COMPONENT_ECSTORE,
@@ -308,10 +152,9 @@ impl ECStore {
let pool_idx = idx;
let store = self.clone();
let rx_clone = rx.clone();
let worker_id = Arc::clone(&expected_id);
workers_started += 1;
tokio::spawn(async move {
if let Err(err) = store.rebalance_buckets(rx_clone, pool_idx, worker_id).await {
if let Err(err) = store.rebalance_buckets(rx_clone, pool_idx).await {
error!(
event = EVENT_REBALANCE_STATE,
component = LOG_COMPONENT_ECSTORE,
@@ -335,8 +178,6 @@ impl ECStore {
}
if workers_started == 0 {
let mut rebalance_meta = self.rebalance_meta.write().await;
rollback_local_rebalance_worker_activation(rebalance_meta.as_mut(), expected_id.as_ref(), &rx);
debug!(
event = EVENT_REBALANCE_STATE,
component = LOG_COMPONENT_ECSTORE,
@@ -360,14 +201,13 @@ impl ECStore {
}
#[tracing::instrument(skip(self, rx))]
async fn rebalance_buckets(self: &Arc<Self>, rx: CancellationToken, pool_index: usize, rebalance_id: Arc<str>) -> Result<()> {
async fn rebalance_buckets(self: &Arc<Self>, rx: CancellationToken, pool_index: usize) -> Result<()> {
ensure_valid_rebalance_pool_index(self.pools.len(), pool_index)?;
let (done_tx, mut done_rx) = tokio::sync::mpsc::channel::<Result<()>>(1);
// Save rebalance metadata periodically
let store = self.clone();
let save_rebalance_id = Arc::clone(&rebalance_id);
let save_task = tokio::spawn(async move {
let mut timer = tokio::time::interval_at(Instant::now() + Duration::from_secs(30), Duration::from_secs(10));
let mut msg: String;
@@ -381,11 +221,6 @@ impl ECStore {
let terminal_event = classify_rebalance_terminal_event(result, now);
msg = terminal_event.message().to_string();
let mut rebalance_meta = store.rebalance_meta.write().await;
super::control::ensure_rebalance_run_id(
rebalance_meta.as_ref(),
save_rebalance_id.as_ref(),
"apply rebalance terminal event",
)?;
if let Some(meta) = rebalance_meta.as_mut() {
let meta_stopped = meta.stopped_at.is_some();
if let Some(pool_stat) = meta.pool_stats.get_mut(pool_index) {
@@ -434,10 +269,7 @@ impl ECStore {
}
}
if let Err(err) = store
.save_rebalance_stats_for_id(pool_index, RebalSaveOpt::Stats, save_rebalance_id.as_ref())
.await
{
if let Err(err) = store.save_rebalance_stats(pool_index, RebalSaveOpt::Stats).await {
let wrapped = Error::other(format!("rebalance save_task stats save failed for pool {pool_index}: {err}"));
error!("{} err: {:?}", msg, wrapped);
if quit {
@@ -503,7 +335,7 @@ impl ECStore {
break;
}
let next_bucket = match self.next_rebal_bucket(pool_index, rebalance_id.as_ref()).await {
let next_bucket = match self.next_rebal_bucket(pool_index).await {
Ok(bucket) => bucket,
Err(err) => {
error!(
@@ -535,8 +367,7 @@ impl ECStore {
);
let outcome = match resolve_rebalance_bucket_result(
self.rebalance_bucket(rx.clone(), bucket.clone(), pool_index, Arc::clone(&rebalance_id))
.await,
self.rebalance_bucket(rx.clone(), bucket.clone(), pool_index).await,
pool_index,
&bucket,
) {
@@ -599,7 +430,7 @@ impl ECStore {
"Deferred rebalance bucket after transient object failures"
);
if let Err(err) = self
.defer_rebalance_bucket(pool_index, bucket.clone(), last_error.clone(), rebalance_id.as_ref())
.defer_rebalance_bucket(pool_index, bucket.clone(), last_error.clone())
.await
{
error!(
@@ -663,7 +494,7 @@ impl ECStore {
"Completed rebalance bucket"
);
source_cleanup_deferred_attempts.remove(&bucket);
if let Err(err) = self.bucket_rebalance_done(pool_index, bucket, rebalance_id.as_ref()).await {
if let Err(err) = self.bucket_rebalance_done(pool_index, bucket).await {
error!(
event = EVENT_REBALANCE_BUCKET,
component = LOG_COMPONENT_ECSTORE,
@@ -724,9 +555,8 @@ impl ECStore {
final_result
}
pub(super) async fn check_if_rebalance_done(&self, pool_index: usize, expected_id: &str) -> Result<bool> {
pub(super) async fn check_if_rebalance_done(&self, pool_index: usize) -> bool {
let mut rebalance_meta = self.rebalance_meta.write().await;
super::control::ensure_rebalance_worker_active(rebalance_meta.as_ref(), expected_id, "check rebalance completion")?;
if let Some(meta) = rebalance_meta.as_mut()
&& let Some(pool_stat) = meta.pool_stats.get_mut(pool_index)
@@ -741,7 +571,7 @@ impl ECStore {
state = "already_completed",
"Rebalance pool is already completed"
);
return Ok(true);
return true;
}
// Mark pool rebalance as done only after it reaches the PercentFreeGoal.
@@ -771,30 +601,19 @@ impl ECStore {
percent_free = pfi,
"Marked rebalance pool completed"
);
return Ok(true);
return true;
}
}
Ok(false)
false
}
}
impl ECStore {
#[tracing::instrument(skip(self))]
pub async fn save_rebalance_stats(&self, pool_idx: usize, opt: RebalSaveOpt) -> Result<()> {
self.save_rebalance_stats_inner(pool_idx, opt, None).await
}
pub async fn save_rebalance_stats_for_id(&self, pool_idx: usize, opt: RebalSaveOpt, expected_id: &str) -> Result<()> {
self.save_rebalance_stats_inner(pool_idx, opt, Some(expected_id)).await
}
async fn save_rebalance_stats_inner(&self, pool_idx: usize, opt: RebalSaveOpt, expected_id: Option<&str>) -> Result<()> {
let meta_to_save = {
let mut rebalance_meta = self.rebalance_meta.write().await;
if let Some(expected_id) = expected_id {
super::control::ensure_rebalance_run_id(rebalance_meta.as_ref(), expected_id, "save rebalance stats")?;
}
let Some(meta) = rebalance_meta.as_mut() else {
return Ok(());
};
@@ -816,14 +635,10 @@ impl ECStore {
"Rebalance metadata save requested"
);
let stage = format!("save_rebalance_stats for pool {pool_idx} opt {opt:?}");
let save_result = match expected_id {
Some(expected_id) => {
self.save_rebalance_meta_for_id_with_merge(pool, &meta_to_save, stage.as_str(), expected_id)
.await
}
None => self.save_rebalance_meta_with_merge(pool, &meta_to_save, stage.as_str()).await,
};
resolve_rebalance_meta_save_result(save_result, stage.as_str())?;
resolve_rebalance_meta_save_result(
self.save_rebalance_meta_with_merge(pool, &meta_to_save, stage.as_str()).await,
stage.as_str(),
)?;
Ok(())
}
@@ -144,8 +144,6 @@ pub struct RebalanceMeta {
#[serde(skip)]
pub cancel: Option<CancellationToken>, // To be invoked on rebalance-stop
#[serde(skip)]
pub activation_gate: std::sync::Arc<tokio::sync::RwLock<()>>,
#[serde(skip)]
pub last_refreshed_at: Option<OffsetDateTime>,
#[serde(rename = "stopTs")]
pub stopped_at: Option<OffsetDateTime>, // Time when rebalance-stop was issued
@@ -100,17 +100,17 @@ pub(super) fn resolve_rebalance_meta_save_result(result: Result<()>, stage: &str
result.map_err(|err| Error::other(format!("rebalance meta save failed during {stage}: {err}")))
}
pub(super) fn rebalance_meta_lock_error(err: rustfs_lock::LockError, mode: &'static str) -> Error {
pub(super) fn rebalance_meta_lock_error(err: rustfs_lock::LockError) -> Error {
match err {
rustfs_lock::LockError::QuorumNotReached { required, achieved } => Error::NamespaceLockQuorumUnavailable {
mode,
mode: "write",
bucket: crate::disk::RUSTFS_META_BUCKET.to_string(),
object: REBAL_META_NAME.to_string(),
required,
achieved,
},
other => Error::other(format!(
"failed to acquire rebalance metadata {mode} lock on {}/{}: {other}",
"failed to acquire rebalance metadata write lock on {}/{}: {other}",
crate::disk::RUSTFS_META_BUCKET,
REBAL_META_NAME
)),
-80
View File
@@ -735,9 +735,6 @@ pub(crate) use core::io_primitives::disk_call_counters;
mod ctx;
mod metadata;
mod ops;
#[cfg(test)]
pub(crate) use ops::hermetic_set_disks_isolated;
#[cfg(test)]
pub(crate) use ops::multipart::NewMultipartUploadCommitObservation;
#[cfg(any(test, feature = "test-util"))]
@@ -1452,21 +1449,6 @@ mod prepared_get_object_metadata_tests {
}
impl SetDisks {
#[cfg(test)]
async fn pause_tiered_metadata_commit(bucket: &str, object: &str) {
let barrier = TIERED_METADATA_COMMIT_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("tiered metadata commit barrier should not be poisoned")
.as_ref()
.filter(|barrier| barrier.bucket == bucket && barrier.object == object)
.cloned();
if let Some(barrier) = barrier {
barrier.arrived.notify_one();
barrier.release.notified().await;
}
}
pub(crate) async fn prepare_get_object_metadata(
&self,
bucket: &str,
@@ -4735,8 +4717,6 @@ impl SetDisks {
)?;
let fi = build_tiered_decommission_file_info(bucket, object, fi, layout);
let write_quorum = layout.write_quorum;
#[cfg(test)]
Self::pause_tiered_metadata_commit(bucket, object).await;
if _lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost())
|| opts
.namespace_lock_fence
@@ -4784,66 +4764,6 @@ impl SetDisks {
}
}
#[cfg(test)]
struct TieredMetadataCommitBarrierState {
bucket: String,
object: String,
arrived: tokio::sync::Notify,
release: tokio::sync::Notify,
}
#[cfg(test)]
pub(crate) struct TieredMetadataCommitBarrier {
state: Arc<TieredMetadataCommitBarrierState>,
}
#[cfg(test)]
static TIERED_METADATA_COMMIT_BARRIER: std::sync::OnceLock<std::sync::Mutex<Option<Arc<TieredMetadataCommitBarrierState>>>> =
std::sync::OnceLock::new();
#[cfg(test)]
impl TieredMetadataCommitBarrier {
pub(crate) fn install(bucket: &str, object: &str) -> Self {
let state = Arc::new(TieredMetadataCommitBarrierState {
bucket: bucket.to_string(),
object: object.to_string(),
arrived: tokio::sync::Notify::new(),
release: tokio::sync::Notify::new(),
});
let mut slot = TIERED_METADATA_COMMIT_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("tiered metadata commit barrier should not be poisoned");
assert!(slot.is_none(), "tiered metadata commit barrier must be unique");
*slot = Some(Arc::clone(&state));
Self { state }
}
pub(crate) async fn wait_until_paused(&self) {
tokio::time::timeout(Duration::from_secs(30), self.state.arrived.notified())
.await
.expect("tiered metadata write should reach its deterministic commit barrier");
}
pub(crate) fn release(&self) {
self.state.release.notify_one();
}
}
#[cfg(test)]
impl Drop for TieredMetadataCommitBarrier {
fn drop(&mut self) {
self.state.release.notify_one();
let mut slot = TIERED_METADATA_COMMIT_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("tiered metadata commit barrier should not be poisoned");
if slot.as_ref().is_some_and(|state| Arc::ptr_eq(state, &self.state)) {
*slot = None;
}
}
}
#[derive(Debug, PartialEq, Eq)]
struct ObjProps {
successor_mod_time: Option<OffsetDateTime>,
-3
View File
@@ -25,6 +25,3 @@ pub(crate) mod list;
pub(crate) mod locking;
pub(crate) mod multipart;
pub(crate) mod object;
#[cfg(test)]
pub(crate) use object::hermetic_set_disks_support::hermetic_set_disks_isolated;
+1 -42
View File
@@ -86,8 +86,6 @@ struct MultipartCommitBarrierState {
pause: MultipartCommitPause,
expected_arrivals: usize,
arrivals: AtomicUsize,
#[cfg(test)]
committed: AtomicBool,
arrived: tokio::sync::Notify,
release: tokio::sync::Semaphore,
}
@@ -115,8 +113,6 @@ impl MultipartCommitBarrier {
pause,
expected_arrivals,
arrivals: AtomicUsize::new(0),
#[cfg(test)]
committed: AtomicBool::new(false),
arrived: tokio::sync::Notify::new(),
release: tokio::sync::Semaphore::new(0),
});
@@ -147,11 +143,6 @@ impl MultipartCommitBarrier {
pub fn release(&self) {
self.state.release.add_permits(self.state.expected_arrivals);
}
#[cfg(test)]
pub(crate) fn commit_observed(&self) -> bool {
self.state.committed.load(Ordering::Acquire)
}
}
#[cfg(any(test, feature = "test-util"))]
@@ -272,20 +263,6 @@ async fn pause_multipart_commit(bucket: &str, object: &str, pause: MultipartComm
}
}
#[cfg(test)]
fn observe_multipart_commit(bucket: &str, object: &str, pause: MultipartCommitPause) {
let slot = MULTIPART_COMMIT_BARRIER
.get_or_init(|| std::sync::Mutex::new(None))
.lock()
.expect("multipart commit barrier mutex should not poison");
if let Some(barrier) = slot
.as_ref()
.filter(|barrier| barrier.bucket == bucket && barrier.object == object && barrier.pause == pause)
{
barrier.committed.store(true, Ordering::Release);
}
}
fn map_upload_id_metadata_error(bucket: &str, object: &str, upload_id: &str, err: DiskError) -> Error {
if err == DiskError::FileNotFound {
return StorageError::InvalidUploadID(bucket.to_owned(), object.to_owned(), upload_id.to_owned());
@@ -1343,19 +1320,6 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
pause_multipart_commit(bucket, object, MultipartCommitPause::PutPartBeforeLockLost).await;
fence_commit_on_lock_loss(_upload_commit_guard.as_ref(), "put_object_part_commit", &upload_id_path)?;
fence_commit_on_lock_loss(_part_commit_guard.as_ref(), "put_object_part_commit", &part_lock_path)?;
if opts
.namespace_lock_fence
.as_ref()
.is_some_and(NamespaceLockFence::is_lock_lost)
{
return Err(StorageError::NamespaceLockQuorumUnavailable {
mode: "put_object_part_outer_lock",
bucket: bucket.to_string(),
object: object.to_string(),
required: 1,
achieved: 0,
});
}
ensure_multipart_bucket_lifecycle_lock_held(bucket, object, opts)?;
let _ = self
@@ -1376,8 +1340,6 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
}),
)
.await?;
#[cfg(test)]
observe_multipart_commit(bucket, object, MultipartCommitPause::PutPartBeforeLockLost);
#[cfg(test)]
pause_multipart_commit(bucket, object, MultipartCommitPause::PutPartAfterRename).await;
@@ -1758,10 +1720,7 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
.await
.map_err(|e| to_object_err(e.into(), vec![bucket, object]))?;
#[cfg(test)]
{
observe_multipart_commit(bucket, object, MultipartCommitPause::NewUploadBeforeLockLost);
observe_new_multipart_upload_commit(bucket, object);
}
observe_new_multipart_upload_commit(bucket, object);
// evalDisks
+5 -4
View File
@@ -6519,8 +6519,6 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
};
let find_vid = Uuid::new_v4();
#[cfg(test)]
pause_delete_object_commit(bucket, object).await;
if mark_delete && (opts.versioned || opts.version_suspended) {
if !delete_marker {
@@ -6589,6 +6587,8 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
dfi.set_skip_tier_free_version();
}
#[cfg(test)]
pause_delete_object_commit(bucket, object).await;
ensure_delete_commit_locks_held(_lock_guard.as_ref(), bucket, object, &opts)?;
self.delete_object_version(bucket, object, &dfi, opts.delete_marker)
.await
@@ -7750,7 +7750,9 @@ pub(in crate::set_disk::ops) mod hermetic_set_disks_support {
/// for tests that never touch context-resolved services registered on the
/// ambient context (tier config manager, expiry state, ...), because the
/// isolated context starts every one of those cells fresh.
pub(crate) async fn hermetic_set_disks_isolated(disk_count: usize) -> (Vec<TempDir>, Vec<DiskStore>, Arc<SetDisks>) {
pub(in crate::set_disk::ops) async fn hermetic_set_disks_isolated(
disk_count: usize,
) -> (Vec<TempDir>, Vec<DiskStore>, Arc<SetDisks>) {
hermetic_set_disks_for_pool_with_default_parity_isolated(disk_count, 0, disk_count / 2).await
}
@@ -11813,7 +11815,6 @@ mod transition_upload_integrity_tests {
crate::data_movement::SourceCleanupBucketFence {
expected_incarnation_id: None,
lifecycle_guard: Some(&bucket_guard),
namespace_lock_lost_signal: None,
..Default::default()
},
"test_data_movement",
+25 -86
View File
@@ -16,14 +16,14 @@
//!
//! `scripts/test/vault_ha_kms_live.sh` owns the official Vault containers and
//! kills the active node while this test continuously decrypts through a
//! surviving standby. KV2 and Transit must recover after the bounded circuit
//! interval, use a bounded number of attempts, and leave the circuit and
//! in-flight gauges at zero after a new leader is elected.
//! surviving standby. KV2 and Transit requests must remain successful, use a
//! bounded number of attempts, and leave the circuit and in-flight gauges at
//! zero after a new leader is elected.
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
use std::sync::{Arc, Mutex};
use std::time::Duration;
use metrics_util::MetricKind;
@@ -43,11 +43,6 @@ const OPERATION_ATTEMPTS: &str = "rustfs_kms_backend_operation_attempts";
const IN_FLIGHT: &str = "rustfs_kms_backend_in_flight";
const CIRCUIT_OPEN: &str = "rustfs_kms_backend_circuit_open";
const MAX_ATTEMPTS: u32 = 10;
const ATTEMPT_TIMEOUT: Duration = Duration::from_secs(2);
const HEALTHY_PROGRESS_TIMEOUT: Duration = Duration::from_secs(20);
// The circuit remains open for 30s after five failed attempts.
const POST_FAILOVER_PROGRESS_TIMEOUT: Duration = Duration::from_secs(35);
const FAILOVER_ERROR_POLL_INTERVAL: Duration = Duration::from_millis(100);
type MetricEntry = (
metrics_util::CompositeKey,
@@ -69,7 +64,7 @@ fn config(backend: KmsBackend, backend_config: BackendConfig) -> KmsConfig {
backend,
backend_config,
allow_insecure_dev_defaults: true,
timeout: ATTEMPT_TIMEOUT,
timeout: Duration::from_secs(2),
retry_attempts: MAX_ATTEMPTS,
enable_cache: false,
..KmsConfig::default()
@@ -169,31 +164,14 @@ fn retryable_failures(snapshot: &[MetricEntry], operation: &str) -> u64 {
.sum()
}
async fn wait_for_count(
counter: &AtomicU64,
failure: &Mutex<Option<String>>,
minimum: u64,
description: &str,
timeout: Duration,
) {
tokio::time::timeout(timeout, async {
async fn wait_for_count(counter: &AtomicU64, minimum: u64, description: &str) {
tokio::time::timeout(Duration::from_secs(20), async {
while counter.load(Ordering::SeqCst) < minimum {
if let Some(error) = failure.lock().expect("decrypt failure lock poisoned").as_ref() {
panic!(
"{description} worker failed after {} successful decrypts: {error}",
counter.load(Ordering::SeqCst)
);
}
tokio::time::sleep(Duration::from_millis(25)).await;
}
})
.await
.unwrap_or_else(|_| {
panic!(
"timed out after {timeout:?} waiting for {description}: completed {}, expected {minimum}",
counter.load(Ordering::SeqCst)
)
});
.unwrap_or_else(|_| panic!("timed out waiting for {description}"));
}
async fn wait_for_file(path: &Path, description: &str) {
@@ -211,8 +189,7 @@ async fn decrypt_loop<B: KmsBackendTrait + Send + Sync + 'static>(
request: DecryptRequest,
expected: Vec<u8>,
completed: Arc<AtomicU64>,
allow_failover_errors: Arc<AtomicBool>,
failure: Arc<Mutex<Option<String>>>,
failed: Arc<AtomicBool>,
stop: CancellationToken,
) {
while !stop.is_cancelled() {
@@ -220,18 +197,8 @@ async fn decrypt_loop<B: KmsBackendTrait + Send + Sync + 'static>(
Ok(response) if response.plaintext == expected => {
completed.fetch_add(1, Ordering::SeqCst);
}
Ok(_) => {
*failure.lock().expect("decrypt failure lock poisoned") =
Some("decrypt returned unexpected plaintext".to_string());
return;
}
Err(rustfs_kms::KmsError::BackendError { .. } | rustfs_kms::KmsError::OperationTimedOut { .. })
if allow_failover_errors.load(Ordering::SeqCst) =>
{
tokio::time::sleep(FAILOVER_ERROR_POLL_INTERVAL).await;
}
Err(error) => {
*failure.lock().expect("decrypt failure lock poisoned") = Some(error.to_string());
Ok(_) | Err(_) => {
failed.store(true, Ordering::SeqCst);
return;
}
}
@@ -329,9 +296,7 @@ async fn exercise_failover(snapshotter: &Snapshotter) {
);
let stop = CancellationToken::new();
let allow_failover_errors = Arc::new(AtomicBool::new(false));
let kv2_failure = Arc::new(Mutex::new(None));
let transit_failure = Arc::new(Mutex::new(None));
let failed = Arc::new(AtomicBool::new(false));
let kv2_completed = Arc::new(AtomicU64::new(0));
let transit_completed = Arc::new(AtomicU64::new(0));
let kv2_worker = tokio::spawn(decrypt_loop(
@@ -339,8 +304,7 @@ async fn exercise_failover(snapshotter: &Snapshotter) {
kv2_request,
kv2_data_key.plaintext_key,
Arc::clone(&kv2_completed),
Arc::clone(&allow_failover_errors),
Arc::clone(&kv2_failure),
Arc::clone(&failed),
stop.clone(),
));
let transit_worker = tokio::spawn(decrypt_loop(
@@ -348,21 +312,12 @@ async fn exercise_failover(snapshotter: &Snapshotter) {
transit_request,
transit_data_key.plaintext_key,
Arc::clone(&transit_completed),
Arc::clone(&allow_failover_errors),
Arc::clone(&transit_failure),
Arc::clone(&failed),
stop.clone(),
));
wait_for_count(&kv2_completed, &kv2_failure, 2, "two healthy KV2 decrypts", HEALTHY_PROGRESS_TIMEOUT).await;
wait_for_count(
&transit_completed,
&transit_failure,
2,
"two healthy Transit decrypts",
HEALTHY_PROGRESS_TIMEOUT,
)
.await;
allow_failover_errors.store(true, Ordering::SeqCst);
wait_for_count(&kv2_completed, 2, "two healthy KV2 decrypts").await;
wait_for_count(&transit_completed, 2, "two healthy Transit decrypts").await;
std::fs::write(&marker, b"ready").expect("publish failover readiness marker");
wait_for_file(&elected, "the replacement Vault leader").await;
@@ -371,39 +326,18 @@ async fn exercise_failover(snapshotter: &Snapshotter) {
let kv2_after_election = kv2_completed.load(Ordering::SeqCst) + 2;
let transit_after_election = transit_completed.load(Ordering::SeqCst) + 2;
wait_for_count(
&kv2_completed,
&kv2_failure,
kv2_after_election,
"post-failover KV2 decrypts",
POST_FAILOVER_PROGRESS_TIMEOUT,
)
.await;
wait_for_count(
&transit_completed,
&transit_failure,
transit_after_election,
"post-failover Transit decrypts",
POST_FAILOVER_PROGRESS_TIMEOUT,
)
.await;
wait_for_count(&kv2_completed, kv2_after_election, "post-failover KV2 decrypts").await;
wait_for_count(&transit_completed, transit_after_election, "post-failover Transit decrypts").await;
stop.cancel();
kv2_worker.await.expect("KV2 decrypt worker must join");
transit_worker.await.expect("Transit decrypt worker must join");
assert!(
kv2_failure.lock().expect("KV2 failure lock poisoned").is_none(),
"no KV2 decrypt may fail or return different plaintext"
);
assert!(
transit_failure.lock().expect("Transit failure lock poisoned").is_none(),
"no Transit decrypt may fail or return different plaintext"
);
assert!(!failed.load(Ordering::SeqCst), "no decrypt may fail or return different plaintext");
}
#[test]
#[ignore = "requires a real three-node Vault Raft cluster; run scripts/test/vault_ha_kms_live.sh"]
fn vault_raft_leader_failure_recovers_kv2_and_transit_decrypts() {
fn vault_raft_leader_failure_preserves_kv2_and_transit_decrypts() {
let recorder = DebuggingRecorder::new();
let snapshotter = recorder.snapshotter();
metrics::with_local_recorder(&recorder, || {
@@ -415,6 +349,11 @@ fn vault_raft_leader_failure_recovers_kv2_and_transit_decrypts() {
});
let snapshot = snapshotter.snapshot().into_vec();
assert_eq!(
counter_value(&snapshot, OPERATIONS_TOTAL, &[("outcome", "circuit_open")]),
0,
"a bounded leader election must not open the circuit"
);
assert_eq!(
counter_value(&snapshot, OPERATIONS_TOTAL, &[("outcome", "budget_exhausted")]),
0,
+253
View File
@@ -270,6 +270,160 @@ pub fn active_replication_rule_destination_arns(config: &ReplicationConfiguratio
arns
}
/// Deployment id extracted from a site-replication target ARN
/// (`arn:{rustfs|minio}:replication::<deployment-id>:<bucket>`), or `None`
/// for an operator-authored ARN.
pub fn replication_target_arn_deployment_id(arn: &str) -> Option<String> {
let parts: Vec<_> = arn.split(':').collect();
if parts.len() == 6
&& parts[0] == "arn"
&& matches!(parts[1], "rustfs" | "minio")
&& parts[2] == "replication"
&& !parts[4].is_empty()
{
return Some(parts[4].to_string());
}
None
}
/// Rule id prefix the site-replication reconciler stamps on the rules it
/// derives (`site-repl-<peer deployment id>`).
pub const SITE_REPLICATION_RULE_ID_PREFIX: &str = "site-repl-";
/// Whether `rule` carries a site-replication rule id (`site-repl-*`). The
/// reconciler and the peer ingestion path treat the whole namespace as theirs
/// on a site-replication bucket; the S3 edit path must not — rule ids are not
/// reserved, so see [`site_replication_rule_deployment_id`].
pub fn is_site_replication_rule(rule: &ReplicationRule) -> bool {
rule.id
.as_deref()
.is_some_and(|id| id.starts_with(SITE_REPLICATION_RULE_ID_PREFIX))
}
/// Deployment id of the peer a reconciler-derived rule replicates to, or
/// `None` for any other rule. The reconciler builds each rule from one peer:
/// the id is `site-repl-<deployment id>` and the destination ARN names that
/// same deployment id — an operator-authored `site-repl-user` rule, or a
/// `site-repl-<peer>` id pasted onto a foreign ARN, fails the agreement check.
/// Callers that know the current peer set must also confirm the id is one of
/// those peers before treating the rule as reconciler-owned.
pub fn site_replication_rule_deployment_id(rule: &ReplicationRule) -> Option<&str> {
let deployment_id = rule.id.as_deref()?.strip_prefix(SITE_REPLICATION_RULE_ID_PREFIX)?;
(!deployment_id.is_empty()
&& replication_target_arn_deployment_id(&rule.destination.bucket).as_deref() == Some(deployment_id))
.then_some(deployment_id)
}
/// Whether `rule` is one the local reconciler derived for a current remote
/// site-replication peer in `peer_deployment_ids`. With an empty peer set
/// (site replication disabled) nothing qualifies, so a bucket outside site
/// replication keeps the verbatim S3 put/delete semantics.
pub fn is_reconciler_owned_site_replication_rule(rule: &ReplicationRule, peer_deployment_ids: &HashSet<String>) -> bool {
site_replication_rule_deployment_id(rule).is_some_and(|deployment_id| peer_deployment_ids.contains(deployment_id))
}
/// Merge an incoming replication config into the local one.
///
/// `site-repl-*` rules encode the *holder's* outbound direction — their
/// destination ARN names another site — so applying an external rule set
/// verbatim replaces the local reverse rule with one this site can never
/// satisfy (no bucket target backs it) and replication silently stops. Only
/// operator-authored rules travel: the site-replication peer ingestion path
/// and the S3 put/delete-bucket-replication path both keep the local site's
/// `site-repl-*` rules through this merge. `incoming == None` models a
/// delete of the operator-authored rules.
pub fn merge_incoming_replication_config(
incoming: Option<ReplicationConfiguration>,
local: Option<ReplicationConfiguration>,
) -> Option<ReplicationConfiguration> {
merge_replication_config_keeping_site_rules(incoming, local, is_site_replication_rule)
}
/// [`merge_incoming_replication_config`] for the S3 put/delete-bucket-replication
/// path (issue #1948): only rules the local reconciler derived for a current
/// peer in `peer_deployment_ids` survive as site rules; every other stored
/// rule — including an operator-authored `site-repl-*` id — is operator state
/// that the request replaces or deletes. An incoming rule whose id is a
/// current peer's `site-repl-<id>` is dropped whatever its ARN: accepting it
/// would duplicate the reconciler rule's id.
pub fn merge_user_replication_config(
incoming: Option<ReplicationConfiguration>,
local: Option<ReplicationConfiguration>,
peer_deployment_ids: &HashSet<String>,
) -> Option<ReplicationConfiguration> {
let incoming = incoming.map(|mut config| {
config.rules.retain(|rule| {
!rule
.id
.as_deref()
.and_then(|id| id.strip_prefix(SITE_REPLICATION_RULE_ID_PREFIX))
.is_some_and(|deployment_id| peer_deployment_ids.contains(deployment_id))
});
config
});
merge_replication_config_keeping_site_rules(incoming, local, |rule| {
is_reconciler_owned_site_replication_rule(rule, peer_deployment_ids)
})
}
fn merge_replication_config_keeping_site_rules(
incoming: Option<ReplicationConfiguration>,
local: Option<ReplicationConfiguration>,
is_site_rule: impl Fn(&ReplicationRule) -> bool,
) -> Option<ReplicationConfiguration> {
let incoming_role = incoming.as_ref().map(|config| config.role.clone()).unwrap_or_default();
// Operator rules first, then the local site rules — the same order the
// site-replication reconciler produces, so its no-op check matches and
// the bucket metadata is written once per broadcast, not twice.
let mut rules: Vec<ReplicationRule> = incoming
.into_iter()
.flat_map(|config| config.rules)
.filter(|rule| !is_site_rule(rule))
.collect();
rules.extend(local.into_iter().flat_map(|config| config.rules).filter(&is_site_rule));
if rules.is_empty() {
return None;
}
assign_site_replication_rule_priorities(&mut rules, &is_site_rule);
// A site-replication ARN in `role` is the sender's, and the reconciler's
// per-peer target lookup reads it — carrying it over would pin the
// receiver's targets to the sender's identity.
let role = match replication_target_arn_deployment_id(&incoming_role) {
Some(_) => String::new(),
None => incoming_role,
};
Some(ReplicationConfiguration { role, rules })
}
/// Give the site rules in `rules` the lowest priorities no operator rule uses,
/// in rule order, leaving every operator rule's priority untouched. Operator
/// priorities decide which rule wins per target, so they are part of the
/// submitted policy; site rules are derived state and only need to be unique
/// (`validate_replication_config_structure` rejects duplicates). The result
/// is a pure function of the rule list, so the site-replication reconciler,
/// the peer ingestion merge and the S3 edit merge all converge on the same
/// bytes and the reconciler's no-op check holds.
pub fn assign_site_replication_rule_priorities(rules: &mut [ReplicationRule], is_site_rule: impl Fn(&ReplicationRule) -> bool) {
let taken: HashSet<i32> = rules
.iter()
.filter(|rule| !is_site_rule(rule))
.map(|rule| rule.priority.unwrap_or(0))
.collect();
let mut next = 1;
for rule in rules.iter_mut().filter(|rule| is_site_rule(rule)) {
while taken.contains(&next) {
next += 1;
}
rule.priority = Some(next);
next = next.saturating_add(1);
}
}
pub fn replication_target_arns(config: &ReplicationConfiguration) -> HashSet<String> {
let role = config.role.trim();
if !role.is_empty() {
@@ -1544,4 +1698,103 @@ mod tests {
"the child rule must win for target A while the overlapping child target B remains eligible"
);
}
#[test]
fn site_replication_rule_deployment_id_requires_id_and_arn_agreement() {
let reconciler_rule = replication_rule("site-repl-peer-dep", "arn:rustfs:replication::peer-dep:bucket");
assert_eq!(site_replication_rule_deployment_id(&reconciler_rule), Some("peer-dep"));
// A remote-target ARN carries the remote's deployment id (or a random
// uuid), never the operator's rule id.
let operator_named_rule = replication_rule("site-repl-user", "arn:minio:replication:us-east-1:2f1c-remote:bucket");
assert_eq!(site_replication_rule_deployment_id(&operator_named_rule), None);
let foreign_arn = replication_rule("site-repl-peer-dep", "arn:rustfs:replication::other-dep:bucket");
assert_eq!(site_replication_rule_deployment_id(&foreign_arn), None);
let empty_id = replication_rule("site-repl-", "arn:rustfs:replication::peer-dep:bucket");
assert_eq!(site_replication_rule_deployment_id(&empty_id), None);
let peers = HashSet::from(["peer-dep".to_string()]);
assert!(is_reconciler_owned_site_replication_rule(&reconciler_rule, &peers));
assert!(!is_reconciler_owned_site_replication_rule(&reconciler_rule, &HashSet::new()));
let removed_peer = replication_rule("site-repl-gone-dep", "arn:rustfs:replication::gone-dep:bucket");
assert!(!is_reconciler_owned_site_replication_rule(&removed_peer, &peers));
}
// The merge must not rewrite the operator's priorities: with the
// priority-5 rule listed first and renumbered 1 then 2, the priority-1
// delete-marker-disabled rule would win the replication decision.
#[test]
fn merge_keeps_operator_priorities_and_replication_decision() {
let user_arn = "arn:minio:replication:us-east-1:2f1c-remote:bucket";
let peer_arn = "arn:rustfs:replication::peer-dep:bucket";
let incoming = ReplicationConfiguration {
role: String::new(),
rules: vec![
delete_marker_rule("dm-enabled", user_arn, "logs/", 5, true),
delete_marker_rule("dm-disabled", user_arn, "logs/2026/", 1, false),
],
};
let mut site_rule = delete_marker_rule("site-repl-peer-dep", peer_arn, "", 7, true);
site_rule.prefix = None;
let local = structure_config(vec![site_rule]);
let opts = ObjectOpts {
name: "logs/2026/app.log".to_string(),
op_type: ReplicationType::Delete,
delete_marker: true,
version_id: None,
..Default::default()
};
let submitted: Vec<_> = incoming.filter_target_replication_decisions(&opts);
let peers = HashSet::from(["peer-dep".to_string()]);
let merged = merge_user_replication_config(Some(incoming.clone()), Some(local.clone()), &peers).expect("rules");
let priorities: Vec<_> = merged
.rules
.iter()
.map(|rule| (rule.id.as_deref().unwrap(), rule.priority))
.collect();
assert_eq!(
priorities,
vec![
("dm-enabled", Some(5)),
("dm-disabled", Some(1)),
("site-repl-peer-dep", Some(2))
],
"operator priorities are kept verbatim; the site rule takes the lowest free slot"
);
assert!(validate_replication_config_structure(&merged).is_ok());
let mut decisions = merged.filter_target_replication_decisions(&opts);
decisions.retain(|(arn, _)| arn == user_arn);
assert_eq!(decisions, submitted, "the merged config must replicate exactly as the operator submitted");
assert_eq!(decisions, vec![(user_arn.to_string(), true)]);
// The peer ingestion merge follows the same rule.
let merged = merge_incoming_replication_config(Some(incoming), Some(local)).expect("rules");
let priorities: Vec<_> = merged.rules.iter().map(|rule| rule.priority).collect();
assert_eq!(priorities, vec![Some(5), Some(1), Some(2)]);
}
#[test]
fn site_rule_priorities_skip_every_operator_priority() {
let mut rules = vec![
delete_marker_rule("a", "arn:a", "", 2, true),
delete_marker_rule("site-repl-x", "arn:rustfs:replication::x:b", "", 9, true),
delete_marker_rule("b", "arn:a", "", 1, true),
delete_marker_rule("site-repl-y", "arn:rustfs:replication::y:b", "", 9, true),
delete_marker_rule("c", "arn:a", "", 4, true),
];
assign_site_replication_rule_priorities(&mut rules, is_site_replication_rule);
let priorities: Vec<_> = rules.iter().map(|rule| rule.priority).collect();
assert_eq!(priorities, vec![Some(2), Some(3), Some(1), Some(5), Some(4)]);
assert!(validate_replication_config_structure(&structure_config(rules.clone())).is_ok());
// Idempotent, so the reconciler's pass over an already-merged config
// is a byte-stable no-op rather than a rewrite every period.
let settled = rules.clone();
assign_site_replication_rule_priorities(&mut rules, is_site_replication_rule);
assert_eq!(rules, settled);
}
}
+5 -3
View File
@@ -32,9 +32,11 @@ pub use config::{
ObjectOpts, REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION, REMOTE_TARGET_UNSUPPORTED_FIELDS, REMOTE_TARGET_WRITABLE_FIELDS,
REPLICATION_CAPABILITY_CONTRACT_VERSION, REPLICATION_READ_ONLY_HISTORICAL_FIELDS, REPLICATION_WRITABLE_FIELDS,
ReplicationConfigStructureError, ReplicationConfigurationExt, ReplicationTargetValidationError,
active_replication_rule_destination_arns, invalid_replication_config_status_field, replication_target_arns,
should_remove_replication_target, unsupported_replication_config_field, validate_replication_config_structure,
validate_replication_config_target_arns,
active_replication_rule_destination_arns, assign_site_replication_rule_priorities, invalid_replication_config_status_field,
is_reconciler_owned_site_replication_rule, is_site_replication_rule, merge_incoming_replication_config,
merge_user_replication_config, replication_target_arn_deployment_id, replication_target_arns,
should_remove_replication_target, site_replication_rule_deployment_id, unsupported_replication_config_field,
validate_replication_config_structure, validate_replication_config_target_arns,
};
pub use delete::{
DeletedObjectReplicationInfo, delete_marker_purge_mrf_entry, delete_marker_purge_version_id,
+38 -255
View File
@@ -177,15 +177,12 @@ async fn rollback_cluster_rebalance_start(
terminal_reload_attempt_at: Some(terminal_reload_attempt_at),
terminal_reload_failures: terminal_reload_failures.clone(),
};
store
.record_rebalance_stop_propagation(rebalance_id, record)
.await
.map_err(|err| {
format!(
"cluster rebalance rollback for {rebalance_id} partial; failed to persist stop propagation: {err}; {}",
rebalance_rollback_failure_message(rebalance_id, &stop_failures, &terminal_reload_failures)
)
})?;
store.record_rebalance_stop_propagation(record).await.map_err(|err| {
format!(
"cluster rebalance rollback for {rebalance_id} partial; failed to persist stop propagation: {err}; {}",
rebalance_rollback_failure_message(rebalance_id, &stop_failures, &terminal_reload_failures)
)
})?;
return Err(rebalance_rollback_failure_message(
rebalance_id,
&stop_failures,
@@ -200,7 +197,7 @@ async fn rollback_cluster_rebalance_start(
.await
.map_err(|err| format!("local stop_rebalance rollback for {rebalance_id} failed: {err}"))?;
store
.save_rebalance_stats_for_id(usize::MAX, RebalSaveOpt::StoppedAt, rebalance_id)
.save_rebalance_stats(usize::MAX, RebalSaveOpt::StoppedAt)
.await
.map_err(|err| format!("local rollback stop metadata save for {rebalance_id} failed: {err}"))?;
Ok(())
@@ -682,7 +679,7 @@ impl Operation for RebalanceStart {
terminal_reload_attempt_at: Some(terminal_reload_attempt_at),
terminal_reload_failures: terminal_reload_failures.clone(),
};
store.record_rebalance_stop_propagation(&id, record).await.map_err(|err| {
store.record_rebalance_stop_propagation(record).await.map_err(|err| {
rebalance_internal_error(format!(
"failed to persist rebalance local-start rollback propagation metadata: {err}"
))
@@ -872,38 +869,6 @@ impl Operation for RebalanceStatus {
}
}
async fn rebalance_stop_target_id(store: &Arc<ECStore>) -> S3Result<Option<String>> {
store
.prepare_rebalance_stop()
.await
.map_err(|e| s3_error!(InternalError, "failed to prepare rebalance metadata for stop: {}", e))
}
async fn stop_rebalance_admission_first(
store: &Arc<ECStore>,
notification_sys: Option<&NotificationSys>,
expected_rebalance_id: &str,
) -> S3Result<Vec<String>> {
// prepare_rebalance_stop already closed admission for this exact run.
if let Some(notification_sys) = notification_sys {
return notification_sys
.stop_rebalance_failures(Some(expected_rebalance_id))
.await
.map_err(|e| s3_error!(InternalError, "failed to stop rebalance via notification system: {}", e));
}
store
.stop_rebalance_for_id(Some(expected_rebalance_id))
.await
.map_err(|e| s3_error!(InternalError, "failed to stop rebalance: {}", e))?;
store
.save_rebalance_stats_for_id(usize::MAX, RebalSaveOpt::StoppedAt, expected_rebalance_id)
.await
.map_err(|e| s3_error!(InternalError, "failed to persist rebalance stop metadata: {}", e))?;
Ok(Vec::new())
}
// RebalanceStop
pub struct RebalanceStop {}
@@ -951,15 +916,36 @@ impl Operation for RebalanceStop {
return Err(s3_error!(InternalError, "object layer is not initialized"));
};
let Some(expected_rebalance_id) = rebalance_stop_target_id(&store).await? else {
store
.load_rebalance_meta()
.await
.map_err(|e| s3_error!(InternalError, "failed to load rebalance metadata before stop: {}", e))?;
let expected_rebalance_id = store.current_rebalance_id().await;
if !store.is_rebalance_conflicting_with_decommission().await {
log_rebalance_request_rejected("stop", "rebalance_not_started", &request_id, &actor, &remote_addr);
return Err(s3_error!(NoSuchResource, "pool rebalance is not started"));
};
}
let notification_sys = current_notification_system();
let stop_attempt_at = OffsetDateTime::now_utc();
let stop_failures =
stop_rebalance_admission_first(&store, notification_sys.as_deref(), expected_rebalance_id.as_str()).await?;
let mut stop_failures = Vec::new();
if let Some(notification_sys) = notification_sys.as_ref() {
stop_failures = notification_sys
.stop_rebalance_failures(expected_rebalance_id.as_deref())
.await
.map_err(|e| s3_error!(InternalError, "failed to stop rebalance via notification system: {}", e))?;
} else {
store
.stop_rebalance_for_id(expected_rebalance_id.as_deref())
.await
.map_err(|e| s3_error!(InternalError, "failed to stop rebalance: {}", e))?;
store
.save_rebalance_stats(usize::MAX, RebalSaveOpt::StoppedAt)
.await
.map_err(|e| s3_error!(InternalError, "failed to persist rebalance stop metadata: {}", e))?;
}
info!(
event = EVENT_ADMIN_REBALANCE_STATE,
@@ -1021,7 +1007,7 @@ impl Operation for RebalanceStop {
terminal_reload_failures: terminal_reload_failures.clone(),
};
store
.record_rebalance_stop_propagation(expected_rebalance_id.as_str(), record)
.record_rebalance_stop_propagation(record)
.await
.map_err(|e| s3_error!(InternalError, "failed to persist rebalance stop propagation metadata: {}", e))?;
@@ -1095,218 +1081,15 @@ mod rebalance_handler_tests {
RebalPoolProgress, RebalanceAdminStatus, RebalancePoolStatus, RebalanceStartStep, RebalanceStopPropagationStatus,
build_rebalance_admin_status, build_rebalance_pool_statuses, build_rebalance_stop_propagation_status,
rebalance_pool_used, rebalance_query_present, rebalance_remaining_buckets, rebalance_rollback_failure_message,
rebalance_rollback_stop_failure_message, rebalance_start_rollback_error, rebalance_start_steps, rebalance_stop_target_id,
rebalance_used_pct, rollback_result_label, stop_rebalance_admission_first,
rebalance_rollback_stop_failure_message, rebalance_start_rollback_error, rebalance_start_steps, rebalance_used_pct,
rollback_result_label,
};
use crate::admin::storage_api::rebalance::{
DiskStat, RebalSaveOpt, RebalStatus, RebalanceCleanupWarningEntry, RebalanceCleanupWarnings, RebalanceInfo,
RebalanceMeta, RebalanceStats, RebalanceStopPropagationRecord, encode_rebalance_stop_propagation_record,
DiskStat, RebalStatus, RebalanceCleanupWarningEntry, RebalanceCleanupWarnings, RebalanceInfo, RebalanceMeta,
RebalanceStats, RebalanceStopPropagationRecord, encode_rebalance_stop_propagation_record,
};
use time::OffsetDateTime;
fn started_rebalance_meta(id: &str) -> RebalanceMeta {
RebalanceMeta {
id: id.to_string(),
pool_stats: vec![RebalanceStats {
participating: true,
info: RebalanceInfo {
start_time: Some(OffsetDateTime::now_utc()),
status: RebalStatus::Started,
..Default::default()
},
..Default::default()
}],
..Default::default()
}
}
#[tokio::test]
#[serial_test::serial]
async fn real_admin_stop_cancels_paused_entry_before_waiting_for_activation_gate() {
const REBALANCE_ID: &str = "admin-stop-paused-entry";
let mut fixture =
crate::admin::storage_api::ecstore_rebalance::test_util::PausedRebalanceEntryTestFixture::new(REBALANCE_ID).await;
fixture.wait_until_entry_paused().await;
let stop_store = fixture.store();
let mut stop_task = tokio::spawn(async move {
let expected_rebalance_id = rebalance_stop_target_id(&stop_store)
.await
.expect("admin stop target resolution should succeed")
.expect("the active rebalance should remain stoppable");
stop_rebalance_admission_first(&stop_store, None, expected_rebalance_id.as_str()).await
});
fixture.wait_until_admission_cancelled().await;
fixture.wait_until_stop_waiting_for_entry().await;
assert!(!stop_task.is_finished(), "admin stop must wait for the paused entry guard to drain");
fixture.release_entry();
fixture.assert_entry_cancelled().await;
let stop_failures = tokio::time::timeout(std::time::Duration::from_secs(5), &mut stop_task)
.await
.expect("admin stop should finish after the entry guard drains")
.expect("admin stop task should not panic")
.expect("admin stop should persist the terminal state");
assert!(stop_failures.is_empty());
assert!(!fixture.store().is_rebalance_conflicting_with_decommission().await);
}
#[tokio::test]
#[serial_test::serial]
async fn real_admin_stop_accepts_same_run_terminalization_after_prepare() {
const REBALANCE_ID: &str = "admin-stop-terminal-after-prepare";
const REPLACEMENT_ID: &str = "admin-stop-replacement";
let (_temp_dirs, store) =
crate::admin::storage_api::ecstore_rebalance::test_util::test_store_with_persisted_rebalance_meta(
started_rebalance_meta(REBALANCE_ID),
)
.await;
let terminal_barrier = std::sync::Arc::new(tokio::sync::Barrier::new(2));
let worker_barrier = std::sync::Arc::clone(&terminal_barrier);
let worker_store = std::sync::Arc::clone(&store);
let terminal_task = tokio::spawn(async move {
worker_barrier.wait().await;
{
let mut rebalance_meta = worker_store.rebalance_meta.write().await;
let meta = rebalance_meta
.as_mut()
.expect("the prepared rebalance metadata should remain installed");
assert_eq!(meta.id, REBALANCE_ID);
let pool = meta
.pool_stats
.first_mut()
.expect("the prepared rebalance should have a pool");
pool.info.status = RebalStatus::Stopped;
pool.info.end_time = Some(OffsetDateTime::now_utc());
}
worker_store
.save_rebalance_stats_for_id(0, RebalSaveOpt::Stats, REBALANCE_ID)
.await
});
let expected_rebalance_id = rebalance_stop_target_id(&store)
.await
.expect("admin stop target resolution should succeed")
.expect("the active rebalance should remain stoppable");
assert_eq!(expected_rebalance_id, REBALANCE_ID);
let cancel = store
.rebalance_meta
.read()
.await
.as_ref()
.and_then(|meta| meta.cancel.clone())
.expect("prepare should install the admission cancellation token");
assert!(cancel.is_cancelled());
terminal_barrier.wait().await;
tokio::time::timeout(std::time::Duration::from_secs(30), terminal_task)
.await
.expect("worker terminalization should finish after the barrier opens")
.expect("worker terminalization task should not panic")
.expect("worker terminalization should persist");
assert!(!store.is_rebalance_conflicting_with_decommission().await);
*store.rebalance_meta.write().await = None;
store
.load_rebalance_meta()
.await
.expect("the worker terminal state should reload before the final stop");
{
let terminal = store.rebalance_meta.read().await;
let terminal = terminal.as_ref().expect("the worker terminal state should remain persisted");
assert_eq!(terminal.id, REBALANCE_ID);
assert_eq!(terminal.pool_stats[0].info.status, RebalStatus::Stopped);
}
let stop_failures = stop_rebalance_admission_first(&store, None, expected_rebalance_id.as_str())
.await
.expect("same-run terminalization after prepare should be a successful stop");
assert!(stop_failures.is_empty());
*store.rebalance_meta.write().await = Some(started_rebalance_meta(REPLACEMENT_ID));
let error = stop_rebalance_admission_first(&store, None, expected_rebalance_id.as_str())
.await
.expect_err("the prepared stop must not mutate a replacement run");
assert!(error.to_string().contains(REBALANCE_ID));
let replacement = store.rebalance_meta.read().await;
let replacement = replacement.as_ref().expect("the replacement run should remain installed");
assert_eq!(replacement.id, REPLACEMENT_ID);
assert_eq!(replacement.pool_stats[0].info.status, RebalStatus::Started);
assert!(replacement.cancel.is_none());
assert!(replacement.stopped_at.is_none());
}
#[tokio::test]
#[serial_test::serial]
async fn real_admin_stop_loads_persisted_active_rebalance_from_cold_memory() {
const REBALANCE_ID: &str = "admin-stop-cold-memory";
let (_temp_dirs, store) =
crate::admin::storage_api::ecstore_rebalance::test_util::test_store_with_persisted_rebalance_meta(
started_rebalance_meta(REBALANCE_ID),
)
.await;
*store.rebalance_meta.write().await = None;
assert!(store.current_rebalance_id().await.is_none());
let expected_rebalance_id = rebalance_stop_target_id(&store)
.await
.expect("admin stop should load persisted rebalance metadata")
.expect("persisted active rebalance should be stoppable");
assert_eq!(expected_rebalance_id, REBALANCE_ID);
let stop_failures = stop_rebalance_admission_first(&store, None, expected_rebalance_id.as_str())
.await
.expect("admin stop should persist the terminal state after a cold load");
assert!(stop_failures.is_empty());
assert!(!store.is_rebalance_conflicting_with_decommission().await);
*store.rebalance_meta.write().await = None;
store
.load_rebalance_meta()
.await
.expect("the persisted terminal rebalance metadata should remain readable");
assert_eq!(store.current_rebalance_id().await.as_deref(), Some(REBALANCE_ID));
assert!(!store.is_rebalance_conflicting_with_decommission().await);
}
#[tokio::test]
#[serial_test::serial]
async fn real_admin_stop_refreshes_persisted_active_over_stale_inactive_memory() {
const PERSISTED_REBALANCE_ID: &str = "admin-stop-persisted-active";
const STALE_REBALANCE_ID: &str = "admin-stop-stale-terminal";
let (_temp_dirs, store) =
crate::admin::storage_api::ecstore_rebalance::test_util::test_store_with_persisted_rebalance_meta(
started_rebalance_meta(PERSISTED_REBALANCE_ID),
)
.await;
*store.rebalance_meta.write().await = Some(RebalanceMeta {
id: STALE_REBALANCE_ID.to_string(),
stopped_at: Some(OffsetDateTime::now_utc()),
..Default::default()
});
assert_eq!(store.current_rebalance_id().await.as_deref(), Some(STALE_REBALANCE_ID));
assert!(!store.is_rebalance_conflicting_with_decommission().await);
let expected_rebalance_id = rebalance_stop_target_id(&store)
.await
.expect("admin stop should refresh stale inactive local metadata")
.expect("persisted active rebalance should replace the stale local terminal state");
assert_eq!(expected_rebalance_id, PERSISTED_REBALANCE_ID);
let stop_failures = stop_rebalance_admission_first(&store, None, expected_rebalance_id.as_str())
.await
.expect("admin stop should persist the refreshed run's terminal state");
assert!(stop_failures.is_empty());
*store.rebalance_meta.write().await = None;
store
.load_rebalance_meta()
.await
.expect("the refreshed run's persisted terminal metadata should remain readable");
assert_eq!(store.current_rebalance_id().await.as_deref(), Some(PERSISTED_REBALANCE_ID));
assert!(!store.is_rebalance_conflicting_with_decommission().await);
}
#[test]
fn test_calculate_rebalance_progress_running() {
let start = OffsetDateTime::from_unix_timestamp(1_000).unwrap();
+42 -73
View File
@@ -31,6 +31,10 @@ use crate::admin::storage_api::bucket::metadata::{
use crate::admin::storage_api::bucket::metadata_sys;
use crate::admin::storage_api::bucket::quota::BucketQuota;
use crate::admin::storage_api::bucket::replication;
use crate::admin::storage_api::bucket::replication::{
assign_site_replication_rule_priorities, is_site_replication_rule, merge_incoming_replication_config,
replication_target_arn_deployment_id,
};
use crate::admin::storage_api::bucket::target::{ARN, BucketTarget, BucketTargetType, BucketTargets, Credentials};
use crate::admin::storage_api::bucket::target_sys::BucketTargetSys;
use crate::admin::storage_api::bucket::utils::{deserialize, serialize};
@@ -1118,6 +1122,36 @@ async fn load_site_replication_state() -> S3Result<SiteReplicationState> {
}
}
/// Whether this deployment participates in site replication (two or more
/// peers in the persisted state). Read by the S3 interface layer to gate
/// replication-config edits (MinIO `ErrReplicationDenyEditError` semantics,
/// issue #1948); a state-read failure propagates so the gate fails closed.
pub(crate) async fn site_replication_enabled() -> S3Result<bool> {
Ok(load_site_replication_state().await?.enabled())
}
/// Deployment ids of the remote peers the reconciler derives a
/// `site-repl-<id>` rule for on every bucket (the same peer filter as
/// `build_site_replication_config`); empty when site replication is not
/// enabled. Read by the bucket usecase so an S3 replication-config edit keeps
/// exactly the reconciler-owned rules (issue #1948); a state-read failure
/// propagates so the edit fails closed.
pub(crate) async fn site_replication_remote_peer_deployment_ids() -> S3Result<HashSet<String>> {
let state = load_site_replication_state().await?;
if !state.enabled() {
return Ok(HashSet::new());
}
let local_peer = current_local_runtime_peer(&state);
Ok(state
.peers
.values()
.filter(|peer| {
peer.deployment_id != local_peer.deployment_id && !same_identity_endpoint(&peer.endpoint, &local_peer.endpoint)
})
.map(|peer| peer.deployment_id.clone())
.collect())
}
async fn load_site_replication_state_no_lock(store: Arc<ECStore>) -> S3Result<SiteReplicationState> {
match read_config_no_lock(store, SITE_REPLICATION_STATE_PATH).await {
Ok(data) => parse_site_replication_state(&data),
@@ -7762,20 +7796,6 @@ fn bucket_target_deployment_id(target: &BucketTarget) -> Option<String> {
replication_target_arn_deployment_id(&target.arn)
}
fn replication_target_arn_deployment_id(arn: &str) -> Option<String> {
let parts: Vec<_> = arn.split(':').collect();
if parts.len() == 6
&& parts[0] == "arn"
&& matches!(parts[1], "rustfs" | "minio")
&& parts[2] == "replication"
&& !parts[4].is_empty()
{
return Some(parts[4].to_string());
}
None
}
fn prune_removed_site_replication_bucket_targets(
existing: BucketTargets,
removed_deployment_ids: &HashSet<String>,
@@ -7800,10 +7820,6 @@ fn prune_removed_site_replication_bucket_targets(
(BucketTargets { targets }, removed)
}
fn is_site_replication_rule(rule: &ReplicationRule) -> bool {
rule.id.as_deref().is_some_and(|id| id.starts_with("site-repl-"))
}
/// Whether every `site-repl-*` rule on this bucket resolves to a live remote target.
///
/// The rule set alone cannot answer this: a rule can be perfectly formed while the endpoint
@@ -7829,52 +7845,6 @@ async fn site_replication_targets_online(bucket: &str, replication_config_xml: &
true
}
/// Merge a peer's replication config into the local one.
///
/// `site-repl-*` rules encode the *sender's* outbound direction — their destination ARN
/// names the receiver — so applying a peer's rule set verbatim replaces the receiver's
/// reverse rule with one pointing at itself. No bucket target can satisfy that ARN
/// (`reconcile_site_replication_bucket_targets` skips the local peer), so the receiver
/// silently stops replicating back: the one-directional symptom. Only operator-authored
/// rules travel between sites; each site owns its own `site-repl-*` rules.
fn merge_incoming_replication_config(
incoming: Option<ReplicationConfiguration>,
local: Option<ReplicationConfiguration>,
) -> Option<ReplicationConfiguration> {
let incoming_role = incoming.as_ref().map(|config| config.role.clone()).unwrap_or_default();
// Operator rules first, then the local site rules — the same order
// `ensure_site_replication_bucket_replication_config_with_runtime` produces, so its
// no-op check matches and the bucket metadata is written once per broadcast, not twice.
let mut rules: Vec<ReplicationRule> = incoming
.into_iter()
.flat_map(|config| config.rules)
.filter(|rule| !is_site_replication_rule(rule))
.collect();
rules.extend(
local
.into_iter()
.flat_map(|config| config.rules)
.filter(is_site_replication_rule),
);
if rules.is_empty() {
return None;
}
for (index, rule) in rules.iter_mut().enumerate() {
rule.priority = Some(i32::try_from(index + 1).unwrap_or(i32::MAX));
}
// A site-replication ARN in `role` is the sender's, and `site_replication_target_arns_by_peer`
// reads it — carrying it over would pin the receiver's targets to the sender's identity.
let role = match replication_target_arn_deployment_id(&incoming_role) {
Some(_) => String::new(),
None => incoming_role,
};
Some(ReplicationConfiguration { role, rules })
}
/// Merge a peer's ILM expiry document into the local lifecycle config.
///
/// Mirrors MinIO's `mergeWithCurrentLCConfig` with one hardening: incoming
@@ -8213,9 +8183,7 @@ fn prune_removed_site_replication_rules(
return (None, removed);
}
for (index, rule) in config.rules.iter_mut().enumerate() {
rule.priority = Some(i32::try_from(index + 1).unwrap_or(i32::MAX));
}
assign_site_replication_rule_priorities(&mut config.rules, is_site_replication_rule);
(Some(config), removed)
}
@@ -8389,9 +8357,10 @@ async fn ensure_site_replication_bucket_replication_config_with_runtime(
.cloned()
.collect();
rules.extend(desired.rules);
for (index, rule) in rules.iter_mut().enumerate() {
rule.priority = Some(i32::try_from(index + 1).unwrap_or(i32::MAX));
}
// Operator priorities are the operator's policy; only the derived rules
// take free slots, by the same function as the config merges so a merged
// write and this pass agree byte for byte.
assign_site_replication_rule_priorities(&mut rules, is_site_replication_rule);
// Only a site-replication ARN in `role` is ours to drop — an operator-authored role is
// part of the bucket's S3-visible configuration, and repairing a reverse rule must not
@@ -17089,7 +17058,7 @@ mod tests {
}
#[test]
fn test_prune_removed_site_replication_rules_removes_site_rule_and_reorders_priorities() {
fn test_prune_removed_site_replication_rules_removes_site_rule_and_keeps_operator_priority() {
let removed_deployment_ids = HashSet::from(["removed-dep".to_string()]);
let kept_rule = build_site_replication_rule("arn:rustfs:replication::kept-dep:photos", 3, "site-repl-kept-dep");
let removed_rule = build_site_replication_rule("arn:rustfs:replication::removed-dep:photos", 1, "site-repl-removed-dep");
@@ -17106,9 +17075,9 @@ mod tests {
assert!(updated.role.is_empty());
assert_eq!(updated.rules.len(), 2);
assert_eq!(updated.rules[0].id.as_deref(), Some("user-managed-rule"));
assert_eq!(updated.rules[0].priority, Some(1));
assert_eq!(updated.rules[0].priority, Some(9), "the operator's priority is policy and stays");
assert_eq!(updated.rules[1].id.as_deref(), Some("site-repl-kept-dep"));
assert_eq!(updated.rules[1].priority, Some(2));
assert_eq!(updated.rules[1].priority, Some(1), "the derived rule moves to the lowest free slot");
}
#[test]
+3 -3
View File
@@ -70,9 +70,7 @@ mod ecstore_notification {
}
#[allow(unused_imports)]
pub(crate) mod ecstore_rebalance {
#[cfg(test)]
pub(crate) use crate::storage::storage_api::ecstore_rebalance::test_util;
mod ecstore_rebalance {
pub(crate) use crate::storage::storage_api::ecstore_rebalance::{
DiskStat, RebalSaveOpt, RebalStatus, RebalanceCleanupWarningEntry, RebalanceCleanupWarnings, RebalanceInfo,
RebalanceMeta, RebalanceStats, RebalanceStopPropagationRecord, decode_rebalance_stop_propagation_record,
@@ -445,6 +443,8 @@ pub(crate) mod replication {
pub(crate) use super::ecstore_bucket::replication::{
REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION, REMOTE_TARGET_UNSUPPORTED_FIELDS, REMOTE_TARGET_WRITABLE_FIELDS,
REPLICATION_CAPABILITY_CONTRACT_VERSION, REPLICATION_READ_ONLY_HISTORICAL_FIELDS, REPLICATION_WRITABLE_FIELDS,
assign_site_replication_rule_priorities, is_site_replication_rule, merge_incoming_replication_config,
replication_target_arn_deployment_id,
};
pub(crate) type BucketReplicationResyncStatus = super::ecstore_bucket::replication::BucketReplicationResyncStatus;
pub(crate) type BucketStats = super::ecstore_bucket::replication::BucketStats;
+268 -16
View File
@@ -38,9 +38,9 @@ use super::storage_api::bucket_usecase::bucket::{
metadata_sys,
policy_sys::PolicySys,
replication::{
ReplicationTargetValidationError, invalid_replication_config_status_field, replication_target_arns,
should_remove_replication_target, unsupported_replication_config_field, validate_replication_config_structure,
validate_replication_config_target_arns,
ReplicationTargetValidationError, invalid_replication_config_status_field, merge_user_replication_config,
replication_target_arns, should_remove_replication_target, unsupported_replication_config_field,
validate_replication_config_structure, validate_replication_config_target_arns,
},
target::{BucketTargetType, BucketTargets},
utils::serialize,
@@ -623,11 +623,52 @@ async fn validate_bucket_replication_update(bucket: &str, config: &ReplicationCo
validate_replication_config_targets(&targets, config)
}
async fn replication_targets_without_config_targets(
/// Defense in depth for site-replication-managed buckets (issue #1948): an S3
/// PutBucketReplication replaces the operator-authored rules but must not wipe
/// the rules the reconciler derived for the current remote peers
/// (`site_peer_deployment_ids`) — until its next pass (600s period) every
/// peer link on this bucket would be silently dead. The same merge also drops
/// incoming impostors of those rules. An empty peer set (site replication
/// disabled) keeps the verbatim overwrite semantics: rule ids are not
/// reserved, so an operator's own `site-repl-*` rule is ordinary state there.
fn merge_user_replication_config_update(
incoming: ReplicationConfiguration,
existing: Option<ReplicationConfiguration>,
site_peer_deployment_ids: &HashSet<String>,
) -> ReplicationConfiguration {
if site_peer_deployment_ids.is_empty() {
return incoming;
}
// `incoming` passed structure validation, so it holds at least one rule;
// `None` is only reachable when every incoming rule impersonates a
// reconciler rule, and then the stored reconciler rules are what remains.
merge_user_replication_config(Some(incoming.clone()), existing, site_peer_deployment_ids).unwrap_or(incoming)
}
/// Split of an S3 DeleteBucketReplication on the stored config (issue #1948):
/// the operator-authored rules are removed, the rules the reconciler derived
/// for the current remote peers survive (`None` means nothing survives and
/// the config is deleted), and the returned ARNs are the ones whose bucket
/// targets may be garbage-collected — never an ARN a surviving reconciler
/// rule still points at.
fn split_replication_config_for_user_delete(
config: ReplicationConfiguration,
site_peer_deployment_ids: &HashSet<String>,
) -> (Option<ReplicationConfiguration>, HashSet<String>) {
let mut removable_arns = replication_target_arns(&config);
let remaining = merge_user_replication_config(None, Some(config), site_peer_deployment_ids);
if let Some(remaining) = remaining.as_ref() {
for rule in &remaining.rules {
removable_arns.remove(rule.destination.bucket.trim());
}
}
(remaining, removable_arns)
}
async fn replication_targets_without_arns(
bucket: &str,
config: &ReplicationConfiguration,
target_arns: &HashSet<String>,
) -> S3Result<Option<(BucketTargets, usize)>> {
let target_arns = replication_target_arns(config);
if target_arns.is_empty() {
return Ok(None);
}
@@ -638,7 +679,7 @@ async fn replication_targets_without_config_targets(
Err(err) => return Err(ApiError::from(err).into()),
};
let removed = remove_replication_targets_from_config_targets(&mut targets, &target_arns);
let removed = remove_replication_targets_from_config_targets(&mut targets, target_arns);
if removed == 0 {
return Ok(None);
}
@@ -1582,9 +1623,15 @@ impl DefaultBucketUsecase {
Ok(S3Response::new(DeleteBucketPolicyOutput {}))
}
/// `site_peers` is the set of remote site-replication peer deployment ids
/// (empty when site replication is disabled). The interface layer reads it
/// from the persisted state and fails closed on a read error, so this
/// usecase stays a pure function of its inputs (layer rule: app never
/// imports interface).
pub async fn execute_delete_bucket_replication(
&self,
req: S3Request<DeleteBucketReplicationInput>,
site_peers: HashSet<String>,
) -> S3Result<S3Response<DeleteBucketReplicationOutput>> {
let expected_incarnation_id = bucket_config_mutation_incarnation(&req, &req.input.bucket)?;
let request_context = req.extensions.get::<request_context::RequestContext>().cloned();
@@ -1604,15 +1651,29 @@ impl DefaultBucketUsecase {
Err(StorageError::ConfigNotFound) => None,
Err(err) => return Err(ApiError::from(err).into()),
};
let updated_targets = if let Some(config) = replication_config.as_ref() {
replication_targets_without_config_targets(&bucket, config).await?
let (remaining_config, updated_targets) = if let Some(config) = replication_config.as_ref() {
let (remaining, removable_arns) = split_replication_config_for_user_delete(config.clone(), &site_peers);
let targets = replication_targets_without_arns(&bucket, &removable_arns).await?;
(remaining, targets)
} else {
None
(None, None)
};
delete_bucket_config_for_incarnation(&bucket, BUCKET_REPLICATION_CONFIG, expected_incarnation_id)
.await
.map_err(ApiError::from)?;
match remaining_config {
// Site-replication rules and the targets backing them survive the
// S3 delete (issue #1948); only the operator-authored rules go.
Some(remaining) => {
let data = serialize_config(&remaining)?;
update_bucket_config_for_incarnation(&bucket, BUCKET_REPLICATION_CONFIG, data, expected_incarnation_id)
.await
.map_err(ApiError::from)?;
}
None => {
delete_bucket_config_for_incarnation(&bucket, BUCKET_REPLICATION_CONFIG, expected_incarnation_id)
.await
.map_err(ApiError::from)?;
}
}
if let Some((targets, removed)) = updated_targets
&& let Err(err) =
write_replication_targets_after_config_delete(&bucket, &targets, removed, expected_incarnation_id).await
@@ -2459,9 +2520,11 @@ impl DefaultBucketUsecase {
Ok(S3Response::new(PutBucketCorsOutput::default()))
}
/// See [`Self::execute_delete_bucket_replication`] for `site_peers`.
pub async fn execute_put_bucket_replication(
&self,
req: S3Request<PutBucketReplicationInput>,
site_peers: HashSet<String>,
) -> S3Result<S3Response<PutBucketReplicationOutput>> {
let expected_incarnation_id = bucket_config_mutation_incarnation(&req, &req.input.bucket)?;
let request_context = req.extensions.get::<request_context::RequestContext>().cloned();
@@ -2485,6 +2548,13 @@ impl DefaultBucketUsecase {
let targets_guard = lock_bucket_targets_metadata(&bucket).await;
validate_bucket_replication_update(&bucket, &replication_configuration).await?;
let existing_config = match metadata_sys::get_replication_config(&bucket).await {
Ok((config, _)) => Some(config),
Err(StorageError::ConfigNotFound) => None,
Err(err) => return Err(ApiError::from(err).into()),
};
let replication_configuration =
merge_user_replication_config_update(replication_configuration, existing_config, &site_peers);
let data = serialize_config(&replication_configuration)?;
update_bucket_config_for_incarnation(&bucket, BUCKET_REPLICATION_CONFIG, data, expected_incarnation_id)
.await
@@ -3114,6 +3184,185 @@ mod tests {
assert!(arns.contains(destination));
}
fn replication_rule_with_id(arn: &str, id: &str, priority: i32) -> ReplicationRule {
let mut rule = replication_rule_for_target(arn);
rule.id = Some(id.to_string());
rule.priority = Some(priority);
rule
}
fn site_peers(deployment_ids: &[&str]) -> HashSet<String> {
deployment_ids.iter().map(|id| id.to_string()).collect()
}
#[test]
fn put_replication_merge_preserves_site_replication_rules() {
let existing = ReplicationConfiguration {
role: String::new(),
rules: vec![
replication_rule_with_id("arn:rustfs:replication::peer-dep:bucket", "site-repl-peer-dep", 1),
replication_rule_with_id("arn:rustfs:replication:us-east-1:old:bucket", "old-user-rule", 2),
],
};
let incoming = ReplicationConfiguration {
role: String::new(),
rules: vec![
replication_rule_with_id("arn:rustfs:replication:us-east-1:new:bucket", "new-user-rule", 1),
replication_rule_with_id("arn:rustfs:replication::forged-dep:bucket", "site-repl-peer-dep", 2),
replication_rule_with_id("arn:rustfs:replication::other-dep:bucket", "site-repl-other", 3),
],
};
let merged = merge_user_replication_config_update(incoming, Some(existing), &site_peers(&["peer-dep"]));
let rules: Vec<_> = merged
.rules
.iter()
.map(|rule| (rule.id.as_deref().unwrap_or_default(), rule.destination.bucket.as_str()))
.collect();
assert_eq!(
rules,
vec![
("new-user-rule", "arn:rustfs:replication:us-east-1:new:bucket"),
("site-repl-other", "arn:rustfs:replication::other-dep:bucket"),
("site-repl-peer-dep", "arn:rustfs:replication::peer-dep:bucket"),
],
"user rules replaced, the reconciler rule for the current peer kept over the incoming impostor, \
a site-repl-* id that names no current peer is ordinary operator state"
);
}
// Rule ids do not reserve `site-repl-*`: outside site replication an
// owner's `site-repl-user` rule is ordinary state, so PUT stores it
// verbatim and DELETE removes it and garbage-collects its target.
#[test]
fn put_then_delete_replication_without_site_replication_treats_site_repl_id_as_user_rule() {
let user_arn = "arn:minio:replication:us-east-1:2f1c-remote:bucket";
let incoming = ReplicationConfiguration {
role: String::new(),
rules: vec![replication_rule_with_id(user_arn, "site-repl-user", 1)],
};
let stored = merge_user_replication_config_update(incoming.clone(), None, &HashSet::new());
assert_eq!(stored, incoming, "PUT on a non-site-replication bucket is verbatim");
let (remaining, removable) = split_replication_config_for_user_delete(stored, &HashSet::new());
assert!(remaining.is_none(), "DELETE must remove the operator's site-repl-* rule");
assert_eq!(removable, HashSet::from([user_arn.to_string()]));
}
// Under site replication only a rule the reconciler would derive — id
// `site-repl-<peer>` for a current peer, destination ARN naming the same
// peer — is reconciler-owned. Everything else is operator state.
#[test]
fn delete_replication_split_keeps_only_reconciler_derived_rules() {
let peer_arn = "arn:rustfs:replication::peer-dep:bucket";
let user_arn = "arn:minio:replication:us-east-1:2f1c-remote:bucket";
let config = ReplicationConfiguration {
role: String::new(),
rules: vec![
replication_rule_with_id(user_arn, "site-repl-user", 1),
replication_rule_with_id(user_arn, "site-repl-peer-dep", 2),
replication_rule_with_id("arn:rustfs:replication::gone-dep:bucket", "site-repl-gone-dep", 3),
replication_rule_with_id(peer_arn, "site-repl-peer-dep", 4),
],
};
let (remaining, removable) = split_replication_config_for_user_delete(config, &site_peers(&["peer-dep"]));
let remaining = remaining.expect("the reconciler-derived rule must survive");
assert_eq!(remaining.rules.len(), 1);
assert_eq!(remaining.rules[0].destination.bucket, peer_arn);
assert_eq!(
removable,
HashSet::from([user_arn.to_string(), "arn:rustfs:replication::gone-dep:bucket".to_string()]),
"targets of operator rules and of a removed peer are garbage-collected"
);
}
#[test]
fn put_replication_merge_returns_incoming_verbatim_without_site_rules() {
let existing = ReplicationConfiguration {
role: String::new(),
rules: vec![replication_rule_with_id(
"arn:rustfs:replication:us-east-1:old:bucket",
"old-user-rule",
7,
)],
};
let incoming = ReplicationConfiguration {
role: String::new(),
rules: vec![replication_rule_with_id(
"arn:rustfs:replication:us-east-1:new:bucket",
"new-user-rule",
5,
)],
};
let merged = merge_user_replication_config_update(incoming.clone(), Some(existing), &HashSet::new());
assert_eq!(merged.role, incoming.role);
assert_eq!(merged.rules, incoming.rules, "non-SR buckets keep the verbatim overwrite semantics");
}
#[test]
fn delete_replication_split_keeps_site_rules_and_their_targets() {
let sr_arn = "arn:rustfs:replication::peer-dep:bucket";
let user_arn = "arn:rustfs:replication:us-east-1:user:bucket";
let config = ReplicationConfiguration {
role: String::new(),
rules: vec![
replication_rule_with_id(user_arn, "user-rule", 1),
replication_rule_with_id(sr_arn, "site-repl-peer-dep", 2),
],
};
let (remaining, removable) = split_replication_config_for_user_delete(config, &site_peers(&["peer-dep"]));
let remaining = remaining.expect("site-replication rules must survive a user delete");
let ids: Vec<_> = remaining
.rules
.iter()
.map(|rule| rule.id.as_deref().unwrap_or_default())
.collect();
assert_eq!(ids, vec!["site-repl-peer-dep"]);
assert_eq!(removable, HashSet::from([user_arn.to_string()]));
}
#[test]
fn delete_replication_split_protects_targets_shared_with_site_rules() {
let sr_arn = "arn:rustfs:replication::peer-dep:bucket";
let config = ReplicationConfiguration {
role: String::new(),
rules: vec![
replication_rule_with_id(sr_arn, "user-rule-on-sr-target", 1),
replication_rule_with_id(sr_arn, "site-repl-peer-dep", 2),
],
};
let (remaining, removable) = split_replication_config_for_user_delete(config, &site_peers(&["peer-dep"]));
assert!(remaining.is_some());
assert!(
removable.is_empty(),
"a target still referenced by a surviving site-replication rule must not be removed"
);
}
#[test]
fn delete_replication_split_removes_everything_without_site_rules() {
let user_arn = "arn:rustfs:replication:us-east-1:user:bucket";
let config = ReplicationConfiguration {
role: String::new(),
rules: vec![replication_rule_with_id(user_arn, "user-rule", 1)],
};
let (remaining, removable) = split_replication_config_for_user_delete(config, &site_peers(&["peer-dep"]));
assert!(remaining.is_none(), "without site-replication rules the whole config is deleted");
assert_eq!(removable, HashSet::from([user_arn.to_string()]));
}
fn replication_targets_with_arn(arns: &[&str]) -> BucketTargets {
BucketTargets {
targets: arns
@@ -3451,7 +3700,10 @@ mod tests {
let req = build_request(input, Method::DELETE);
let usecase = DefaultBucketUsecase::without_context();
let err = usecase.execute_delete_bucket_replication(req).await.unwrap_err();
let err = usecase
.execute_delete_bucket_replication(req, HashSet::new())
.await
.unwrap_err();
assert_eq!(err.code(), &S3ErrorCode::InternalError);
}
@@ -4537,7 +4789,7 @@ mod tests {
let req = build_request(input, Method::PUT);
let usecase = DefaultBucketUsecase::without_context();
let err = usecase.execute_put_bucket_replication(req).await.unwrap_err();
let err = usecase.execute_put_bucket_replication(req, HashSet::new()).await.unwrap_err();
assert_eq!(err.code(), &S3ErrorCode::InternalError);
}
@@ -4555,7 +4807,7 @@ mod tests {
.unwrap();
let err = DefaultBucketUsecase::without_context()
.execute_put_bucket_replication(build_request(input, Method::PUT))
.execute_put_bucket_replication(build_request(input, Method::PUT), HashSet::new())
.await
.expect_err("unsupported fields must be rejected before store access");
+2
View File
@@ -619,6 +619,8 @@ pub(crate) mod bucket {
use crate::storage::storage_api::ecstore_bucket::replication as replication_contracts;
pub(crate) use replication_contracts::merge_user_replication_config;
type ReplicationObjectBridge = crate::storage::storage_api::ecstore_bucket::replication::ReplicationObjectBridge;
pub(crate) type DeleteReplicationConfigSnapshot =
crate::storage::storage_api::ecstore_bucket::replication::DeleteReplicationConfigSnapshot;
+169 -2
View File
@@ -63,6 +63,69 @@ use crate::app::storage_api::object_usecase::bucket::replication::{
};
use crate::storage::storage_api::ecfs_consumer::StorageObjectOptions as ObjectOptions;
#[cfg(test)]
static SITE_REPLICATION_GATE_TEST_OVERRIDE: std::sync::atomic::AtomicU8 = std::sync::atomic::AtomicU8::new(0);
#[cfg(test)]
const SITE_REPLICATION_GATE_FORCE_DISABLED: u8 = 1;
#[cfg(test)]
const SITE_REPLICATION_GATE_FORCE_ENABLED: u8 = 2;
async fn site_replication_gate_enabled() -> S3Result<bool> {
#[cfg(test)]
match SITE_REPLICATION_GATE_TEST_OVERRIDE.load(std::sync::atomic::Ordering::SeqCst) {
SITE_REPLICATION_GATE_FORCE_DISABLED => return Ok(false),
SITE_REPLICATION_GATE_FORCE_ENABLED => return Ok(true),
_ => {}
}
crate::admin::handlers::site_replication::site_replication_enabled().await
}
/// Remote site-replication peer deployment ids handed to the bucket usecase
/// so an S3 replication-config edit keeps exactly the reconciler-owned rules
/// (issue #1948). Read here, in the interface layer, because the usecase must
/// not import the admin handlers (layer guard); a state-read failure
/// propagates so the edit fails closed.
async fn site_replication_peer_deployment_ids_for_edit() -> S3Result<std::collections::HashSet<String>> {
// While the gate override is in effect the test exercises the deny/allow
// branch, not the peer set; there is no persisted state to read.
#[cfg(test)]
if SITE_REPLICATION_GATE_TEST_OVERRIDE.load(std::sync::atomic::Ordering::SeqCst) != 0 {
return Ok(std::collections::HashSet::new());
}
crate::admin::handlers::site_replication::site_replication_remote_peer_deployment_ids().await
}
/// MinIO `ErrReplicationDenyEditError`.
fn replication_deny_edit_error() -> S3Error {
let mut err = S3Error::with_message(
S3ErrorCode::Custom("XMinioReplicationDenyEdit".into()),
"Sub-User is not allowed to edit Replication configuration",
);
err.set_status_code(StatusCode::BAD_REQUEST);
err
}
/// Site-replication gate for S3 replication-config edits (issue #1948).
///
/// On a site-replication deployment the bucket's replication config carries
/// the operator-managed `site-repl-*` rules that keep every peer in sync, and
/// a successful edit is broadcast to all peers — so a user holding only
/// bucket-scoped `s3:PutReplicationConfiguration` could rewrite or erase
/// replication net-wide. MinIO parity (`ErrReplicationDenyEditError`): only
/// owner credentials (root or root-parented) may edit. Runs after the policy
/// authorization in the access layer and only on the external S3 path — the
/// reconciler and peer bucket-meta ingestion never route through these
/// handlers.
async fn deny_replication_config_edit_for_non_owner<T>(req: &S3Request<T>) -> S3Result<()> {
if crate::storage::access::req_info_ref(req)?.is_owner {
return Ok(());
}
if site_replication_gate_enabled().await? {
return Err(replication_deny_edit_error());
}
Ok(())
}
#[derive(Debug, Clone)]
pub struct FS {
/// This server's late-bound application-context slot (backlog#1052 S2).
@@ -500,8 +563,10 @@ impl S3 for FS {
&self,
req: S3Request<DeleteBucketReplicationInput>,
) -> S3Result<S3Response<DeleteBucketReplicationOutput>> {
deny_replication_config_edit_for_non_owner(&req).await?;
let site_peers = site_replication_peer_deployment_ids_for_edit().await?;
let usecase = s3_api::bucket_usecase_for(self);
usecase.execute_delete_bucket_replication(req).await
usecase.execute_delete_bucket_replication(req, site_peers).await
}
#[instrument(level = "debug", skip(self))]
@@ -1353,8 +1418,10 @@ impl S3 for FS {
&self,
req: S3Request<PutBucketReplicationInput>,
) -> S3Result<S3Response<PutBucketReplicationOutput>> {
deny_replication_config_edit_for_non_owner(&req).await?;
let site_peers = site_replication_peer_deployment_ids_for_edit().await?;
let usecase = s3_api::bucket_usecase_for(self);
usecase.execute_put_bucket_replication(req).await
usecase.execute_put_bucket_replication(req, site_peers).await
}
async fn put_bucket_request_payment(
@@ -1919,3 +1986,103 @@ impl S3 for FS {
Box::pin(usecase.execute_upload_part_copy(req)).await
}
}
#[cfg(test)]
mod tests {
use super::{
FS, SITE_REPLICATION_GATE_FORCE_DISABLED, SITE_REPLICATION_GATE_FORCE_ENABLED, SITE_REPLICATION_GATE_TEST_OVERRIDE,
};
use crate::storage::access::ReqInfo;
use http::Method;
use http::StatusCode;
use s3s::dto::{DeleteBucketReplicationInput, PutBucketReplicationInput, ReplicationConfiguration};
use s3s::{S3, S3Error, S3ErrorCode, S3Request};
use std::sync::atomic::Ordering;
fn replication_config_edit_request<T>(input: T, is_owner: bool) -> S3Request<T> {
let mut req = S3Request {
input,
method: Method::PUT,
uri: http::Uri::from_static("/"),
headers: http::HeaderMap::new(),
extensions: http::Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
req.extensions.insert(ReqInfo {
is_owner,
..Default::default()
});
req
}
fn put_bucket_replication_input() -> PutBucketReplicationInput {
PutBucketReplicationInput {
bucket: "test-bucket".to_string(),
checksum_algorithm: None,
content_md5: None,
expected_bucket_owner: None,
replication_configuration: ReplicationConfiguration {
role: String::new(),
rules: Vec::new(),
},
token: None,
}
}
fn delete_bucket_replication_input() -> DeleteBucketReplicationInput {
DeleteBucketReplicationInput {
bucket: "test-bucket".to_string(),
expected_bucket_owner: None,
}
}
fn assert_replication_deny_edit(err: &S3Error) {
match err.code() {
S3ErrorCode::Custom(code) => assert_eq!(code, "XMinioReplicationDenyEdit"),
other => panic!("expected XMinioReplicationDenyEdit, got {other:?}"),
}
assert_eq!(err.status_code(), Some(StatusCode::BAD_REQUEST));
}
/// Single test on purpose: the branches share the process-wide gate
/// override, and parallel tests would race it.
#[tokio::test]
async fn replication_config_edit_gate_denies_only_non_owner_under_site_replication() {
let fs = FS::new();
SITE_REPLICATION_GATE_TEST_OVERRIDE.store(SITE_REPLICATION_GATE_FORCE_ENABLED, Ordering::SeqCst);
// Non-owner PUT/DELETE through the real S3 handlers: denied by the
// gate before the usecase (and thus the store) is ever touched.
let err = fs
.put_bucket_replication(replication_config_edit_request(put_bucket_replication_input(), false))
.await
.expect_err("non-owner PutBucketReplication must be denied while site replication is enabled");
assert_replication_deny_edit(&err);
let err = fs
.delete_bucket_replication(replication_config_edit_request(delete_bucket_replication_input(), false))
.await
.expect_err("non-owner DeleteBucketReplication must be denied while site replication is enabled");
assert_replication_deny_edit(&err);
// Owner passes the gate (the usecase's empty-rules structure error
// proves the request reached the usecase instead of the deny path).
let err = fs
.put_bucket_replication(replication_config_edit_request(put_bucket_replication_input(), true))
.await
.expect_err("owner request should pass the gate and fail later on config validation");
assert_eq!(err.code(), &S3ErrorCode::InvalidRequest);
// Without site replication the policy check alone still governs the edit.
SITE_REPLICATION_GATE_TEST_OVERRIDE.store(SITE_REPLICATION_GATE_FORCE_DISABLED, Ordering::SeqCst);
let err = fs
.put_bucket_replication(replication_config_edit_request(put_bucket_replication_input(), false))
.await
.expect_err("non-owner request should pass the gate and fail later on config validation");
assert_eq!(err.code(), &S3ErrorCode::InvalidRequest);
SITE_REPLICATION_GATE_TEST_OVERRIDE.store(0, Ordering::SeqCst);
}
}
-2
View File
@@ -501,8 +501,6 @@ pub(crate) mod ecstore_notification {
#[allow(unused_imports)]
pub(crate) mod ecstore_rebalance {
#[cfg(test)]
pub(crate) use rustfs_ecstore::api::rebalance::test_util;
pub(crate) use rustfs_ecstore::api::rebalance::{
DiskStat, RebalSaveOpt, RebalStatus, RebalanceCleanupWarningEntry, RebalanceCleanupWarnings, RebalanceInfo,
RebalanceMeta, RebalanceStats, RebalanceStopPropagationRecord, decode_rebalance_stop_propagation_record,
+1 -1
View File
@@ -241,7 +241,7 @@ env \
RUSTFS_TEST_VAULT_FAILOVER_MARKER="$MARKER" \
RUSTFS_TEST_VAULT_OLD_LEADER="$OLD_LEADER" \
cargo test -p rustfs-kms --test vault_ha_failover_live \
vault_raft_leader_failure_recovers_kv2_and_transit_decrypts -- \
vault_raft_leader_failure_preserves_kv2_and_transit_decrypts -- \
--ignored --nocapture --test-threads=1 &
TEST_PID=$!