* fix(volume): keep preallocated growth above -volume.minFreeSpace
When free space fell below -volume.minFreeSpace, every volume went
read-only and the master kept growing new volumes. With preallocation on,
each new volume reserved its full size, driving the disk further below the
floor (15 Oct incident: 66 empty volumes reserved about 1980 GiB).
Two causes, both in weed/storage/store.go:
- MaybeAdjustVolumeMax sized a disk's volume slots from raw free space, so
space inside the floor was counted as available. It now counts only free
space above the floor.
- addVolume trusted the once-a-minute low-disk flag. A preallocated volume
is now refused when the disk is low now, or when its reservation would
not fit above the floor. Volumes without preallocation, including merges
that copy existing data, are not affected.
addVolume runs under a store-level lock, so two concurrent creates cannot
both pass the room check against the same free space.
* volume: mirror headroom guard in the rust store
find_free_location only trusted the once-a-probe is_disk_space_low flag,
so a burst of preallocated creates could still run the disk under
-volume.minFreeSpace between probes, and maybe_adjust_volume_max sized
slots from raw free space. Adds MinFreeSpace::is_low/available_space to
match Go and applies both checks the same way. store.write() already
serializes add_volume, so no extra lock is needed.
* volume: read rust headroom tests from a fake disk
The floor arithmetic used a live free-space reading, so unrelated disk
activity could flip a test between pass and fail. Store.disk_stats_of is
now the same seam Go uses (diskStatusOf): the growth-loop and floor tests
run against a controlled disk, mirroring the Go suite one for one.
* volume: hold back create metadata from the headroom check
An exact-fit preallocation left free space exactly at the floor, so the
.vif and .idx writes that follow a create could dip under it and get the
location flagged read-only at the next probe. Reserve room for .vif on
the data filesystem, and for .idx growth only when dir_idx shares it.
* volume: mirror the metadata allowance in the rust store
Same rule as the Go side: charge .vif against the checked filesystem and
.idx only when dir_idx shares it, and grow the exact-fit test's fake
disk by the per-create cost.
* volume: skip the reservation check where create cannot preallocate
* volume: mirror the platform preallocation gate in the rust store
* volume.copy: count the preallocation reservation in the destination space check
The copy path picked a location with room for the copied .dat, then reserved
the master's full volume size limit on it. A volume smaller than the limit
could land on a disk whose free space above -volume.minFreeSpace fit the
copy but not the reservation, driving the disk under the floor the same way
the growth loop did.
Fetch the preallocation setting before choosing the location and require
room for the reservation itself, only where CreateVolumeFile actually
reserves blocks.
* seaweed-volume: store min-free-space-adjusted available space like Go
Go's DiskLocation.AvailableSpace holds free space above the
-volume.minFreeSpace floor; the Rust probe cached raw free instead, so the
VolumeCopy location check could accept a disk already inside the floor.
Store the same adjusted figure and cover it with a test.
* seaweed-volume: reserve the copied .dat like Go's CreateVolumeFile
VolumeCopy called set_len for the reservation, which only grows a sparse
file — no blocks are held, so a disk with room for the copied bytes but
not the full reservation only fails mid-copy. preallocate_file fallocates
KEEP_SIZE on Linux like Go, reserving the blocks up front; off Linux it
is a no-op, matching PREALLOCATE_SUPPORTED.
* seaweed-volume: charge a copy's destination pick for the .dat reservation
Go's VolumeCopy now sizes its free-location check against the master's
volume size limit when the platform preallocates, not just the copied
bytes. The Rust pick only measured the copied bytes, so a destination
could accept a copy whose reservation then dips under the floor. The
config fetch moves ahead of the pick (same order as Go) and feeds both
the space check and the reservation itself.
* volume.copy: charge the picked location's free space under volumeCreateLock
The copy path picked a destination off the once-a-minute AvailableSpace
probe and released the decision long before the .dat reservation landed.
Two copies could pass the same cached headroom and together drive the disk
under -volume.minFreeSpace.
ReserveFreeLocation serializes the pick under volumeCreateLock, verifies
the disk still has live room for the reservation, and charges it against
the location's cached free space until the next probe refreshes it.
* seaweed-volume: mirror the copy pick's live-check and headroom charge
Go's VolumeCopy now picks through Store.ReserveFreeLocation: live disk
stats under the store write lock, then the reservation charged against the
location's cached free space. The Rust plan_copy_destination gains the
same store.reserve_free_location so two copies cannot spend the same
headroom before the first .dat reservation lands.
* volume.copy: saturate the free-space charge against a live-space drop
The two statfs reads bracketing a pick can race a large external write; a
plain subtraction would underflow and leave the location looking roomy
until the next probe.
* volume.copy: release the headroom reservation when a copy does not land
ReserveFreeLocation charges the picked location's cached free space, but
a failed copy never returned it, so the headroom stayed booked until the
next disk-space probe and blocked retries. ReleaseFreeLocation refunds
the charge; the copy's cleanup defer now registers with the reservation
so every failure path (existing-replica delete, .note write, stream send)
releases it, including the post-transfer mount/validate failures covered
by the outer defer.
* seaweed-volume: mirror the copy headroom release on failed copies
release_free_location refunds reserve_free_location's charge. The copy
returns it when the plan's existing-replica delete fails, when the .note
write fails, and in cleanup_failed_copy, so a failed copy stops holding
the headroom until the next disk-space probe.
* volume.copy: keep pending copy headroom out of probe readings
ReleaseFreeLocation refunded the reservation into AvailableSpace even when
a CheckDiskSpace probe had already rebased the cache on live stats — the
probe's reading still shows the in-flight copy's bytes as free, so the
refund double-counted headroom and two copies could spend the same space,
driving the disk under -volume.minFreeSpace.
Track the outstanding charge in DiskLocation.ReservedSpace instead: probes
subtract it so they cannot re-spend it, a failed copy removes only its own
charge, and a landed copy commits it — the charge and the new usage cancel
out. hasRoomForNewVolume respects pending headroom too, so a preallocating
create cannot overrun an in-flight copy.
* seaweed-volume: mirror pending copy headroom across disk probes
reserved_space tracks the in-flight copy's charge separately from the
cached free space: check_disk_space subtracts it, release removes only the
failed copy's charge, a landed copy commits it, and has_room_for_new_volume
respects it — matching the Go side.
* volume.copy: settle a copy's headroom as its bytes land on disk
The pending charge held the copy's full reservation until commit, but
written and fallocated bytes already show up in statfs, so the live-space
check counted them twice. A slow copy could then block other copies and
volume growth despite free space above -volume.minFreeSpace.
Settle the charge instead: commit the bytes CreateVolumeFile actually
reserved (a failed fallocate leaves nothing committed), then settle each
file's write progress as it lands. The failure paths now release only the
unsettled remainder; allocatedSize is exported for the .dat's st_blocks.
* seaweed-volume: mirror copy headroom settling as bytes land
reserved_space keeps only the promised bytes statfs cannot see yet:
preallocate_dat settles whatever blocks the filesystem actually reserved
(0 when fallocate cannot), each copied file's progress settles the rest as
it lands, and a failed copy releases only the unsettled remainder —
matching the Go side.
* volume.copy: release a failed copy's headroom charge only once
Both defers fired on the same in-closure error: the inner released the
unsettled charge and the outer released it again, driving ReservedSpace
negative (an atomic.Uint64 underflow) and inflating AvailableSpace —
the location would then fail every later headroom check, or worse,
pass them with fabricated space. Zero the charge once it is released;
the Rust mirror already releases via swap(0) for the same reason.
* volume.copy: write into the preallocated .dat instead of staging over it
The .dat stream staged into .copying and renamed over the file
CreateVolumeFile had just fallocated, so the copy's transient footprint
was preallocate+dat — more than the reservation the pick just checked —
and the reservation itself was discarded at rename. Writing in place
reuses the fallocated extent, keeps the peak at the reserved maximum,
and leaves the copied volume preallocated like a created one.
* seaweed-volume: mirror the in-place write into the preallocated .dat
The .dat copy opened the just-fallocated file with truncate, freeing its
blocks at open and spending the headroom again as the stream re-grew it.
Append mode preserves them, matching the Go side.
---------
Co-authored-by: chrislusf <chrislusf@gmail.com>
Co-authored-by: user.email <chrislusf@users.noreply.github.com>
* s3 backend: optional HTTP client settings for the cloud tier
The S3 tier backend builds its client from a bare aws.Config, so it always
uses Go's default transport: HTTP/2 wherever the endpoint offers it over TLS,
no cap on connections per host, and a 90s idle timeout. Against some
S3-compatible services that is a poor fit. Over HTTP/2 every upload and
range read of a tiered volume shares one connection and its flow-control
window; with a small window, tier uploads and concurrent reads of tiered
volumes become throughput-bound on that one connection, while HTTP/1.1 opens
a connection per concurrent request. Some services also limit parallel
connections per client address, or drop idle connections before 90s.
Add three optional settings to [storage.backend.s3.<id>], all off by
default so existing deployments keep the SDK's default client:
- disable_http2: keep the client on HTTP/1.1 (ForceAttemptHTTP2=false and an
empty TLSNextProto, the documented opt-out);
- max_connections: MaxConnsPerHost, 0 = no cap;
- idle_connection_timeout_seconds: IdleConnTimeout, 0 = Go's default.
The session cache key includes the options when any is set, so two backends
on the same endpoint with different settings do not share a client.
ToProperties reports the settings; the master.toml scaffold documents them.
Tests: TestS3BackendHTTPOptionsReachTheClient fails without the change (the
settings never reach the client); TestS3BackendDefaultHTTPClientUnchanged,
TestS3BackendSessionsDifferByHTTPOptions, TestParseNonNegative.
* s3 backend: reject an idle timeout too large for time.Duration
An idle_connection_timeout_seconds above math.MaxInt64/1e9 overflowed when
multiplied by time.Second. The negative result made the transport keep its
default timeout and ToProperties export a negative value. Warn and use the
default instead.
* s3 backend: parse the idle timeout as 64-bit seconds
parseIdleConnTimeout went through strconv.Atoi, so on 32-bit platforms
any value above 2147483647 seconds was rejected while the same value was
accepted on 64-bit, and the overflow test failed under GOARCH=386. Parse
with ParseInt(…, 64) and export the effective value with FormatInt so the
accepted range is the same on every platform.
---------
Co-authored-by: Javier de la Torre <jatorre@lakehousebox.com>
* pb: add multipart concurrency fields to RemoteConf and tier move requests
RemoteConf gains upload_concurrency/download_concurrency (0 = client
default); VolumeTierMoveDatToRemote/FromRemote requests gain a
concurrency field (0 = backend default).
* remote storage: honor RemoteConf upload/download concurrency in s3 and azure clients
s3 client: ReadFile passes conf download_concurrency to the downloader,
WriteFile uses upload_concurrency for the uploader; previously
hard-coded 1 upload / 5 download parts. 0 keeps defaults. Same for
azure client.
* storage: plumb concurrency through backend interface and tier upload/download
BackendStorage.CopyFile/DownloadFile take a concurrency hint (<=0 =
backend configured default); s3 backend reads
upload_concurrency/download_concurrency from scaffold config with
parseConcurrency fallback, rclone updated to the new signature. Tier
move gRPC handlers forward the request concurrency to the backend.
* shell: -upload_concurrency/-download_concurrency for remote.configure, -concurrent for volume.tier
remote.configure exposes upload/download concurrency persisted into
RemoteConf; volume.tier move/evict commands forward -concurrent to the
tier move requests. Documented in master-cloud.toml scaffold.
* test: cover concurrency propagation in remote tier integration test
* remote.configure: merge existing config on partial update
Load the stored RemoteConf before saving so a partial update (e.g. only
-upload_concurrency) preserves credentials, endpoints, and type instead
of replacing them with new-config defaults. Only treat a confirmed
ErrNotFound as a new configuration; propagate all other load errors so a
transient filer failure does not overwrite stored settings.
On a type transition, reset backend-specific fields to the destination
type's new-config defaults rather than inheriting the old backend's
empty values. Bound configured concurrency to a sane maximum.
* remote storage: honor configured download concurrency in S3 and Azure
ReadFileWithConcurrency now resolves a zero request override against the
client's configured download_concurrency (new downloadConcurrency()
helpers), so the remote-mount/cache read path honors
RemoteConf.DownloadConcurrency instead of the hard-coded default.
Azure also clamps the resolved value to math.MaxUint16 regardless of
whether the fallback was used, preventing uint16 wraparound when a
configured value exceeds 65535.
* shell: rename -concurrent to -concurrency and validate tier transfer bounds
Rename the -concurrent flag to -concurrency across volume.tier.upload,
volume.tier.download, and volume.tier.compact to match the proto field and
RemoteConf field names. Add validateTierConcurrency to reject values that
would wrap int32 or exceed a 1024 cap before constructing the request.
* server: clamp tier move concurrency in gRPC handlers
Add clampTierConcurrency to both VolumeTierMoveDatToRemote and
VolumeTierMoveDatFromRemote handlers so a direct gRPC caller cannot spawn
an unbounded number of network workers.
* trim verbose comments added with concurrency feature
Remove redundant doc comments on the backend interface, rclone backend,
s3_backend parseConcurrency, and test helpers that restated the obvious.
* remote.configure: apply type defaults before re-parse so explicit flags win
applyTypeDefaults ran after the second flag parse, overwriting explicit
destination flags (e.g. -s3.region=eu-west-1) with new-config defaults.
Move the type-transition default reset before the re-parse so user-supplied
flags override the destination defaults.
* remote.configure: only treat explicit -type as a type transition
The first parse defaults -type to s3, so a concurrency-only update on an
existing non-S3 config captured requestedType=s3 and wrongly triggered a
type transition, resetting the stored backend to S3. Use fs.Visit to
detect whether -type was explicitly supplied; an omitted -type keeps the
stored backend.
---------
Co-authored-by: Jack Meredith <9480542+jackusm@users.noreply.github.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* volume server: open volume files with O_NOATIME
Nothing reads the atime of .dat, .idx, .sdx, or EC files, but every
needle read still dirtied the inode: even relatime writes atime on the
first read after each write, so an actively written volume paid a
metadata write per read/write cycle, and strictatime mounts paid one
per read. Open the serving handles with O_NOATIME, falling back to a
plain open when the file belongs to another owner (EPERM).
Claude-Session: https://claude.ai/code/session_015uVY4diBgEn3VYQoc2eMuD
* seaweed-volume: mirror the O_NOATIME volume file opens
Same change as the Go volume server: serving handles for .dat, .idx,
.sdx, .ecx, .ecj, and shard files open with O_NOATIME on Linux, with a
plain-open fallback on EPERM.
Claude-Session: https://claude.ai/code/session_015uVY4diBgEn3VYQoc2eMuD
* route the tier-down and recreate .dat opens through the no-atime helper
Review caught the Rust tier-down swap opening the local .dat directly.
The Go swapToLocalDatBackend and the zero-length read-only .dat
recreate in maybeWriteSuperBlock had the same gap: all three install
long-lived serving handles.
Claude-Session: https://claude.ai/code/session_015uVY4diBgEn3VYQoc2eMuD
* Add GitHub Actions workflow for codespell on master
* Add rudimentary codespell config
* Tune codespell config: skip generated code, ignore camelCase, whitelist domain terms
Add camelCase/PascalCase regex to ignore common Go/Rust/JS identifiers
like allLocations, publishErr, ReadInside, FlushInterval. Also skip
templ-generated *_templ.go files, and whitelist a handful of
short/domain-specific words (visibles, fo, te, ser, bject, unparseable,
keep-alives, tread, anc, ue) that show up as false positives across the
tree.
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix ambiguous typos and protect false positives
Fixes typos that codespell reports with multiple candidate suggestions
(so `codespell -w` cannot auto-apply them), plus one inline pragma and
one config entry to protect legitimate identifiers.
Manual fixes (single correct answer chosen from context):
- pattens -> patterns (5x) in filer/upload/shell flag help strings
- finded -> found (2x) in tarantool storage.lua comment
- spacify -> specify (2x) in helm chart values.yaml comment
- wether -> whether in skiplist.go docstring
- simpe -> simple in mq schema test case name
False-positive protection:
- Add `//codespell:ignore` next to `source GET's` (possessive of HTTP
verb) in s3api_object_handlers_copy_stream.go
- Whitelist `auther` in .codespellrc — it's a local variable meaning
"authenticator" in weed/security/tls.go, not a typo of "author".
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Extend codespell ignore list: .git-meta path and thirdparty groupId
Also skip `.git-meta` (scratch dir for commit messages that may contain
typo words verbatim) and whitelist `thirdparty` — it appears as the
literal Maven groupId `org.apache.hadoop.thirdparty` in hdfs3 poms
and cannot be renamed.
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [DATALAD RUNCMD] Fix non-ambiguous typos with codespell -w
Auto-applied fixes to the 44 remaining single-suggestion typos across
docs, comments, log messages, tests, config, and one Java pom.
=== Do not change lines below ===
{
"chain": [],
"cmd": "uvx codespell -w",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
* Revert breaking codespell fixes; whitelist unknwon and atleast
Two of the auto-applied `codespell -w` fixes were false positives that
would break the build/tests:
- go.mod: `github.com/unknwon/goconfig` is a real Go module path — the
upstream author's GitHub handle is literally `unknwon`. Renaming to
`unknown` would fail dependency resolution.
- test/benchmark/fuse_db/bin/{sqlite_verify.py,run_mysql.sh,run_sqlite.sh}:
`atleast` is a literal CLI mode value (a string constant compared and
passed as a positional argument). Rewriting to `at least` splits it
into two arguments and breaks the mode check.
Reverted those files and whitelisted both words in .codespellrc so
future runs won't re-suggest the same broken fixes.
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: report short S3 ReaderAt reads
Problem: S3BackendStorageFile.ReadAt returned a short buffer with a nil error, hiding truncated remote data.
Root cause: Every terminal io.EOF was cleared regardless of how many bytes were read.
Fix: Clear io.EOF only when the requested buffer was completely filled.
Validation: go test ./weed/storage/backend/...
Co-authored-by: Codex <noreply@openai.com>
* fix: validate S3 ReaderAt requests
Co-authored-by: Codex <noreply@openai.com>
* s3 ReadAt: simplify negative offset error
* s3 ReadAt: stop reading once the buffer is full
Skips the extra Read that only collected the terminal EOF, and bounds
the loop if the server ignores the Range header and returns more data
than requested, where Read on an empty slice can spin forever.
* s3 ReadAt: cover full reads that arrive with io.EOF
---------
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
The s3 and rclone tiered-storage backends were registered via blank imports
in weed/storage (volume_tier.go and volume_info/volume_info.go). That forced
every library consumer of weed/storage -- weed/shell, and through it external
tools -- to link aws-sdk-go and, under the rclone build tag, the full rclone
backend set and its cloud-storage SDKs, even though those consumers never tier
volumes.
Move the registrations into a new weed/storage/backend/all aggregator and
blank-import it once from weed/command, the binary's composition root. The weed
binary still registers both backends; weed/storage and its library consumers no
longer pull the backend SDKs into their dependency graph.
* tiering: stop a shared remote object being deleted while replicas still point at it
A remote-tiered volume's .dat content lives only in one cloud object that all
N replica .vif files point at. Deleting that object while destroying any one
replica, or before a downloaded replica is durable, bricks the survivors.
- volume.tier.move cleanup now deletes old replicas with keepRemoteData=true so
surviving replicas keep the shared object. Document why the alreadyPlaced
anchor needs no replica sync (same-object replicas are byte-identical).
- VolumeTierMoveDatFromRemote now fsyncs the downloaded .dat, fsyncs the
containing directory, trims the .vif (fsynced) and swaps to the local DiskFile
BEFORE deleting the remote object, on both the keep-remote and delete paths.
Only the final DeleteFile is gated by keep_remote_dat_file, so a keep-remote
download leaves the replica served from local disk rather than the shared
object, and a crash before delete merely leaks the object.
- volume.tier.download keeps the shared object for every replica except the
last, which deletes it.
- s3 and rclone download paths fsync the .dat before close.
* storage: swap the volume data backend under the data lock
The tier-download swap closed v.DataBackend and assigned the new local DiskFile
without holding dataFileAccessLock, racing concurrent reads/writes (use of a
closed file / nil deref). Add an exported Volume.SwapDataBackend that performs
the close-and-replace under the lock, and call it from the tier download.
* server: skip directory fsync on Windows in the tier download path
os.Open(dir).Sync() is unsupported on Windows and returns an error, which would
fail VolumeTierMoveDatFromRemote entirely there. Skip the directory fsync on
Windows, matching how the storage-side helper tolerates the unsupported case.
* shell: make multi-replica tier.download resilient to already-local replicas
If a multi-replica download is interrupted and retried, a replica made local
in the prior attempt returns "already on local disk", which aborted the whole
command and left the remaining remote replicas dangling. Treat that case as a
skip-and-continue so a retry completes the rest.
* server: assert downloaded .dat content, not just length, in the tier test
A length-only check passes even if the bytes are corrupted; compare the full
content of the local .dat against the original.
* chore(weed/storage/backend/s3_backend): remove unused function
* fix(s3_backend): cache session under the composite region|endpoint key
createSession looked up sessions by region|endpoint but stored them by
region alone, so the cache never hit and a new session was built every
call. With getSession gone the lock can also drop to a plain Mutex.
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* perf(cache): drop OS page cache after disk cache reads
After reading from the on-disk chunk cache, advise the kernel via
FADV_DONTNEED to release the corresponding page cache pages. This
prevents double-caching the same data in both user-space and kernel
page caches, freeing RAM for other uses on systems with large disk
caches.
* fix(cache): guard dropReadCache against zero length and invalid fd
A zero-length fadvise is interpreted as "to end of file" on Linux,
which would inadvertently drop the page cache for the entire remainder
of the cache volume. Also check fd >= 0 to avoid unnecessary syscalls
when the backend file is closed.
* perf(cache): only apply FADV_DONTNEED for reads >= 1 MiB
For small needle reads the syscall overhead outweighs the memory
savings, and the kernel page cache is more beneficial for warm data.
Restrict fadvise to reads of at least 1 MiB where the freed page
cache is meaningful.
* Replace removeDuplicateSlashes with NormalizeObjectKey
Use s3_constants.NormalizeObjectKey instead of removeDuplicateSlashes in most places
for consistency. NormalizeObjectKey handles both duplicate slash removal and ensures
the path starts with '/', providing more complete normalization.
* Fix double slash issues after NormalizeObjectKey
After using NormalizeObjectKey, object keys have a leading '/'. This commit ensures:
- getVersionedObjectDir strips leading slash before concatenation
- getEntry calls receive names without leading slash
- String concatenation with '/' doesn't create '//' paths
This prevents path construction errors like:
/buckets/bucket//object (wrong)
/buckets/bucket/object (correct)
* ensure object key leading "/"
* fix compilation
* fix: Strip leading slash from object keys in S3 API responses
After introducing NormalizeObjectKey, all internal object keys have a
leading slash. However, S3 API responses must return keys without
leading slashes to match AWS S3 behavior.
Fixed in three functions:
- addVersion: Strip slash for version list entries
- processRegularFile: Strip slash for regular file entries
- processExplicitDirectory: Strip slash for directory entries
This ensures ListObjectVersions and similar APIs return keys like
'bar' instead of '/bar', matching S3 API specifications.
* fix: Normalize keyMarker for consistent pagination comparison
The S3 API provides keyMarker without a leading slash (e.g., 'object-001'),
but after introducing NormalizeObjectKey, all internal object keys have
leading slashes (e.g., '/object-001').
When comparing keyMarker < normalizedObjectKey in shouldSkipObjectForMarker,
the ASCII value of '/' (47) is less than 'o' (111), causing all objects
to be incorrectly skipped during pagination. This resulted in page 2 and
beyond returning 0 results.
Fix: Normalize the keyMarker when creating versionCollector so comparisons
work correctly with normalized object keys.
Fixes pagination tests:
- TestVersioningPaginationOver1000Versions
- TestVersioningPaginationMultipleObjectsManyVersions
* refactor: Change NormalizeObjectKey to return keys without leading slash
BREAKING STRATEGY CHANGE:
Previously, NormalizeObjectKey added a leading slash to all object keys,
which required stripping it when returning keys to S3 API clients and
caused complexity in marker normalization for pagination.
NEW STRATEGY:
- NormalizeObjectKey now returns keys WITHOUT leading slash (e.g., 'foo/bar' not '/foo/bar')
- This matches the S3 API format directly
- All path concatenations now explicitly add '/' between bucket and object
- No need to strip slashes in responses or normalize markers
Changes:
1. Modified NormalizeObjectKey to strip leading slash instead of adding it
2. Fixed all path concatenations to use:
- BucketsPath + '/' + bucket + '/' + object
instead of:
- BucketsPath + '/' + bucket + object
3. Reverted response key stripping in:
- addVersion()
- processRegularFile()
- processExplicitDirectory()
4. Reverted keyMarker normalization in findVersionsRecursively()
5. Updated matchesPrefixFilter() to work with keys without leading slash
6. Fixed paths in handlers:
- s3api_object_handlers.go (GetObject, HeadObject, cacheRemoteObjectForStreaming)
- s3api_object_handlers_postpolicy.go
- s3api_object_handlers_tagging.go
- s3api_object_handlers_acl.go
- s3api_version_id.go (getVersionedObjectDir, getVersionIdFormat)
- s3api_object_versioning.go (getObjectVersionList, updateLatestVersionAfterDeletion)
All versioning tests pass including pagination stress tests.
* adjust format
* Update post policy tests to match new NormalizeObjectKey behavior
- Update TestPostPolicyKeyNormalization to expect keys without leading slashes
- Update TestNormalizeObjectKey to expect keys without leading slashes
- Update TestPostPolicyFilenameSubstitution to expect keys without leading slashes
- Update path construction in tests to use new pattern: BucketsPath + '/' + bucket + '/' + object
* Fix ListObjectVersions prefix filtering
Remove leading slash addition to prefix parameter to allow correct filtering
of .versions directories when listing object versions with a specific prefix.
The prefix parameter should match entry paths relative to bucket root.
Adding a leading slash was breaking the prefix filter for paginated requests.
Fixes pagination issue where second page returned 0 versions instead of
continuing with remaining versions.
* no leading slash
* Fix urlEscapeObject to add leading slash for filer paths
NormalizeObjectKey now returns keys without leading slashes to match S3 API format.
However, urlEscapeObject is used for filer paths which require leading slashes.
Add leading slash back after normalization to ensure filer paths are correct.
Fixes TestS3ApiServer_toFilerPath test failures.
* adjust tests
* normalize
* Fix: Normalize prefixes and markers in LIST operations using NormalizeObjectKey
Ensure consistent key normalization across all S3 operations (GET, PUT, LIST).
Previously, LIST operations were not applying the same normalization rules
(handling backslashes, duplicate slashes, leading slashes) as GET/PUT operations.
Changes:
- Updated normalizePrefixMarker() to call NormalizeObjectKey for both prefix and marker
- This ensures prefixes with leading slashes, backslashes, or duplicate slashes are
handled consistently with how object keys are normalized
- Fixes Parquet test failures where pads.write_dataset creates implicit directory
structures that couldn't be discovered by subsequent LIST operations
- Added TestPrefixNormalizationInList and TestListPrefixConsistency tests
All existing LIST tests continue to pass with the normalization improvements.
* Add debugging logging to LIST operations to track prefix normalization
* Fix: Remove leading slash addition from GetPrefix to work with NormalizeObjectKey
The NormalizeObjectKey function removes leading slashes to match S3 API format
(e.g., 'foo/bar' not '/foo/bar'). However, GetPrefix was adding a leading slash
back, which caused LIST operations to fail with incorrect path handling.
Now GetPrefix only normalizes duplicate slashes without adding a leading slash,
which allows NormalizeObjectKey changes to work correctly for S3 LIST operations.
All Parquet integration tests now pass (20/20).
* Fix: Handle object paths without leading slash in checkDirectoryObject
NormalizeObjectKey() removes the leading slash to match S3 API format.
However, checkDirectoryObject() was assuming the object path has a leading
slash when processing directory markers (paths ending with '/').
Now we ensure the object has a leading slash before processing it for
filer operations.
Fixes implicit directory marker test (explicit_dir/) while keeping
Parquet integration tests passing (20/20).
All tests pass:
- Implicit directory tests: 6/6
- Parquet integration tests: 20/20
* Fix: Handle explicit directory markers with trailing slashes
Explicit directory markers created with put_object(Key='dir/', ...) are stored
in the filer with the trailing slash as part of the name. The checkDirectoryObject()
function now checks for both:
1. Explicit directories: lookup with trailing slash preserved (e.g., 'explicit_dir/')
2. Implicit directories: lookup without trailing slash (e.g., 'implicit_dir')
This ensures both types of directory markers are properly recognized.
All tests pass:
- Implicit directory tests: 6/6 (including explicit directory marker test)
- Parquet integration tests: 20/20
* Fix: Preserve trailing slash in NormalizeObjectKey
NormalizeObjectKey now preserves trailing slashes when normalizing object keys.
This is important for explicit directory markers like 'explicit_dir/' which rely
on the trailing slash to be recognized as directory objects.
The normalization process:
1. Notes if trailing slash was present
2. Removes duplicate slashes and converts backslashes
3. Removes leading slash for S3 API format
4. Restores trailing slash if it was in the original
This ensures explicit directory markers created with put_object(Key='dir/', ...)
are properly normalized and can be looked up by their exact name.
All tests pass:
- Implicit directory tests: 6/6
- Parquet integration tests: 20/20
* clean object
* Fix: Don't restore trailing slash if result is empty
When normalizing paths that are only slashes (e.g., '///', '/'), the function
should return an empty string, not a single slash. The fix ensures we only
restore the trailing slash if the result is non-empty.
This fixes the 'just_slashes' test case:
- Input: '///'
- Expected: ''
- Previous: '/'
- Fixed: ''
All tests now pass:
- Unit tests: TestNormalizeObjectKey (13/13)
- Implicit directory tests: 6/6
- Parquet integration tests: 20/20
* prefixEndsOnDelimiter
* Update s3api_object_handlers_list.go
* Update s3api_object_handlers_list.go
* handle create directory
* Add Rclone storage backend
* Support templating the name of files stored via Rclone
* Enable Rclone accounting
* Remove redundant type conversion
* Provide progress information for Rclone download/upload operations
* Log error when Rclone can't instantiate filesystem
* Remove filename templating functionality for Rclone storage
To (maybe) be later reintroduced as a generic functionality for all
storage backends.
* Remove S3 specific check
* Move Rclone config initialisation to init() method
This reverts commit 670cb759f8.
with the pr
weed/storage () - (master) > go test -count=1 ./...
ok github.com/seaweedfs/seaweedfs/weed/storage 18.486s
? github.com/seaweedfs/seaweedfs/weed/storage/backend [no test files]
ok github.com/seaweedfs/seaweedfs/weed/storage/backend/memory_map 0.025s
? github.com/seaweedfs/seaweedfs/weed/storage/backend/s3_backend [no test files]
ok github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding 0.864s
? github.com/seaweedfs/seaweedfs/weed/storage/idx [no test files]
ok github.com/seaweedfs/seaweedfs/weed/storage/needle 0.110s
ok github.com/seaweedfs/seaweedfs/weed/storage/needle_map 24.414s
ok github.com/seaweedfs/seaweedfs/weed/storage/super_block 0.203s
? github.com/seaweedfs/seaweedfs/weed/storage/types [no test files]
? github.com/seaweedfs/seaweedfs/weed/storage/volume_info [no test files]
weed/storage () - (master) >
weed/storage () - (master) >
without the pr
weed/storage () - (master) >
weed/storage () - (master) > go test -count=1 ./...
ok github.com/seaweedfs/seaweedfs/weed/storage 1.617s
? github.com/seaweedfs/seaweedfs/weed/storage/backend [no test files]
ok github.com/seaweedfs/seaweedfs/weed/storage/backend/memory_map 0.026s
? github.com/seaweedfs/seaweedfs/weed/storage/backend/s3_backend [no test files]
ok github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding 0.906s
? github.com/seaweedfs/seaweedfs/weed/storage/idx [no test files]
ok github.com/seaweedfs/seaweedfs/weed/storage/needle 0.202s
ok github.com/seaweedfs/seaweedfs/weed/storage/needle_map 24.533s
ok github.com/seaweedfs/seaweedfs/weed/storage/super_block 0.280s
? github.com/seaweedfs/seaweedfs/weed/storage/types [no test files]
? github.com/seaweedfs/seaweedfs/weed/storage/volume_info [no test files]
