Files
rustfs/scripts/check_s3s_footprint.sh
T
唐小鸭 46907c05cf fix(replication): close the GA blocker set from backlog#2366 (#7503)
* fix(replication): close GA blockers from backlog#2366

Implements the P1 set from the pre-GA replication audit:

- Replication rule tag filters now require every And.Tag to match, replacing
  the s3s OR semantics with a local AND matcher that fails closed on a
  malformed tag.
- A replicated group membership change no longer writes the group status, so
  a membership update carrying the default Enabled status cannot silently
  re-enable a disabled group on the peer.
- A successful IAM import schedules one collapsed full-IAM snapshot per remote
  peer instead of leaving the imported entities local-only.
- A pending endpoint refresh is redriven by the heavyweight reconcile tick,
  carries its own ilm-expiry override, and no longer blocks a remove that
  drops every unacknowledged peer.
- Site metrics expose local replication failure totals and rolling windows;
  node-level counters no longer report a constructed zero.
- set/remove-remote-target notify peer metadata caches before returning, so a
  follow-up put-bucket-replication on another node sees the target.
- Adds the site-replication operations runbook, a docs index, a replication
  support boundary section, and the Replication changelog section.

* fix(site-replication): resume only a locally driven endpoint refresh

The peer-side edit handler journals a pending endpoint refresh with an empty
`remote_peers` map and commits it inside the same request through
`apply_internal_peer_edit`. The reconcile tick could not tell that journal
from the coordinator's own: with no required peers it reads as complete on
sight, so the tick committed it with `edit_state` - losing the local-name
sync - and cleared it under the request that owned it, whose commit then
reported the refresh as changed and denied the coordinator the peer
acknowledgement it was waiting for.

Resume now runs only for a journal that carries the fan-out topology. A
receiver's journal stays for the coordinator to redrive with the same
refresh id, which is the path that already recovers it.

* fix(site-replication): keep an explicit disabled group status on a snapshot

Skipping the group-status write whenever an item carries members stopped a
membership change from re-enabling a disabled group, but it also silenced the
full-IAM snapshot, which always sends members together with the sender's real
status. A peer that did not have the group yet created it through
`GroupInfo::new` - enabled - so a bootstrap, a repair, or the snapshot an IAM
import now schedules handed every member of a frozen group live access there.

The madmin wire maps an unset `groupStatus` to Enabled, so only Enabled can be
a default. Disabled is always explicit and is applied again.

* fix(site-replication): schedule the import snapshot without recording a failure

`import-iam` reused the failure-recording path to queue its full-IAM
snapshot. That raises `retry_count` on every call, so three imports - the
normal shape of a bulk migration done one archive at a time - escalated a
healthy peer to `retryStats.failed` with the scheduling note shown as
`lastError`, which is exactly the signal the runbook tells operators to
repair. A full retry queue also turned a completed import into a 503.

Scheduling now only ensures the collapsed entry exists, and a failure to
schedule is logged instead of failing the request: the entities are already
imported and the reconcile pass still closes the gap.

* fix(admin): stop reporting replication failures as retries

`retries` is the minio-go counter for redeliveries, and mc prints it as such.
Filling it with the failure count claimed a redelivery that never happens: a
failed object is not retried by an event today, it waits for the scanner heal
pass. `errors` keeps the failure counters; `retries` stays zero until there is
a real redelivery to count, and the runbook now says so.

* perf(site-replication): aggregate failure windows without cloning bucket stats

`site_metrics_snapshot` went through `get_all`, which clones every bucket's
stats, and then scanned each target's sample deque twice. That deque is
bounded only by the one-hour window, so an unreachable target under load -
the case an operator polls this endpoint for - made every
`mc admin replicate status` copy the whole backlog and hold the read lock
against the failure path while doing it.

It now folds under the read lock and takes both windows in one walk. The
`max` against the serialized `last_minute` / `last_hour` snapshots is dropped:
those are stamped onto per-bucket clones elsewhere and are always zero in this
node-local cache.

