mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-16 18:08:21 +00:00
fix(site-replication): merge incoming ILM expiry documents instead of overwriting (#6130)
* test(site-replication): pin ILM expiry merge contract for incoming lc-config Red-light evidence for backlog#1675 P1-1: the lc-config receiver overwrites the whole local lifecycle config with whatever the peer sends (and deletes it wholesale on peer delete), so an expiry-only document erases the receiver's local tier/transition rules, and peer transition rules get installed across sites. The new tests pin the MinIO mergeWithCurrentLCConfig semantics plus RustFS hardening: - incoming expiry documents merge with (never replace) local rules - local transition sides are authoritative for same-id rules - incoming transition fields are discarded at the trust boundary - dropped expiry rules strip the expiry side but keep transitions; pure-expiry rules are removed - delete merges with the empty set instead of dropping the config - disabled rules survive; abort-mpu-only rules stay site-local - deterministic order (idempotent re-delivery) and expiry_updated_at stamping for the staleness axis All fail against the current overwrite implementation (identity extraction of merge_incoming_lifecycle_config). * fix(site-replication): merge incoming ILM expiry documents instead of overwriting The lc-config receiver replaced the whole local lifecycle config with the peer's document (and deleted it wholesale on peer delete), so an expiry-only update erased the receiver's local tier/transition rules, and a peer's transition rules were installed across sites (backlog#1675 P1-1). Receiver (apply_bucket_meta_item): - lc-config now merges via merge_incoming_lifecycle_config, mirroring MinIO's mergeWithCurrentLCConfig with a trust-boundary hardening: incoming transition fields are discarded outright; the local transition side of a same-id rule is authoritative. A peer delete merges with the empty set — pure-expiry rules go away, transition rules survive with their expiry side cleared, and only an empty result deletes the config file. - Staleness moves to the expiry axis (config.expiry_updated_at): lifecycle_config_updated_at also moves on local transition-only edits, which shadowed newer peer expiry updates. - Receiver-side replicateILMExpiry gate, symmetric with the sender hook (previously any peer could install expiry rules while the option was off). - Rule order is deterministic (local order, incoming-new appended), so re-delivering the same document is byte-stable and does not rewrite bucket metadata per broadcast. Sender: - Both admin choke points — the bucket-meta hook and the SRInfo bucket entry feeding bootstrap/repair and consistency views — now emit only the expiry subset (transition fields stripped, non-expiry rules dropped). MinIO receivers install incoming rules verbatim, so transition rules must never leave the site. An unparseable local config is forwarded unfiltered rather than degraded to a delete. Not covered here (follow-up): a two-site e2e with a real tier backend to exercise transition-rule preservation end to end; receiver-side validate_transition_tier for merged configs. * fix(site-replication): close ILM merge review findings Adversarial review of the lc-config merge surfaced four real defects, all fixed here: - Deletion tombstone regression: with the staleness axis moved to the in-config expiry_updated_at, a deleted lifecycle config fell back to UNIX_EPOCH and any delayed stale broadcast could resurrect deleted expiry rules. The axis now falls back to the whole-config write time (which survives deletion in bucket metadata as the deletion's lower bound), also covering legacy configs that predate the axis field. - MinIO zero-rule documents: MinIO's delete tombstone / transition-only state marshals a lifecycle document with no <Rule>, which the strict s3s deserializer rejects — the receiver now recognizes it as the 'no expiry rules here' statement (delete semantics) instead of erroring on every MinIO heal pass. - Inflated expiry axis at the sender: PutBucketLifecycle stamped expiry_updated_at unconditionally, so a transition-only edit advanced the axis and let this site's stale expiry subset shadow and roll back newer peer expiry edits fleet-wide. The stamp is now conditional (expiry subset present before or after the edit, MinIO parity), the hook item travels with the config's expiry axis (UNIX_EPOCH when the site has none), and the SRInfo bucket entry feeds bootstrap/repair the same axis instead of the whole-config write time. - Del-marker parity: MinIO's CloneNonTransition never emits del-marker or abort-mpu fields, so treating del_marker_expiration as traveling expiry let a MinIO broadcast delete this site's del-marker-only rules. Both fields are now site-local on every edge: stripped from outbound subsets and inbound rules, restored from the local side on same-id merges, and never a deletion criterion. Receiver-side validation of merged configs (object-lock / tier constraints, MinIO runs finalLcCfg.Validate) remains a follow-up. * fix(site-replication): close the second ILM review round - Missed-delete repair: a deleted expiry state now travels through bootstrap/repair as an explicit timestamped lc-config delete item (lifecycle_expiry_statement distinguishes deletion — whole-config write time advanced past the created backfill — from never-configured buckets and from transition-only configs without an expiry axis, which say nothing). A peer that missed the live delete converges on repair; the receiver's staleness guard protects newer peer state. - Strict tombstone recognition: only a well-delimited zero-rule <LifecycleConfiguration> document maps to delete semantics; truncated or foreign payloads that fail the strict deserializer are rejected instead of being treated as a delete that erases local expiry rules. - Staleness fallback axis narrowed: the whole-config write time is used only for deleted or legacy-with-expiry state. A present transition-only config without an expiry axis compares at epoch — its whole-config time moves on transition edits and must not shadow or block independent peer expiry updates and same-timestamp repairs. * fix(site-replication): validate tombstone children structurally Second review round: a well-delimited root could still smuggle malformed content — e.g. <LifecycleConfiguration><ExpiryUpdatedAt> </LifecycleConfiguration> passed the no-<Rule check and was applied as a delete. The tombstone body must now be a sequence of well-formed simple children (matching open/close or self-closing, no nested markup, no stray text, none named Rule); anything else surfaces InvalidRequest. Malformed-child cases pinned in the recognition test. * fix(site-replication): serialize lifecycle merges --------- Co-authored-by: overtrue <anzhengchao@gmail.com>
This commit is contained in:
@@ -135,7 +135,8 @@ pub mod bucket {
|
||||
pub use crate::bucket::metadata_sys::ConfigWriteLockProbe;
|
||||
pub use crate::bucket::metadata_sys::{
|
||||
BucketMetadataMutationGuard, BucketMetadataSys, ObjectLockConfigState, acquire_bucket_metadata_transaction_lock,
|
||||
capture_bucket_metadata_incarnation, delete, delete_if_incarnation, get, get_accelerate_config, get_bucket_policy,
|
||||
acquire_bucket_metadata_transaction_lock_for_incarnation, capture_bucket_metadata_incarnation, delete,
|
||||
delete_if_incarnation, delete_under_transaction_lock, get, get_accelerate_config, get_bucket_policy,
|
||||
get_bucket_policy_raw, get_bucket_targets_config, get_config_from_disk, get_cors_config, get_durability_config,
|
||||
get_global_bucket_metadata_sys, get_lifecycle_config, get_logging_config, get_notification_config,
|
||||
get_object_lock_config, get_object_lock_config_state, get_public_access_block_config, get_quota_config,
|
||||
|
||||
@@ -656,6 +656,16 @@ pub async fn update_under_transaction_lock(
|
||||
update_under_config_write_guard(get_bucket_metadata_sys()?, guard, config_file, data).await
|
||||
}
|
||||
|
||||
/// Clear one config file while the caller holds this bucket's transaction lock.
|
||||
pub async fn delete_under_transaction_lock(
|
||||
guard: &BucketMetadataMutationGuard,
|
||||
bucket: &str,
|
||||
config_file: &str,
|
||||
) -> Result<OffsetDateTime> {
|
||||
guard.ensure_valid(bucket)?;
|
||||
delete_under_config_write_guard(get_bucket_metadata_sys()?, guard, config_file).await
|
||||
}
|
||||
|
||||
pub async fn update_quota_if_incarnation(
|
||||
bucket: &str,
|
||||
data: Vec<u8>,
|
||||
@@ -795,6 +805,14 @@ pub async fn acquire_bucket_metadata_transaction_lock(bucket: &str) -> Result<Bu
|
||||
acquire_config_write_guard(get_bucket_metadata_sys()?, bucket).await
|
||||
}
|
||||
|
||||
/// Acquire the bucket transaction lock only if its incarnation still matches.
|
||||
pub async fn acquire_bucket_metadata_transaction_lock_for_incarnation(
|
||||
bucket: &str,
|
||||
expected_incarnation_id: Uuid,
|
||||
) -> Result<BucketMetadataMutationGuard> {
|
||||
acquire_config_write_guard_for_incarnation(get_bucket_metadata_sys()?, bucket, Some(expected_incarnation_id)).await
|
||||
}
|
||||
|
||||
pub(crate) async fn acquire_bucket_metadata_transaction_lock_in(
|
||||
ctx: &crate::runtime::instance::InstanceContext,
|
||||
bucket: &str,
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -321,6 +321,34 @@ pub(crate) mod metadata_sys {
|
||||
crate::storage::storage_api::acquire_bucket_metadata_transaction_lock(bucket).await
|
||||
}
|
||||
|
||||
pub(crate) async fn acquire_bucket_metadata_transaction_lock_for_incarnation(
|
||||
bucket: &str,
|
||||
expected_incarnation_id: uuid::Uuid,
|
||||
) -> Result<super::ecstore_bucket::metadata_sys::BucketMetadataMutationGuard> {
|
||||
super::ecstore_bucket::metadata_sys::acquire_bucket_metadata_transaction_lock_for_incarnation(
|
||||
bucket,
|
||||
expected_incarnation_id,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn update_under_transaction_lock(
|
||||
guard: &super::ecstore_bucket::metadata_sys::BucketMetadataMutationGuard,
|
||||
bucket: &str,
|
||||
config_file: &str,
|
||||
data: Vec<u8>,
|
||||
) -> Result<OffsetDateTime> {
|
||||
super::ecstore_bucket::metadata_sys::update_under_transaction_lock(guard, bucket, config_file, data).await
|
||||
}
|
||||
|
||||
pub(crate) async fn delete_under_transaction_lock(
|
||||
guard: &super::ecstore_bucket::metadata_sys::BucketMetadataMutationGuard,
|
||||
bucket: &str,
|
||||
config_file: &str,
|
||||
) -> Result<OffsetDateTime> {
|
||||
super::ecstore_bucket::metadata_sys::delete_under_transaction_lock(guard, bucket, config_file).await
|
||||
}
|
||||
|
||||
pub(crate) async fn update_bucket_targets_under_transaction_lock(
|
||||
guard: &super::ecstore_bucket::metadata_sys::BucketMetadataMutationGuard,
|
||||
bucket: &str,
|
||||
|
||||
@@ -1158,6 +1158,19 @@ fn lifecycle_has_expiry_rules(config: &BucketLifecycleConfiguration) -> bool {
|
||||
})
|
||||
}
|
||||
|
||||
/// Status-independent presence of the expiry subset that site replication
|
||||
/// propagates (`replicateILMExpiry`): expiration / noncurrent-version
|
||||
/// expiration only. Distinct from [`lifecycle_has_expiry_rules`], which
|
||||
/// filters on ENABLED for scanner scheduling — editing a Disabled expiry rule
|
||||
/// must still advance the replication axis. Del-marker expiration and
|
||||
/// abort-multipart are site-local and never travel.
|
||||
fn lifecycle_rules_have_expiry(config: &BucketLifecycleConfiguration) -> bool {
|
||||
config
|
||||
.rules
|
||||
.iter()
|
||||
.any(|rule| rule.expiration.is_some() || rule.noncurrent_version_expiration.is_some())
|
||||
}
|
||||
|
||||
fn lifecycle_has_abort_multipart_rules(config: &BucketLifecycleConfiguration) -> bool {
|
||||
config.rules.iter().any(|rule| {
|
||||
rule.status == ExpirationStatus::from_static(ExpirationStatus::ENABLED)
|
||||
@@ -2186,7 +2199,24 @@ impl DefaultBucketUsecase {
|
||||
return Err(s3_error!(InvalidArgument, "{err}"));
|
||||
}
|
||||
|
||||
input_cfg.expiry_updated_at = Some(Timestamp::from(time::OffsetDateTime::now_utc()));
|
||||
// Stamp the expiry axis only when the expiry subset can have changed
|
||||
// (MinIO: HasExpiry() || expiryRuleRemoved). Site-replication peers
|
||||
// judge lc-config staleness on this axis; a transition-only edit that
|
||||
// advanced it would let this site's stale expiry subset shadow — and
|
||||
// roll back — a newer peer expiry edit fleet-wide.
|
||||
let previous_expiry_updated_at = match metadata_sys::get_lifecycle_config(&bucket).await {
|
||||
Ok((previous, _)) => {
|
||||
if lifecycle_rules_have_expiry(&input_cfg) || lifecycle_rules_have_expiry(&previous) {
|
||||
Some(Timestamp::from(time::OffsetDateTime::now_utc()))
|
||||
} else {
|
||||
previous.expiry_updated_at
|
||||
}
|
||||
}
|
||||
// No previous config (or unreadable): stamping is the
|
||||
// conservative pre-existing behavior.
|
||||
Err(_) => lifecycle_rules_have_expiry(&input_cfg).then(|| Timestamp::from(time::OffsetDateTime::now_utc())),
|
||||
};
|
||||
input_cfg.expiry_updated_at = previous_expiry_updated_at;
|
||||
let data = serialize_config(&input_cfg)?;
|
||||
update_bucket_config_for_incarnation(&bucket, BUCKET_LIFECYCLE_CONFIG, data, expected_incarnation_id)
|
||||
.await
|
||||
@@ -2197,7 +2227,14 @@ impl DefaultBucketUsecase {
|
||||
let mut item = sr_bucket_meta_item(bucket.clone(), "lc-config");
|
||||
item.expiry_lc_config =
|
||||
Some(serialize_config(&input_cfg).and_then(|bytes| String::from_utf8(bytes).map_err(to_internal_error))?);
|
||||
item.expiry_updated_at = item.updated_at;
|
||||
// The item travels with the expiry axis, not the wall clock: a site
|
||||
// whose expiry knowledge is old (or absent — UNIX_EPOCH) must not
|
||||
// out-rank newer peer expiry state at the receivers.
|
||||
item.expiry_updated_at = input_cfg
|
||||
.expiry_updated_at
|
||||
.clone()
|
||||
.map(time::OffsetDateTime::from)
|
||||
.or(Some(time::OffsetDateTime::UNIX_EPOCH));
|
||||
if let Err(err) = site_replication_bucket_meta_hook(item).await {
|
||||
warn!(bucket = %bucket, error = ?err, "site replication bucket lifecycle hook failed");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user