* fix(site-replication): reject a conflicting ilm-expiry override on a re-run

The commit now reads the ilm-expiry override back out of the pending refresh
journal, so a second edit that asks for a different value had it dropped while
the request still reported success. Re-running without the flag keeps pinning
the recorded value - that is the documented way to redrive a stuck refresh -
but an explicit different value is now rejected instead of ignored.

* fix(admin): do not fail a remote-target write on a peer reload error

set/remove-remote-target propagated the peer metadata reload error, so a
target that was already persisted and live on this node reported a 5xx to the
client whenever one peer could not be reached. Every S3 bucket-config write
path treats that reload as best effort and only warns; these two admin
handlers now do the same, and the reason is logged with the bucket and action.

* fix(site-replication): undo every bucket a cut-short refresh rewrote

When a remove accepted on another node clears the refresh journal mid-pass,
only the bucket holding the lock at that moment had its restored target
undone. The buckets rewritten earlier in the same pass kept a target pointing
at the removed peer whenever the remove's own cleanup had already walked past
them. The undo now covers every bucket this pass rewrote, attempting all of
them so one failure does not strand the rest.

* fix(site-replication): keep replay running while an endpoint refresh is pending

A pending endpoint refresh took the whole heavyweight pass with it, so a peer
that never came back froze IAM and bucket replay to every healthy peer too -
the stall this journal's resume path was meant to end. The refresh arm now
drains the retry queue before returning; it replays per-peer deliveries
against the endpoints currently committed in state, so it is unaffected by the
edit in flight. Bucket wiring reconciliation still waits, because it rewrites
the very targets the refresh is changing, and the runbook now says so.

* test(e2e): cover the AND semantics of a two-tag replication filter

The acceptance matrix only had a single-tag rule, which matches under both AND
and OR semantics and therefore proved nothing about the filter this fix
changed. It now also carries a two-tag `And` rule - the shape
`mc replicate add --tags "k1=v1&k2=v2"` writes - and asserts that an object
with one of the two tags is not admitted while an object with both is.

No new test function, so the nightly selection digest is unchanged.

* refactor(site-replication): fold the refresh state-change error into one constructor

The endpoint-refresh work added three `s3_error!` invocation lines, which the
s3s footprint ratchet is meant to prevent. Five copies of the same
concurrent-change error now share one constructor, so the surface nets one
line smaller than main; the baseline is retightened to match.

* fix(site-replication): report a peer whose IAM snapshot waits for a repair

An escalated snapshot entry records a deletion a snapshot cannot replay, so
only a repair settles it and the marker must survive. Scheduling an import
snapshot therefore leaves that peer's entry alone - and now says so, instead
of returning success while nothing was scheduled for it.

* docs(operations): state the group-status and escalation convergence limits

Two boundaries the fixes in this branch make load-bearing: a membership change
never carries an enable, so a group disabled on one site only has to be
re-enabled there explicitly; and a peer holding an escalated IAM entry does
not receive a scheduled snapshot, including the one a bulk import schedules,
until a repair settles it.
2026-09-08 14:58:41 +00:00

157 lines
7.6 KiB
Bash
Executable File
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
#!/usr/bin/env bash
# Ratchet guard freezing the s3s dependency footprint ahead of the
# s3gate/gateway migration (rustfs/backlog#1677, review finding F1;
# acceptance criteria recorded in rustfs/backlog#1733).
#
# The migration's goal is to shrink the direct s3s surface, so new code must
# not grow it. Two counters are ratcheted (verification dates live next to the
# baseline values below):
#
# - files referencing s3s paths: rg -l "$S3S_PATH_PATTERN" --type rust . (files)
# - s3_error! invocation lines: rg -c 's3_error!' --type rust . (summed)
#
# Every rg invocation MUST pass an explicit path ('.' for repo-wide): without
# one, rg searches stdin instead of the tree whenever stdin is a readable
# pipe — which is exactly what GitHub Actions attaches to run steps — and
# silently counts 0 (observed on run 32978746357, where both repo-wide
# counters read 0 and were waved through as "shrank"). The sanity assertions
# below fail hard if that ever regresses.
#
# Either count exceeding its baseline fails the check with the offending
# delta. Baselines are LOWER-ONLY: when a PR shrinks the footprint, lower the
# matching baseline in the same PR so the ratchet stays tight. Never raise a
# baseline to get green (AGENTS.md, Verification Before PR) — route new S3
# API code through the gateway abstractions instead of importing s3s
# directly.
#
# Usage: scripts/check_s3s_footprint.sh
set -euo pipefail
cd "$(dirname "$0")/.."
# Baselines verified on 2026-08-26. Lower-only; see header.
# Excludes crates/e2e_test/ — test infrastructure legitimately uses s3s
# to verify S3 behavior and does not widen the production s3s surface.
# 208 → 215 on 2026-08-26: PR #6670 mechanically split
# rustfs/src/app/object_usecase.rs into 8 per-operation modules (net +7
# files, zero new s3s code — the same handler-layer surface redistributed).
# The file counter is split-sensitive; the s3_error! line counter confirms
# no growth (unchanged at 1620).
# 1620 → 1616 on 2026-08-27: backlog#1840 moved the site-replication service
# subsystem to rustfs/src/site_replication/ (s3s access funneled through the
# root storage facade's s3 shim, keeping the file count at 215). The move
# inlined one s3_error! call in transport.rs (+1); measured 1615 on the
# pre-move main (after #6694) and 1616 after, so the slack 1620 baseline is
# retightened to the measured 1616.
# 215 → 213 on 2026-08-28: multipart foreground admission cleanup moved the
# new usecase dependency behind the app object-domain facade while main had
# already shed two direct s3s-importing files. Retighten the file counter only;
# s3_error! stays flat at 1616.
# 1616 → 1613 on 2026-09-02: dependency refresh verified the current tree has
# already shed three s3_error! invocation lines; retighten the line counter.
# 1613 → 1589 on 2026-09-06: rustfs/backlog#2309 and rustfs/rustfs#7225 both
# folded the ten per-config arms of ExportBucketMetadata into one helper, which
# now reports an unreadable configuration as a plain string instead of raising
# an S3 error per arm (24 invocation lines removed from
# rustfs/src/admin/handlers/bucket_meta.rs; measured after merging the two).
# 1589 -> 1588 on 2026-09-08: the GA blocker set (rustfs/backlog#2366) added
# three invocation lines to the endpoint-refresh paths and folded the five
# copies of the concurrent-change error into one constructor, netting -1.
S3S_IMPORT_FILES_BASELINE=213
S3_ERROR_LINES_BASELINE=1588
# ecstore-scoped ratchet (rustfs/backlog#1842): the storage engine must not
# know S3 wire/DTO types (ARCHITECTURE.md invariant 4). The S3-*consuming*
# client was extracted to crates/s3-client, where s3s usage is legitimate;
# this counter ratchets the remaining serving-side s3s references out of
# crates/ecstore. Baseline verified on 2026-08-26.
S3S_ECSTORE_FILES_BASELINE=39
S3S_PATH_PATTERN='(^|[^"[:alnum:]_])s3s::'
E2E_TEST_GLOB='--glob=!crates/e2e_test/**'
TMP_DIR="$(mktemp -d)"
trap 'rm -rf "$TMP_DIR"' EXIT
# rg exits 1 on zero matches (a legitimate count of 0 at the end of the
# migration) and >1 on real errors; only the latter may abort the check.
run_rg_to() {
local out="$1" rg_status=0
shift
rg "$@" >"$out" || rg_status=$?
if ((rg_status > 1)); then
echo "error: 'rg $*' failed with status $rg_status" >&2
exit 1
fi
}
# Explicit '.' path is load-bearing — see header. Never drop it.
run_rg_to "$TMP_DIR/import_files" -l "$S3S_PATH_PATTERN" --type rust "$E2E_TEST_GLOB" .
run_rg_to "$TMP_DIR/error_lines" -c 's3_error!' --type rust "$E2E_TEST_GLOB" .
run_rg_to "$TMP_DIR/ecstore_files" -l "$S3S_PATH_PATTERN" --type rust crates/ecstore/src
s3s_import_files="$(grep -c . "$TMP_DIR/import_files" || true)"
s3_error_lines="$(awk -F: '{sum += $NF} END {print sum + 0}' "$TMP_DIR/error_lines")"
s3s_ecstore_files="$(grep -c . "$TMP_DIR/ecstore_files" || true)"
for value in "$s3s_import_files" "$s3_error_lines" "$s3s_ecstore_files"; do
if ! [[ "$value" =~ ^[0-9]+$ ]]; then
echo "error: could not compute s3s footprint counts (got: '$value')" >&2
exit 1
fi
done
# Sanity assertions: a counter reading 0 while its baseline is positive, or
# the repo-wide file count dropping below the ecstore-scoped one (a strict
# subset of it), means the counter itself broke — most likely rg searching
# stdin instead of the tree (see header) — not that the footprint shrank.
# Fail hard rather than waving the ratchet through. If the footprint ever
# genuinely reaches zero, lower the baseline to 0 in the same PR.
sanity_nonzero() {
local label="$1" count="$2" baseline="$3"
if ((count == 0 && baseline > 0)); then
echo "error: $label counted 0 with a baseline of $baseline — the counter is" >&2
echo " broken (rg likely searched stdin; every rg call needs an explicit path)." >&2
exit 1
fi
}
sanity_nonzero "files importing s3s" "$s3s_import_files" "$S3S_IMPORT_FILES_BASELINE"
sanity_nonzero "s3_error! invocation lines" "$s3_error_lines" "$S3_ERROR_LINES_BASELINE"
if ((s3s_import_files < s3s_ecstore_files)); then
echo "error: repo-wide s3s file count ($s3s_import_files) is below the ecstore-scoped" >&2
echo " count ($s3s_ecstore_files); the repo-wide counter is broken (see header)." >&2
exit 1
fi
status=0
check_ratchet() {
local label="$1" count="$2" baseline="$3" inspect_cmd="$4"
if ((count > baseline)); then
echo "❌ s3s footprint ratchet violation: $label is $count, baseline is $baseline (+$((count - baseline)))" >&2
echo " New code must not widen the s3s surface being removed by the s3gate migration" >&2
echo " (rustfs/backlog#1677 F1, rustfs/backlog#1733). Use the gateway abstractions" >&2
echo " instead of importing s3s directly. To find the offenders, compare" >&2
echo " '$inspect_cmd' against origin/main." >&2
status=1
elif ((count < baseline)); then
echo "️ s3s footprint shrank: $label is $count, baseline is $baseline ($((count - baseline)))." >&2
echo " Lower the baseline in scripts/check_s3s_footprint.sh in this PR to keep the ratchet tight." >&2
else
echo "s3s footprint OK: $label is $count (baseline: $baseline)"
fi
}
check_ratchet "files importing s3s" "$s3s_import_files" "$S3S_IMPORT_FILES_BASELINE" \
"rg -l '$S3S_PATH_PATTERN' --type rust $E2E_TEST_GLOB ."
check_ratchet "s3_error! invocation lines" "$s3_error_lines" "$S3_ERROR_LINES_BASELINE" \
"rg -c 's3_error!' --type rust $E2E_TEST_GLOB ."
check_ratchet "ecstore files referencing s3s" "$s3s_ecstore_files" "$S3S_ECSTORE_FILES_BASELINE" \
"rg -l '$S3S_PATH_PATTERN' --type rust crates/ecstore/src"
if ((status != 0)); then
exit 1
fi
echo "✅ s3s footprint ratchet check passed"