Tree: 4a764dbb37

add-ec-vacuum

add-weed-mini-command

add_fasthttp_client

add_remote_storage

adding-message-queue-integration-tests

adjust-fsck-cutoff-default

also-delete-parent-directory-if-empty

avoid_releasing_temp_file_on_write

changing-to-zap

collect-public-metrics

copilot/fix-helm-chart-installation

copilot/fix-s3-object-tagging-issue

copilot/make-renew-interval-configurable

copilot/make-renew-interval-configurable-again

copilot/sub-pr-7677

create-table-snapshot-api-design

data_query_pushdown

dependabot/maven/other/java/client/com.google.protobuf-protobuf-java-3.25.5

dependabot/maven/other/java/examples/org.apache.hadoop-hadoop-common-3.4.0

detect-and-plan-ec-tasks

do-not-retry-if-error-is-NotFound

ec-disk-type-support

enhance-erasure-coding

fasthttp

filer1_maintenance_branch

fix-GetObjectLockConfigurationHandler

fix-mount-http-parallelism

fix-mount-read-throughput-7504

fix-s3-object-tagging-issue-7589

fix-versioning-listing-only

ftp

gh-pages

improve-fuse-mount

improve-fuse-mount2

logrus

master

message_send

mount2

mq-subscribe

mq2

nfs-cookie-prefix-list-fixes

optimize-delete-lookups

original_weed_mount

pr-7412

raft-dual-write

random_access_file

refactor-needle-read-operations

refactor-volume-write

remote_overlay

remove-implicit-directory-handling

revert-5134-patch-1

revert-5819-patch-1

revert-6434-bugfix-missing-s3-audit

s3-remote-cache-singleflight

s3-select

sub

tcp_read

test-reverting-lock-table

test_udp

testing

testing-sdx-generation

tikv

track-mount-e2e

upgrade-versions-to-4.00

volume_buffered_writes

worker-execute-ec-tasks

0.72

0.72.release

0.73

0.74

0.75

0.76

0.77

0.90

0.91

0.92

0.93

0.94

0.95

0.96

0.97

0.98

0.99

1.00

1.01

1.02

1.03

1.04

1.05

1.06

1.07

1.08

1.09

1.10

1.11

1.12

1.14

1.15

1.16

1.17

1.18

1.19

1.20

1.21

1.22

1.23

1.24

1.25

1.26

1.27

1.28

1.29

1.30

1.31

1.32

1.33

1.34

1.35

1.36

1.37

1.38

1.40

1.41

1.42

1.43

1.44

1.45

1.46

1.47

1.48

1.49

1.50

1.51

1.52

1.53

1.54

1.55

1.56

1.57

1.58

1.59

1.60

1.61

1.61RC

1.62

1.63

1.64

1.65

1.66

1.67

1.68

1.69

1.70

1.71

1.72

1.73

1.74

1.75

1.76

1.77

1.78

1.79

1.80

1.81

1.82

1.83

1.84

1.85

1.86

1.87

1.88

1.90

1.91

1.92

1.93

1.94

1.95

1.96

1.97

1.98

1.99

1;70

2.00

2.01

2.02

2.03

2.04

2.05

2.06

2.07

2.08

2.09

2.10

2.11

2.12

2.13

2.14

2.15

2.16

2.17

2.18

2.19

2.20

2.21

2.22

2.23

2.24

2.25

2.26

2.27

2.28

2.29

2.30

2.31

2.32

2.33

2.34

2.35

2.36

2.37

2.38

2.39

2.40

2.41

2.42

2.43

2.47

2.48

2.49

2.50

2.51

2.52

2.53

2.54

2.55

2.56

2.57

2.58

2.59

2.60

2.61

2.62

2.63

2.64

2.65

2.66

2.67

2.68

2.69

2.70

2.71

2.72

2.73

2.74

2.75

2.76

2.77

2.78

2.79

2.80

2.81

2.82

2.83

2.84

2.85

2.86

2.87

2.88

2.89

2.90

2.91

2.92

2.93

2.94

2.95

2.96

2.97

2.98

2.99

3.00

3.01

3.02

3.03

3.04

3.05

3.06

3.07

3.08

3.09

3.10

3.11

3.12

3.13

3.14

3.15

3.16

3.18

3.19

3.20

3.21

3.22

3.23

3.24

3.25

3.26

3.27

3.28

3.29

3.30

3.31

3.32

3.33

3.34

3.35

3.36

3.37

3.38

3.39

3.40

3.41

3.42

3.43

3.44

3.45

3.46

3.47

3.48

3.50

3.51

3.52

3.53

3.54

3.55

3.56

3.57

3.58

3.59

3.60

3.61

3.62

3.63

3.64

3.65

3.66

3.67

3.68

3.69

3.71

3.72

3.73

3.74

3.75

3.76

3.77

3.78

3.79

3.80

3.81

3.82

3.83

3.84

3.85

3.86

3.87

3.88

3.89

3.90

3.91

3.92

3.93

3.94

3.95

3.96

3.97

3.98

3.99

4.00

4.01

4.02

4.03

dev

helm-3.65.1

v0.69

v0.70beta

v3.33

Branches Tags

${ item.name }

Create tag ${ searchTerm }

Create branch ${ searchTerm }

from '4a764dbb37'

${ noResults }

Commit Graph

117 Commits (4a764dbb3718fd42cd8a096aaa28294ef9ded41b)

Author	SHA1	Message	Date
chrislu	4a764dbb37	fmt	2 days ago
Chris Lu	4aa50bfa6a	fix: EC rebalance fails with replica placement 000 (#7812) * fix: EC rebalance fails with replica placement 000 This PR fixes several issues with EC shard distribution: 1. Pre-flight check before EC encoding - Verify target disk type has capacity before encoding starts - Prevents encoding shards only to fail during rebalance - Shows helpful error when wrong diskType is specified (e.g., ssd when volumes are on hdd) 2. Fix EC rebalance with replica placement 000 - When DiffRackCount=0, shards should be distributed freely across racks - The '000' placement means 'no volume replication needed' because EC provides redundancy - Previously all racks were skipped with error 'shards X > replica placement limit (0)' 3. Add unit tests for EC rebalance slot calculation - TestECRebalanceWithLimitedSlots: documents the limited slots scenario - TestECRebalanceZeroFreeSlots: reproduces the 0 free slots error 4. Add Makefile for manual EC testing - make setup: start cluster and populate data - make shell: open weed shell for EC commands - make clean: stop cluster and cleanup * fix: default -rebalance to true for ec.encode The -rebalance flag was defaulting to false, which meant ec.encode would only print shard moves but not actually execute them. This is a poor default since the whole point of EC encoding is to distribute shards across servers for fault tolerance. Now -rebalance defaults to true, so shards are actually distributed after encoding. Users can use -rebalance=false if they only want to see what would happen without making changes. * test/erasure_coding: improve Makefile safety and docs - Narrow pkill pattern for volume servers to use TEST_DIR instead of port pattern, avoiding accidental kills of unrelated SeaweedFS processes - Document external dependencies (curl, jq) in header comments * shell: refactor buildRackWithEcShards to reuse buildEcShards Extract common shard bit construction logic to avoid duplication between buildEcShards and buildRackWithEcShards helper functions. * shell: update test for EC replication 000 behavior When DiffRackCount=0 (replication "000"), EC shards should be distributed freely across racks since erasure coding provides its own redundancy. Update test expectation to reflect this behavior. * erasure_coding: add distribution package for proportional EC shard placement Add a new reusable package for EC shard distribution that: - Supports configurable EC ratios (not hard-coded 10+4) - Distributes shards proportionally based on replication policy - Provides fault tolerance analysis - Prefers moving parity shards to keep data shards spread out Key components: - ECConfig: Configurable data/parity shard counts - ReplicationConfig: Parsed XYZ replication policy - ECDistribution: Target shard counts per DC/rack/node - Rebalancer: Plans shard moves with parity-first strategy This enables seaweed-enterprise custom EC ratios and weed worker integration while maintaining a clean, testable architecture. * shell: integrate distribution package for EC rebalancing Add shell wrappers around the distribution package: - ProportionalECRebalancer: Plans moves using distribution.Rebalancer - NewProportionalECRebalancerWithConfig: Supports custom EC configs - GetDistributionSummary/GetFaultToleranceAnalysis: Helper functions The shell layer converts between EcNode types and the generic TopologyNode types used by the distribution package. * test setup * ec: improve data and parity shard distribution across racks - Add shardsByTypePerRack helper to track data vs parity shards - Rewrite doBalanceEcShardsAcrossRacks for two-pass balancing: 1. Balance data shards (0-9) evenly, max ceil(10/6)=2 per rack 2. Balance parity shards (10-13) evenly, max ceil(4/6)=1 per rack - Add balanceShardTypeAcrossRacks for generic shard type balancing - Add pickRackForShardType to select destination with room for type - Add unit tests for even data/parity distribution verification This ensures even read load during normal operation by spreading both data and parity shards across all available racks. * ec: make data/parity shard counts configurable in ecBalancer - Add dataShardCount and parityShardCount fields to ecBalancer struct - Add getDataShardCount() and getParityShardCount() methods with defaults - Replace direct constant usage with configurable methods - Fix unused variable warning for parityPerRack This allows seaweed-enterprise to use custom EC ratios while defaulting to standard 10+4 scheme. * Address PR 7812 review comments Makefile improvements: - Save PIDs for each volume server for precise termination - Use PID-based killing in stop target with pkill fallback - Use more specific pkill patterns with TEST_DIR paths Documentation: - Document jq dependency in README.md Rebalancer fix: - Fix duplicate shard count updates in applyMovesToAnalysis - All planners (DC/rack/node) update counts inline during planning - Remove duplicate updates from applyMovesToAnalysis to avoid double-counting * test/erasure_coding: use mktemp for test file template Use mktemp instead of hardcoded /tmp/testfile_template.bin path to provide better isolation for concurrent test runs.	2 days ago
Chris Lu	ed1da07665	Add consistent -debug and -debug.port flags to commands (#7816) * Add consistent -debug and -debug.port flags to commands Add -debug and -debug.port flags to weed master, weed volume, weed s3, weed mq.broker, and weed filer.sync commands for consistency with weed filer. When -debug is enabled, an HTTP server starts on the specified port (default 6060) serving runtime profiling data at /debug/pprof/. For mq.broker, replaced the older -port.pprof flag with the new -debug and -debug.port pattern for consistency. * Update weed/util/grace/pprof.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	2 days ago
Chris Lu	bccef78082	fix: reduce N+1 queries in S3 versioned object list operations (#7814) * fix: achieve single-scan efficiency for S3 versioned object listing When listing objects in a versioning-enabled bucket, the original code triggered multiple getEntry calls per versioned object (up to 12 with retries), causing excessive 'find' operations visible in Grafana and leading to high memory usage. This fix achieves single-scan efficiency by caching list metadata (size, ETag, mtime, owner) directly in the .versions directory: 1. Add new Extended keys for caching list metadata in .versions dir 2. Update upload/copy/multipart paths to cache metadata when creating versions 3. Update getLatestVersionEntryFromDirectoryEntry to use cached metadata (zero getEntry calls when cache is available) 4. Update updateLatestVersionAfterDeletion to maintain cache consistency Performance improvement for N versioned objects: - Before: N×1 to N×12 find operations per list request - After: 0 extra find operations (all metadata from single scan) This matches the efficiency of normal (non-versioned) object listing. * Update s3api_object_versioning.go * s3api: fix ETag handling for versioned objects and simplify delete marker creation - Add Md5 attribute to synthetic logicalEntry for single-part uploads to ensure filer.ETag() returns correct value in ListObjects response - Simplify delete marker creation by initializing entry directly in mkFile callback - Add bytes and encoding/hex imports for ETag parsing * s3api: preserve default attributes in delete marker mkFile callback Only modify Mtime field instead of replacing the entire Attributes struct, preserving default values like Crtime, FileMode, Uid, and Gid that mkFile initializes. * s3api: fix ETag handling in newListEntry for multipart uploads Prioritize ExtETagKey from Extended attributes before falling back to filer.ETag(). This properly handles multipart upload ETags (format: md5-parts) for versioned objects, where the synthetic entry has cached ETag metadata but no chunks to calculate from. * s3api: reduce code duplication in delete marker creation Extract deleteMarkerExtended map to be reused in both mkFile callback and deleteMarkerEntry construction. * test: add multipart upload versioning tests for ETag verification Add tests to verify that multipart uploaded objects in versioned buckets have correct ETags when listed: - TestMultipartUploadVersioningListETag: Basic multipart upload with 2 parts - TestMultipartUploadMultipleVersionsListETag: Multiple multipart versions - TestMixedSingleAndMultipartVersionsListETag: Mix of single-part and multipart These tests cover a bug where synthetic entries for versioned objects didn't include proper ETag handling for multipart uploads. * test: add delete marker test for multipart uploaded versioned objects TestMultipartUploadDeleteMarkerListBehavior verifies: - Delete marker creation hides object from ListObjectsV2 - ListObjectVersions shows both version and delete marker - Version ETag (multipart format) is preserved after delete marker - Object can be accessed by version ID after delete marker - Removing delete marker restores object visibility * refactor: address code review feedback - test: use assert.ElementsMatch for ETag verification (more idiomatic) - s3api: optimize newListEntry ETag logic (check ExtETagKey first) - s3api: fix edge case in ETag parsing (>= 2 instead of > 2) * s3api: prevent stale cached metadata and preserve existing extended attrs - setCachedListMetadata: clear old cached keys before setting new values to prevent stale data when new version lacks certain fields (e.g., owner) - createDeleteMarker: merge extended attributes instead of overwriting to preserve any existing metadata on the entry * s3api: extract clearCachedVersionMetadata to reduce code duplication - clearCachedVersionMetadata: clears only metadata fields (size, mtime, etag, owner, deleteMarker) - clearCachedListMetadata: now reuses clearCachedVersionMetadata + clears ID/filename - setCachedListMetadata: uses clearCachedVersionMetadata (not clearCachedListMetadata because caller has already set ID/filename) * s3api: share timestamp between version entry and cache entry Capture versionMtime once before mkFile and reuse for both: - versionEntry.Attributes.Mtime in the mkFile callback - versionEntryForCache.Attributes.Mtime for list caching This keeps list vs. HEAD LastModified timestamps aligned. * s3api: remove amzAccountId variable shadowing in multipart upload Extract amzAccountId before mkFile callback and reuse in both places, similar to how versionMtime is handled. Avoids confusion from redeclaring the same variable.	2 days ago
Chris Lu	2763f105f4	fix: use unique bucket name in TestS3IAMPresignedURLIntegration to avoid flaky test (#7801) The test was using a static bucket name 'test-iam-bucket' that could conflict with buckets created by other tests or previous runs. Each test framework creates new RSA keys for JWT signing, so the 'admin-user' identity differs between runs. When the bucket exists from a previous test, the new admin cannot access or delete it, causing AccessDenied errors. Changed to use GenerateUniqueBucketName() which ensures each test run gets its own bucket, avoiding cross-test conflicts.	4 days ago
Chris Lu	347ed7cbfa	fix: sync replica entries before ec.encode and volume.tier.move (#7798) * fix: sync replica entries before ec.encode and volume.tier.move (#7797) This addresses the data inconsistency risk in multi-replica volumes. When ec.encode or volume.tier.move operates on a multi-replica volume: 1. Find the replica with the highest file count (the 'best' one) 2. Copy missing entries from other replicas INTO this best replica 3. Use this union replica for the destructive operation This ensures no data is lost due to replica inconsistency before EC encoding or tier moving. Added: - command_volume_replica_check.go: Core sync and select logic - command_volume_replica_check_test.go: Test coverage Modified: - command_ec_encode.go: Call syncAndSelectBestReplica before encoding - command_volume_tier_move.go: Call syncAndSelectBestReplica before moving Fixes #7797 * test: add integration test for replicated volume sync during ec.encode * test: improve retry logic for replicated volume integration test * fix: resolve JWT issue in integration tests by using empty security.toml * address review comments: add readNeedleMeta, parallelize status fetch, fix collection param, fix test issues * test: use collection parameter consistently in replica sync test * fix: convert weed binary path to absolute to work with changed working directory * fix: remove skip behavior, keep tests failing on missing binary * fix: always check recency for each needle, add divergent replica test	4 days ago
G-OD	504b258258	s3: fix remote object not caching (#7790) * s3: fix remote object not caching * s3: address review comments for remote object caching - Fix leading slash in object name by using strings.TrimPrefix - Return cached entry from CacheRemoteObjectToLocalCluster to get updated local chunk locations - Reuse existing helper function instead of inline gRPC call * s3/filer: add singleflight deduplication for remote object caching - Add singleflight.Group to FilerServer to deduplicate concurrent cache operations - Wrap CacheRemoteObjectToLocalCluster with singleflight to ensure only one caching operation runs per object when multiple clients request the same file - Add early-return check for already-cached objects - S3 API calls filer gRPC with timeout and graceful fallback on error - Clear negative bucket cache when bucket is created via weed shell - Add integration tests for remote cache with singleflight deduplication This benefits all clients (S3, HTTP, Hadoop) accessing remote-mounted objects by preventing redundant cache operations and improving concurrent access performance. Fixes: https://github.com/seaweedfs/seaweedfs/discussions/7599 * fix: data race in concurrent remote object caching - Add mutex to protect chunks slice from concurrent append - Add mutex to protect fetchAndWriteErr from concurrent read/write - Fix incorrect error check (was checking assignResult.Error instead of parseErr) - Rename inner variable to avoid shadowing fetchAndWriteErr * fix: address code review comments - Remove duplicate remote caching block in GetObjectHandler, keep only singleflight version - Add mutex protection for concurrent chunk slice and error access (data race fix) - Use lazy initialization for S3 client in tests to avoid panic during package load - Fix markdown linting: add language specifier to code fence, blank lines around tables - Add 'all' target to Makefile as alias for test-with-server - Remove unused 'util' import * style: remove emojis from test files * fix: add defensive checks and sort chunks by offset - Add nil check and type assertion check for singleflight result - Sort chunks by offset after concurrent fetching to maintain file order * fix: improve test diagnostics and path normalization - runWeedShell now returns error for better test diagnostics - Add all targets to .PHONY in Makefile (logs-primary, logs-remote, health) - Strip leading slash from normalizedObject to avoid double slashes in path --------- Co-authored-by: chrislu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	5 days ago
chrislu	9150d84eea	test: use -master.peers=none for faster test server startup	5 days ago
chrislu	26121c55c9	test: improve pagination stress test with QUICK_TEST option and better assertions	5 days ago
chrislu	f517bc39fc	test: fix nil pointer dereference and add debugging to pagination stress tests	5 days ago
chrislu	0972a0acf3	test: add pagination stress tests for S3 versioning with >1000 versions	5 days ago
Chris Lu	f5c666052e	feat: add S3 bucket size and object count metrics (#7776) * feat: add S3 bucket size and object count metrics Adds periodic collection of bucket size metrics: - SeaweedFS_s3_bucket_size_bytes: logical size (deduplicated across replicas) - SeaweedFS_s3_bucket_physical_size_bytes: physical size (including replicas) - SeaweedFS_s3_bucket_object_count: object count (deduplicated) Collection runs every 1 minute via background goroutine that queries filer Statistics RPC for each bucket's collection. Also adds Grafana dashboard panels for: - S3 Bucket Size (logical vs physical) - S3 Bucket Object Count * address PR comments: fix bucket size metrics collection 1. Fix collectCollectionInfoFromMaster to use master VolumeList API - Now properly queries master for topology info - Uses WithMasterClient to get volume list from master - Correctly calculates logical vs physical size based on replication 2. Return error when filerClient is nil to trigger fallback - Changed from 'return nil, nil' to 'return nil, error' - Ensures fallback to filer stats is properly triggered 3. Implement pagination in listBucketNames - Added listBucketPageSize constant (1000) - Uses StartFromFileName for pagination - Continues fetching until fewer entries than limit returned 4. Handle NewReplicaPlacementFromByte error and prevent division by zero - Check error return from NewReplicaPlacementFromByte - Default to 1 copy if error occurs - Add explicit check for copyCount == 0 * simplify bucket size metrics: remove filer fallback, align with quota enforcement - Remove fallback to filer Statistics RPC - Use only master topology for collection info (same as s3.bucket.quota.enforce) - Updated comments to clarify this runs the same collection logic as quota enforcement - Simplified code by removing collectBucketSizeFromFilerStats * use s3a.option.Masters directly instead of querying filer * address PR comments: fix dashboard overlaps and improve metrics collection Grafana dashboard fixes: - Fix overlapping panels 55 and 59 in grafana_seaweedfs.json (moved 59 to y=30) - Fix grid collision in k8s dashboard (moved panel 72 to y=48) - Aggregate bucket metrics with max() by (bucket) for multi-instance S3 gateways Go code improvements: - Add graceful shutdown support via context cancellation - Use ticker instead of time.Sleep for better shutdown responsiveness - Distinguish EOF from actual errors in stream handling * improve bucket size metrics: multi-master failover and proper error handling - Initial delay now respects context cancellation using select with time.After - Use WithOneOfGrpcMasterClients for multi-master failover instead of hardcoding Masters[0] - Properly propagate stream errors instead of just logging them (EOF vs real errors) * improve bucket size metrics: distributed lock and volume ID deduplication - Add distributed lock (LiveLock) so only one S3 instance collects metrics at a time - Add IsLocked() method to LiveLock for checking lock status - Fix deduplication: use volume ID tracking instead of dividing by copyCount - Previous approach gave wrong results if replicas were missing - Now tracks seen volume IDs and counts each volume only once - Physical size still includes all replicas for accurate disk usage reporting * rename lock to s3.leader * simplify: remove StartBucketSizeMetricsCollection wrapper function * fix data race: use atomic operations for LiveLock.isLocked field - Change isLocked from bool to int32 - Use atomic.LoadInt32/StoreInt32 for all reads/writes - Sync shared isLocked field in StartLongLivedLock goroutine * add nil check for topology info to prevent panic * fix bucket metrics: use Ticker for consistent intervals, fix pagination logic - Use time.Ticker instead of time.After for consistent interval execution - Fix pagination: count all entries (not just directories) for proper termination - Update lastFileName for all entries to prevent pagination issues * address PR comments: remove redundant atomic store, propagate context - Remove redundant atomic.StoreInt32 in StartLongLivedLock (AttemptToLock already sets it) - Propagate context through metrics collection for proper cancellation on shutdown - collectAndUpdateBucketSizeMetrics now accepts ctx - collectCollectionInfoFromMaster uses ctx for VolumeList RPC - listBucketNames uses ctx for ListEntries RPC	5 days ago
dependabot[bot]	d0cc51e7c6	chore(deps): bump io.netty:netty-codec-http from 4.1.125.Final to 4.1.129.Final in /test/java/spark (#7773) chore(deps): bump io.netty:netty-codec-http in /test/java/spark Bumps [io.netty:netty-codec-http](https://github.com/netty/netty) from 4.1.125.Final to 4.1.129.Final. - [Commits](https://github.com/netty/netty/compare/netty-4.1.125.Final...netty-4.1.129.Final) --- updated-dependencies: - dependency-name: io.netty:netty-codec-http dependency-version: 4.1.129.Final dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	6 days ago
dependabot[bot]	c6e07429e7	chore(deps): bump golang.org/x/image from 0.33.0 to 0.34.0 (#7764) * chore(deps): bump golang.org/x/image from 0.33.0 to 0.34.0 Bumps [golang.org/x/image](https://github.com/golang/image) from 0.33.0 to 0.34.0. - [Commits](https://github.com/golang/image/compare/v0.33.0...v0.34.0) --- updated-dependencies: - dependency-name: golang.org/x/image dependency-version: 0.34.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chrislu <chris.lu@gmail.com>	6 days ago
Chris Lu	5a03b5538f	filer: improve FoundationDB performance by disabling batch by default (#7770) * filer: improve FoundationDB performance by disabling batch by default This PR addresses a performance issue where FoundationDB filer was achieving only ~757 ops/sec with 12 concurrent S3 clients, despite FDB being capable of 17,000+ ops/sec. Root cause: The write batcher was waiting up to 5ms for each operation to batch, even though S3 semantics require waiting for durability confirmation. This added artificial latency that defeated the purpose of batching. Changes: - Disable write batching by default (batch_enabled = false) - Each write now commits immediately in its own transaction - Reduce batch interval from 5ms to 1ms when batching is enabled - Add batch_enabled config option to toggle behavior - Improve batcher to collect available ops without blocking - Add benchmarks comparing batch vs no-batch performance Benchmark results (16 concurrent goroutines): - With batch: 2,924 ops/sec (342,032 ns/op) - Without batch: 4,625 ops/sec (216,219 ns/op) - Improvement: +58% faster Configuration: - Default: batch_enabled = false (optimal for S3 PUT latency) - For bulk ingestion: set batch_enabled = true Also fixes ARM64 Docker test setup (shell compatibility, fdbserver path). * fix: address review comments - use atomic counter and remove duplicate batcher - Use sync/atomic.Uint64 for unique filenames in concurrent benchmarks - Remove duplicate batcher creation in createBenchmarkStoreWithBatching (initialize() already creates batcher when batchEnabled=true) * fix: add realistic default values to benchmark store helper Set directoryPrefix, timeout, and maxRetryDelay to reasonable defaults for more realistic benchmark conditions.	6 days ago
Chris Lu	44beb42eb9	s3: fix PutObject ETag format for multi-chunk uploads (#7771) * s3: fix PutObject ETag format for multi-chunk uploads Fix issue #7768: AWS S3 SDK for Java fails with 'Invalid base 16 character: -' when performing PutObject on files that are internally auto-chunked. The issue was that SeaweedFS returned a composite ETag format (<md5hash>-<count>) for regular PutObject when the file was split into multiple chunks due to auto-chunking. However, per AWS S3 spec, the composite ETag format should only be used for multipart uploads (CreateMultipartUpload/UploadPart/CompleteMultipartUpload API). Regular PutObject should always return a pure MD5 hash as the ETag, regardless of how the file is stored internally. The fix ensures the MD5 hash is always stored in entry.Attributes.Md5 for regular PutObject operations, so filer.ETag() returns the pure MD5 hash instead of falling back to ETagChunks() composite format. * test: add comprehensive ETag format tests for issue #7768 Add integration tests to ensure PutObject ETag format compatibility: Go tests (test/s3/etag/): - TestPutObjectETagFormat_SmallFile: 1KB single chunk - TestPutObjectETagFormat_LargeFile: 10MB auto-chunked (critical for #7768) - TestPutObjectETagFormat_ExtraLargeFile: 25MB multi-chunk - TestMultipartUploadETagFormat: verify composite ETag for multipart - TestPutObjectETagConsistency: ETag consistency across PUT/HEAD/GET - TestETagHexValidation: simulate AWS SDK v2 hex decoding - TestMultipleLargeFileUploads: stress test multiple large uploads Java tests (other/java/s3copier/): - Update pom.xml to include AWS SDK v2 (2.20.127) - Add ETagValidationTest.java with comprehensive SDK v2 tests - Add README.md documenting SDK versions and test coverage Documentation: - Add test/s3/SDK_COMPATIBILITY.md documenting validated SDK versions - Add test/s3/etag/README.md explaining test coverage These tests ensure large file PutObject (>8MB) returns pure MD5 ETags (not composite format), which is required for AWS SDK v2 compatibility. * fix: lower Java version requirement to 11 for CI compatibility * address CodeRabbit review comments - s3_etag_test.go: Handle rand.Read error, fix multipart part-count logging - Makefile: Add 'all' target, pass S3_ENDPOINT to test commands - SDK_COMPATIBILITY.md: Add language tag to fenced code block - ETagValidationTest.java: Add pagination to cleanup logic - README.md: Clarify Go SDK tests are in separate location * ci: add s3copier ETag validation tests to Java integration tests - Enable S3 API (-s3 -s3.port=8333) in SeaweedFS test server - Add S3 API readiness check to wait loop - Add step to run ETagValidationTest from s3copier This ensures the fix for issue #7768 is continuously tested against AWS SDK v2 for Java in CI. * ci: add S3 config with credentials for s3copier tests - Add -s3.config pointing to docker/compose/s3.json - Add -s3.allowDeleteBucketNotEmpty for test cleanup - Set S3_ACCESS_KEY and S3_SECRET_KEY env vars for tests * ci: pass S3 config as Maven system properties Pass S3_ENDPOINT, S3_ACCESS_KEY, S3_SECRET_KEY via -D flags so they're available via System.getProperty() in Java tests	6 days ago
Chris Lu	1b1e5f69a2	Add TUS protocol support for resumable uploads (#7592) * Add TUS protocol integration tests This commit adds integration tests for the TUS (resumable upload) protocol in preparation for implementing TUS support in the filer. Test coverage includes: - OPTIONS handler for capability discovery - Basic single-request upload - Chunked/resumable uploads - HEAD requests for offset tracking - DELETE for upload cancellation - Error handling (invalid offsets, missing uploads) - Creation-with-upload extension - Resume after interruption simulation Tests are skipped in short mode and require a running SeaweedFS cluster. * Add TUS session storage types and utilities Implements TUS upload session management: - TusSession struct for tracking upload state - Session creation with directory-based storage - Session persistence using filer entries - Session retrieval and offset updates - Session deletion with chunk cleanup - Upload completion with chunk assembly into final file Session data is stored in /.uploads.tus/{upload-id}/ directory, following the pattern used by S3 multipart uploads. * Add TUS HTTP handlers Implements TUS protocol HTTP handlers: - tusHandler: Main entry point routing requests - tusOptionsHandler: Capability discovery (OPTIONS) - tusCreateHandler: Create new upload (POST) - tusHeadHandler: Get upload offset (HEAD) - tusPatchHandler: Upload data at offset (PATCH) - tusDeleteHandler: Cancel upload (DELETE) - tusWriteData: Upload data to volume servers Features: - Supports creation-with-upload extension - Validates TUS protocol headers - Offset conflict detection - Automatic upload completion when size is reached - Metadata parsing from Upload-Metadata header * Wire up TUS protocol routes in filer server Add TUS handler route (/.tus/) to the filer HTTP server. The TUS route is registered before the catch-all route to ensure proper routing of TUS protocol requests. TUS protocol is now accessible at: - OPTIONS /.tus/ - Capability discovery - POST /.tus/{path} - Create upload - HEAD /.tus/.uploads/{id} - Get offset - PATCH /.tus/.uploads/{id} - Upload data - DELETE /.tus/.uploads/{id} - Cancel upload * Improve TUS integration test setup Add comprehensive Makefile for TUS tests with targets: - test-with-server: Run tests with automatic server management - test-basic/chunked/resume/errors: Specific test categories - manual-start/stop: For development testing - debug-logs/status: For debugging - ci-test: For CI/CD pipelines Update README.md with: - Detailed TUS protocol documentation - All endpoint descriptions with headers - Usage examples with curl commands - Architecture diagram - Comparison with S3 multipart uploads Follows the pattern established by other tests in test/ folder. * Fix TUS integration tests and creation-with-upload - Fix test URLs to use full URLs instead of relative paths - Fix creation-with-upload to refresh session before completing - Fix Makefile to properly handle test cleanup - Add FullURL helper function to TestCluster * Add TUS protocol tests to GitHub Actions CI - Add tus-tests.yml workflow that runs on PRs and pushes - Runs when TUS-related files are modified - Automatic server management for integration testing - Upload logs on failure for debugging * Make TUS base path configurable via CLI - Add -tus.path CLI flag to filer command - TUS is disabled by default (empty path) - Example: -tus.path=/.tus to enable at /.tus endpoint - Update test Makefile to use -tus.path flag - Update README with TUS enabling instructions * Rename -tus.path to -tusBasePath with default .tus - Rename CLI flag from -tus.path to -tusBasePath - Default to .tus (TUS enabled by default) - Add -filer.tusBasePath option to weed server command - Properly handle path prefix (prepend / if missing) * Address code review comments - Sort chunks by offset before assembling final file - Use chunk.Offset directly instead of recalculating - Return error on invalid file ID instead of skipping - Require Content-Length header for PATCH requests - Use fs.option.Cipher for encryption setting - Detect MIME type from data using http.DetectContentType - Fix concurrency group for push events in workflow - Use os.Interrupt instead of Kill for graceful shutdown in tests * fmt * Address remaining code review comments - Fix potential open redirect vulnerability by sanitizing uploadLocation path - Add language specifier to README code block - Handle os.Create errors in test setup - Use waitForHTTPServer instead of time.Sleep for master/volume readiness - Improve test reliability and debugging * Address critical and high-priority review comments - Add per-session locking to prevent race conditions in updateTusSessionOffset - Stream data directly to volume server instead of buffering entire chunk - Only buffer 512 bytes for MIME type detection, then stream remaining data - Clean up session locks when session is deleted * Fix race condition to work across multiple filer instances - Store each chunk as a separate file entry instead of updating session JSON - Chunk file names encode offset, size, and fileId for atomic storage - getTusSession loads chunks from directory listing (atomic read) - Eliminates read-modify-write race condition across multiple filers - Remove in-memory mutex that only worked for single filer instance * Address code review comments: fix variable shadowing, sniff size, and test stability - Rename path variable to reqPath to avoid shadowing path package - Make sniff buffer size respect contentLength (read at most contentLength bytes) - Handle Content-Length < 0 in creation-with-upload (return error for chunked encoding) - Fix test cluster: use temp directory for filer store, add startup delay * Fix test stability: increase cluster stabilization delay to 5 seconds The tests were intermittently failing because the volume server needed more time to create volumes and register with the master. Increasing the delay from 2 to 5 seconds fixes the flaky test behavior. * Address PR review comments for TUS protocol support - Fix strconv.Atoi error handling in test file (lines 386, 747) - Fix lossy fileId encoding: use base64 instead of underscore replacement - Add pagination support for ListDirectoryEntries in getTusSession - Batch delete chunks instead of one-by-one in deleteTusSession * Address additional PR review comments for TUS protocol - Fix UploadAt timestamp: use entry.Crtime instead of time.Now() - Remove redundant JSON content in chunk entry (metadata in filename) - Refactor tusWriteData to stream in 4MB chunks to avoid OOM on large uploads - Pass filer.Entry to parseTusChunkPath to preserve actual upload time * Address more PR review comments for TUS protocol - Normalize TUS path once in filer_server.go, store in option.TusPath - Remove redundant path normalization from TUS handlers - Remove goto statement in tusCreateHandler, simplify control flow * Remove unnecessary mutexes in tusWriteData The upload loop is sequential, so uploadErrLock and chunksLock are not needed. * Rename updateTusSessionOffset to saveTusChunk Remove unused newOffset parameter and rename function to better reflect its purpose. * Improve TUS upload performance and add path validation - Reuse operation.Uploader across sub-chunks for better connection reuse - Guard against TusPath='/' to prevent hijacking all filer routes * Address PR review comments for TUS protocol - Fix critical chunk filename parsing: use strings.Cut instead of SplitN to correctly handle base64-encoded fileIds that may contain underscores - Rename tusPath to tusBasePath for naming consistency across codebase - Add background garbage collection for expired TUS sessions (runs hourly) - Improve error messages with %w wrapping for better debuggability * Address additional TUS PR review comments - Fix tusBasePath default to use leading slash (/.tus) for consistency - Add chunk contiguity validation in completeTusUpload to detect gaps/overlaps - Fix offset calculation to find maximum contiguous range from 0, not just last chunk - Return 413 Request Entity Too Large instead of silently truncating content - Document tusChunkSize rationale (4MB balances memory vs request overhead) - Fix Makefile xargs portability by removing GNU-specific -r flag - Add explicit -tusBasePath flag to integration test for robustness - Fix README example to use /.uploads/tus path format * Revert log_buffer changes (moved to separate PR) * Minor style fixes from PR review - Simplify tusBasePath flag description to use example format - Add 'TUS upload' prefix to session not found error message - Remove duplicate tusChunkSize comment - Capitalize warning message for consistency - Add grep filter to Makefile xargs for better empty input handling	6 days ago
Chris Lu	51c2ab0107	fix: admin UI bucket deletion with filer group configured (#7735)	1 week ago
Chris Lu	36b8b2147b	test: add integration test for versioned object listing path fix (#7731) * test: add integration test for versioned object listing path fix Add integration test that validates the fix for GitHub discussion #7573. The test verifies that: - Entry names use path.Base() to get base filename only - Path doubling bug is prevented when listing versioned objects - Logical entries are created correctly with proper attributes - .versions folder paths are handled correctly This test documents the Velero/Kopia compatibility fix and prevents regression of the path doubling bug. * test: add Velero/Kopia integration test for versioned object listing Add integration tests that simulate Velero/Kopia's exact access patterns when using S3 versioning. These tests validate the fix for GitHub discussion #7573 where versioned objects with nested paths would have their paths doubled in ListObjects responses. Tests added: - TestVeleroKopiaVersionedObjectListing: Tests various Kopia path patterns - TestVeleroKopiaGetAfterList: Verifies list-then-get workflow works - TestVeleroMultipleVersionsWithNestedPaths: Tests multi-version objects - TestVeleroListVersionsWithNestedPaths: Tests ListObjectVersions API Each test verifies: 1. Listed keys match original keys without path doubling 2. Objects can be retrieved using the listed keys 3. Content integrity is maintained Related: https://github.com/seaweedfs/seaweedfs/discussions/7573 * refactor: remove old unit test, keep only Velero integration test Remove weed/s3api/s3api_versioning_list_test.go as it was a simpler unit test that the comprehensive Velero integration test supersedes. The integration test in test/s3/versioning/s3_velero_integration_test.go provides better coverage by actually exercising the S3 API with real AWS SDK calls. * refactor: use defer for response body cleanup in test loop Use anonymous function with defer for getResp.Body.Close() to be more defensive against future code additions in the loop body. * refactor: improve hasDoubledPath function clarity and efficiency - Fix comment to accurately describe checking for repeated pairs - Tighten outer loop bound from len(parts)-2 to len(parts)-3 - Remove redundant bounds checks in the condition	1 week ago
Chris Lu	c6d6ee8297	test: fix master client timeout causing test hangs (#7715) * test: fix master client timeout causing test hangs Use the main test context for KeepConnectedToMaster instead of creating a separate 60s context. The tests have 180s outer timeouts but the master client was disconnecting after 60s, causing subsequent commands to hang waiting for reconnection. * test: add -peers=none to all test masters and timeout for lock - Add -peers=none flag to all master servers for faster startup - Add tryLockWithTimeout helper to avoid tests hanging on lock acquisition - Skip tests if lock cannot be acquired within 30 seconds * test: extract connectToMasterAndSync helper to reduce duplication * test: fix captureCommandOutput pipe deadlock Close write end of pipe before calling io.ReadAll to signal EOF, otherwise ReadAll blocks forever waiting for more data. * test: fix tryLockWithTimeout to check lock command errors Propagate lock command error through channel and only treat as locked if command succeeded. Previously any completion (including errors) was treated as successful lock acquisition.	1 week ago
Chris Lu	df4f2f7020	ec: add -diskType flag to EC commands for SSD support (#7607) * ec: add diskType parameter to core EC functions Add diskType parameter to: - ecBalancer struct - collectEcVolumeServersByDc() - collectEcNodesForDC() - collectEcNodes() - EcBalance() This allows EC operations to target specific disk types (hdd, ssd, etc.) instead of being hardcoded to HardDriveType only. For backward compatibility, all callers currently pass types.HardDriveType as the default value. Subsequent commits will add -diskType flags to the individual EC commands. * ec: update helper functions to use configurable diskType Update the following functions to accept/use diskType parameter: - findEcVolumeShards() - addEcVolumeShards() - deleteEcVolumeShards() - moveMountedShardToEcNode() - countShardsByRack() - pickNEcShardsToMoveFrom() All ecBalancer methods now use ecb.diskType instead of hardcoded types.HardDriveType. Non-ecBalancer callers (like volumeServer.evacuate and ec.rebuild) use types.HardDriveType as the default. Update all test files to pass diskType where needed. * ec: add -diskType flag to ec.balance and ec.encode commands Add -diskType flag to specify the target disk type for EC operations: - ec.balance -diskType=ssd - ec.encode -diskType=ssd The disk type can be 'hdd', 'ssd', or empty for default (hdd). This allows placing EC shards on SSD or other disk types instead of only HDD. Example usage: ec.balance -collection=mybucket -diskType=ssd -apply ec.encode -collection=mybucket -diskType=ssd -force * test: add integration tests for EC disk type support Add integration tests to verify the -diskType flag works correctly: - TestECDiskTypeSupport: Tests EC encode and balance with SSD disk type - TestECDiskTypeMixedCluster: Tests EC operations on a mixed HDD/SSD cluster The tests verify: - Volume servers can be configured with specific disk types - ec.encode accepts -diskType flag and encodes to the correct disk type - ec.balance accepts -diskType flag and balances on the correct disk type - Mixed disk type clusters work correctly with separate collections * ec: add -sourceDiskType to ec.encode and -diskType to ec.decode ec.encode: - Add -sourceDiskType flag to filter source volumes by disk type - This enables tier migration scenarios (e.g., SSD volumes → HDD EC shards) - -diskType specifies target disk type for EC shards ec.decode: - Add -diskType flag to specify source disk type where EC shards are stored - Update collectEcShardIds() and collectEcNodeShardBits() to accept diskType Examples: # Encode SSD volumes to HDD EC shards (tier migration) ec.encode -collection=mybucket -sourceDiskType=ssd -diskType=hdd # Decode EC shards from SSD ec.decode -collection=mybucket -diskType=ssd Integration tests updated to cover new flags. * ec: fix variable shadowing and add -diskType to ec.rebuild and volumeServer.evacuate Address code review comments: 1. Fix variable shadowing in collectEcVolumeServersByDc(): - Rename loop variable 'diskType' to 'diskTypeKey' and 'diskTypeStr' to avoid shadowing the function parameter 2. Fix hardcoded HardDriveType in ecBalancer methods: - balanceEcRack(): use ecb.diskType instead of types.HardDriveType - collectVolumeIdToEcNodes(): use ecb.diskType 3. Add -diskType flag to ec.rebuild command: - Add diskType field to ecRebuilder struct - Pass diskType to collectEcNodes() and addEcVolumeShards() 4. Add -diskType flag to volumeServer.evacuate command: - Add diskType field to commandVolumeServerEvacuate struct - Pass diskType to collectEcVolumeServersByDc() and moveMountedShardToEcNode() * test: add diskType field to ecBalancer in TestPickEcNodeToBalanceShardsInto Address nitpick comment: ensure test ecBalancer struct has diskType field set for consistency with other tests. * ec: filter disk selection by disk type in pickBestDiskOnNode When evacuating or rebalancing EC shards, pickBestDiskOnNode now filters disks by the target disk type. This ensures: 1. EC shards from SSD disks are moved to SSD disks on destination nodes 2. EC shards from HDD disks are moved to HDD disks on destination nodes 3. No cross-disk-type shard movement occurs This maintains the storage tier isolation when moving EC shards between nodes during evacuation or rebalancing operations. * ec: allow disk type fallback during evacuation Update pickBestDiskOnNode to accept a strictDiskType parameter: - strictDiskType=true (balancing): Only use disks of matching type. This maintains storage tier isolation during normal rebalancing. - strictDiskType=false (evacuation): Prefer same disk type, but fall back to other disk types if no matching disk is available. This ensures evacuation can complete even when same-type capacity is insufficient. Priority order for evacuation: 1. Same disk type with lowest shard count (preferred) 2. Different disk type with lowest shard count (fallback) * test: use defer for lock/unlock to prevent lock leaks Use defer to ensure locks are always released, even on early returns or test failures. This prevents lock leaks that could cause subsequent tests to hang or fail. Changes: - Return early if lock acquisition fails - Immediately defer unlock after successful lock - Remove redundant explicit unlock calls at end of tests - Fix unused variable warning (err -> encodeErr/locErr) * ec: dynamically discover disk types from topology for evacuation Disk types are free-form tags (e.g., 'ssd', 'nvme', 'archive') that come from the topology, not a hardcoded set. Only 'hdd' (or empty) is the default disk type. Use collectVolumeDiskTypes() to discover all disk types present in the cluster topology instead of hardcoding [HardDriveType, SsdType]. * test: add evacuation fallback and cross-rack EC placement tests Add two new integration tests: 1. TestEvacuationFallbackBehavior: - Tests that when same disk type has no capacity, shards fall back to other disk types during evacuation - Creates cluster with 1 SSD + 2 HDD servers (limited SSD capacity) - Verifies pickBestDiskOnNode behavior with strictDiskType=false 2. TestCrossRackECPlacement: - Tests EC shard distribution across different racks - Creates cluster with 4 servers in 4 different racks - Verifies shards are spread across multiple racks - Tests that ec.balance respects rack placement Helper functions added: - startLimitedSsdCluster: 1 SSD + 2 HDD servers - startMultiRackCluster: 4 servers in 4 racks - countShardsPerRack: counts EC shards per rack from disk * test: fix collection mismatch in TestCrossRackECPlacement The EC commands were using collection 'rack_test' but uploaded test data uses collection 'test' (default). This caused ec.encode/ec.balance to not find the uploaded volume. Fix: Change EC commands to use '-collection test' to match the uploaded data. Addresses review comment from PR #7607. * test: close log files in MultiDiskCluster.Stop() to prevent FD leaks Track log files in MultiDiskCluster.logFiles and close them in Stop() to prevent file descriptor accumulation in long-running or many-test scenarios. Addresses review comment about logging resources cleanup. * test: improve EC integration tests with proper assertions - Add assertNoFlagError helper to detect flag parsing regressions - Update diskType subtests to fail on flag errors (ec.encode, ec.balance, ec.decode) - Update verify_disktype_flag_parsing to check help output contains diskType - Remove verify_fallback_disk_selection (was documentation-only, not executable) - Add assertion to verify_cross_rack_distribution for minimum 2 racks - Consolidate uploadTestDataWithDiskType to accept collection parameter - Remove duplicate uploadTestDataWithDiskTypeMixed function * test: extract captureCommandOutput helper and fix error handling - Add captureCommandOutput helper to reduce code duplication in diskType tests - Create commandRunner interface to match shell command Do method - Update ec_encode_with_ssd_disktype, ec_balance_with_ssd_disktype, ec_encode_with_source_disktype, ec_decode_with_disktype to use helper - Fix filepath.Glob error handling in countShardsPerRack instead of ignoring it * test: add flag validation to ec_balance_targets_correct_disk_type Add assertNoFlagError calls after ec.balance commands to ensure -diskType flag is properly recognized for both SSD and HDD disk types. * test: add proper assertions for EC command results - ec_encode_with_ssd_disktype: check for expected volume-related errors - ec_balance_with_ssd_disktype: require success with require.NoError - ec_encode_with_source_disktype: check for expected no-volume errors - ec_decode_with_disktype: check for expected no-ec-volume errors - upload_to_ssd_and_hdd: use require.NoError for setup validation Tests now properly fail on unexpected errors rather than just logging. * test: fix missing unlock in ec_encode_with_disk_awareness Add defer unlock pattern to ensure lock is always released, matching the pattern used in other subtests. * test: improve helper robustness - Make assertNoFlagError case-insensitive for pattern matching - Use defer in captureCommandOutput to restore stdout/stderr and close pipe ends to avoid FD leaks even if cmd.Do panics	1 week ago
Chris Lu	d970c15d71	fix: prevent filer.backup stall in single-filer setups (#7695) * fix: prevent filer.backup stall in single-filer setups (#4977) When MetaAggregator.MetaLogBuffer is empty (which happens in single-filer setups with no peers), ReadFromBuffer was returning nil error, causing LoopProcessLogData to enter an infinite wait loop on ListenersCond. This fix returns ResumeFromDiskError instead, allowing SubscribeMetadata to loop back and read from persisted logs on disk. This ensures filer.backup continues processing events even when the in-memory aggregator buffer is empty. Fixes #4977 * test: add integration tests for metadata subscription Add integration tests for metadata subscription functionality: - TestMetadataSubscribeBasic: Tests basic subscription and event receiving - TestMetadataSubscribeSingleFilerNoStall: Regression test for #4977, verifies subscription doesn't stall under high load in single-filer setups - TestMetadataSubscribeResumeFromDisk: Tests resuming subscription from disk Related to #4977 * ci: add GitHub Actions workflow for metadata subscribe tests Add CI workflow that runs on: - Push/PR to master affecting filer, log_buffer, or metadata subscribe code - Runs the integration tests for metadata subscription - Uploads logs on failure for debugging Related to #4977 * fix: use multipart form-data for file uploads in integration tests The filer expects multipart/form-data for file uploads, not raw POST body. This fixes the 'Content-Type isn't multipart/form-data' error. * test: use -peers=none for faster master startup * test: add -peers=none to remaining master startup in ec tests * fix: use filer HTTP port 8888, WithFilerClient adds 10000 for gRPC WithFilerClient calls ToGrpcAddress() which adds 10000 to the port. Passing 18888 resulted in connecting to 28888. Use 8888 instead. * test: add concurrent writes and million updates tests - TestMetadataSubscribeConcurrentWrites: 50 goroutines writing 20 files each - TestMetadataSubscribeMillionUpdates: 1 million metadata entries via gRPC (metadata only, no actual file content for speed) * fix: address PR review comments - Handle os.MkdirAll errors explicitly instead of ignoring - Handle log file creation errors with proper error messages - Replace silent event dropping with 100ms timeout and warning log * Update metadata_subscribe_integration_test.go	2 weeks ago
Chris Lu	d5f21fd8ba	fix: add missing backslash for volume extraArgs in helm chart (#7676) Fixes #7467 The -mserver argument line in volume-statefulset.yaml was missing a trailing backslash, which prevented extraArgs from being passed to the weed volume process. Also: - Extracted master server list generation logic into shared helper templates in _helpers.tpl for better maintainability - Updated all occurrences of deprecated -mserver flag to -master across docker-compose files, test files, and documentation	2 weeks ago
dependabot[bot]	51c630b5ff	chore(deps): bump golang.org/x/sync from 0.18.0 to 0.19.0 (#7664) * chore(deps): bump golang.org/x/sync from 0.18.0 to 0.19.0 Bumps [golang.org/x/sync](https://github.com/golang/sync) from 0.18.0 to 0.19.0. - [Commits](https://github.com/golang/sync/compare/v0.18.0...v0.19.0) --- updated-dependencies: - dependency-name: golang.org/x/sync dependency-version: 0.19.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chrislu <chris.lu@gmail.com>	2 weeks ago
dependabot[bot]	4709dbf4f8	chore(deps): bump github.com/klauspost/reedsolomon from 1.12.5 to 1.12.6 (#7662) * chore(deps): bump github.com/klauspost/reedsolomon from 1.12.5 to 1.12.6 Bumps [github.com/klauspost/reedsolomon](https://github.com/klauspost/reedsolomon) from 1.12.5 to 1.12.6. - [Release notes](https://github.com/klauspost/reedsolomon/releases) - [Commits](https://github.com/klauspost/reedsolomon/compare/v1.12.5...v1.12.6) --- updated-dependencies: - dependency-name: github.com/klauspost/reedsolomon dependency-version: 1.12.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chrislu <chris.lu@gmail.com>	2 weeks ago
chrislu	5167bbd2a9	Remove deprecated allowEmptyFolder CLI option The allowEmptyFolder option is no longer functional because: 1. The code that used it was already commented out 2. Empty folder cleanup is now handled asynchronously by EmptyFolderCleaner The CLI flags are kept for backward compatibility but marked as deprecated and ignored. This removes: - S3ApiServerOption.AllowEmptyFolder field - The actual usage in s3api_object_handlers_list.go - Helm chart values and template references - References in test Makefiles and docker-compose files	2 weeks ago
Chris Lu	55f0fbf364	s3: optimize DELETE by skipping lock check for buckets without Object Lock (#7642) This optimization avoids an expensive filer gRPC call for every DELETE operation on buckets that don't have Object Lock enabled. Before this change, enforceObjectLockProtections() would always call getObjectEntry() to fetch object metadata to check for retention/legal hold, even for buckets that never had Object Lock configured. Changes: 1. Add early return in enforceObjectLockProtections() if bucket has no Object Lock config or bucket doesn't exist 2. Add isObjectLockEnabled() helper function to check if a bucket has Object Lock configured 3. Fix validateObjectLockHeaders() to check ObjectLockConfig instead of just versioningEnabled - this ensures object-lock headers are properly rejected on buckets without Object Lock enabled, which aligns with AWS S3 semantics 4. Make bucket creation with Object Lock atomic - set Object Lock config in the same CreateEntry call as bucket creation, preventing race conditions where bucket exists without Object Lock enabled 5. Properly handle Object Lock setup failures during bucket creation - if StoreObjectLockConfigurationInExtended fails, roll back the bucket creation and return an error instead of leaving a bucket without the requested Object Lock configuration This significantly improves DELETE latency for non-Object-Lock buckets, which is the common case (lockCheck time reduced from 1-10ms to ~1µs).	2 weeks ago
chrislu	8d110b29dd	fmt	2 weeks ago
Chris Lu	e361daa754	fix: SFTP HomeDir path translation for user operations (#7611) * fix: SFTP HomeDir path translation for user operations When users have a non-root HomeDir (e.g., '/sftp/user'), their SFTP operations should be relative to that directory. Previously, when a user uploaded to '/' via SFTP, the path was not translated to their home directory, causing 'permission denied for / for permission write'. This fix adds a toAbsolutePath() method that implements chroot-like behavior where the user's HomeDir becomes their root. All file and directory operations now translate paths through this method. Example: User with HomeDir='/sftp/user' uploading to '/' now correctly maps to '/sftp/user'. Fixes: https://github.com/seaweedfs/seaweedfs/issues/7470 * test: add SFTP integration tests Add comprehensive integration tests for the SFTP server including: - HomeDir path translation tests (verifies fix for issue #7470) - Basic file upload/download operations - Directory operations (mkdir, rmdir, list) - Large file handling (1MB test) - File rename operations - Stat/Lstat operations - Path edge cases (trailing slashes, .., unicode filenames) - Admin root access verification The test framework starts a complete SeaweedFS cluster with: - Master server - Volume server - Filer server - SFTP server with test user credentials Test users are configured in testdata/userstore.json: - admin: HomeDir=/ with full access - testuser: HomeDir=/sftp/testuser with access to home - readonly: HomeDir=/public with read-only access * fix: correct SFTP HomeDir path translation and add CI Fix path.Join issue where paths starting with '/' weren't joined correctly. path.Join('/sftp/user', '/file') returns '/file' instead of '/sftp/user/file'. Now we strip the leading '/' before joining. Test improvements: - Update go.mod to Go 1.24 - Fix weed binary discovery to prefer local build over PATH - Add stabilization delay after service startup - All 8 SFTP integration tests pass locally Add GitHub Actions workflow for SFTP tests: - Runs on push/PR affecting sftpd code or tests - Tests HomeDir path translation, file ops, directory ops - Covers issue #7470 fix verification * security: update golang.org/x/crypto to v0.45.0 Addresses security vulnerability in golang.org/x/crypto < 0.45.0 * security: use proper SSH host key verification in tests Replace ssh.InsecureIgnoreHostKey() with ssh.FixedHostKey() that verifies the server's host key matches the known test key we generated. This addresses CodeQL warning go/insecure-hostkeycallback. Also updates go.mod to specify go 1.24.0 explicitly. * security: fix path traversal vulnerability in SFTP toAbsolutePath The previous implementation had a critical security vulnerability: - Path traversal via '../..' could escape the HomeDir chroot jail - Absolute paths were not correctly prefixed with HomeDir The fix: 1. Concatenate HomeDir with userPath directly, then clean 2. Add security check to ensure final path stays within HomeDir 3. If traversal detected, safely return HomeDir instead Also adds path traversal prevention tests to verify the fix. * fix: address PR review comments 1. Fix SkipCleanup check to use actual test config instead of default - Added skipCleanup field to SftpTestFramework struct - Store config.SkipCleanup during Setup() - Use f.skipCleanup in Cleanup() instead of DefaultTestConfig() 2. Fix path prefix check false positive in mkdir - Changed from strings.HasPrefix(absPath, fs.user.HomeDir) - To: absPath == fs.user.HomeDir || strings.HasPrefix(absPath, fs.user.HomeDir+"/") - Prevents matching partial directory names (e.g., /sftp/username when HomeDir is /sftp/user) * fix: check write permission on parent dir for mkdir Aligns makeDir's permission check with newFileWriter for consistency. To create a directory, a user needs write permission on the parent directory, not mkdir permission on the new directory path. * fix: refine SFTP path traversal logic and tests 1. Refine toAbsolutePath: - Use path.Join with strings.TrimPrefix for idiomatic path construction - Return explicit error on path traversal attempt instead of clamping - Updated all call sites to handle the error 2. Add Unit Tests: - Added sftp_server_test.go to verify toAbsolutePath logic - Covers normal paths, root path, and various traversal attempts 3. Update Integration Tests: - Updated PathTraversalPrevention test to reflect that standard SFTP clients sanitize paths before sending. The test now verifies successful containment within the jail rather than blocking (since the server receives a clean path). - The server-side blocking is verified by the new unit tests. 4. Makefile: - Removed -v from default test target * fix: address PR comments on tests and makefile 1. Enhanced Unit Tests: - Added edge cases (empty path, multiple slashes, trailing slash) to sftp_server_test.go 2. Makefile Improvements: - Added 'all' target as default entry point 3. Code Clarity: - Added comment to mkdir permission check explaining defensive nature of HomeDir check * fix: address PR review comments on permissions and tests 1. Security: - Added write permission check on target directory in renameEntry 2. Logging: - Changed dispatch log verbosity from V(0) to V(1) 3. Testing: - Updated Makefile .PHONY targets - Added unit test cases for empty/root HomeDir behavior in toAbsolutePath * fix: set SFTP starting directory to virtual root 1. Critical Fix: - Changed sftp.WithStartDirectory from fs.user.HomeDir to '/' - Prevents double-prefixing when toAbsolutePath translates paths - Users now correctly start at their virtual root which maps to HomeDir 2. Test Improvements: - Use pointer for homeDir in tests for clearer nil vs empty distinction * fix: clean HomeDir at config load time Clean HomeDir path when loading users from JSON config. This handles trailing slashes and other path anomalies at the source, ensuring consistency throughout the codebase and avoiding repeated cleaning on every toAbsolutePath call. * test: strengthen assertions and add error checking in SFTP tests 1. Add error checking for cleanup operations in TestWalk 2. Strengthen cwd assertion to expect '/' explicitly in TestCurrentWorkingDirectory 3. Add error checking for cleanup in PathTraversalPrevention test	3 weeks ago
Chris Lu	4f038820dc	Add disk-aware EC rebalancing (#7597) * Add placement package for EC shard placement logic - Consolidate EC shard placement algorithm for reuse across shell and worker tasks - Support multi-pass selection: racks, then servers, then disks - Include proper spread verification and scoring functions - Comprehensive test coverage for various cluster topologies * Make ec.balance disk-aware for multi-disk servers - Add EcDisk struct to track individual disks on volume servers - Update EcNode to maintain per-disk shard distribution - Parse disk_id from EC shard information during topology collection - Implement pickBestDiskOnNode() for selecting best disk per shard - Add diskDistributionScore() for tie-breaking node selection - Update all move operations to specify target disk in RPC calls - Improves shard balance within multi-disk servers, not just across servers * Use placement package in EC detection for consistent disk-level placement - Replace custom EC disk selection logic with shared placement package - Convert topology DiskInfo to placement.DiskCandidate format - Use SelectDestinations() for multi-rack/server/disk spreading - Convert placement results back to topology DiskInfo for task creation - Ensures EC detection uses same placement logic as shell commands * Make volume server evacuation disk-aware - Use pickBestDiskOnNode() when selecting evacuation target disk - Specify target disk in evacuation RPC requests - Maintains balanced disk distribution during server evacuations * Rename PlacementConfig to PlacementRequest for clarity PlacementRequest better reflects that this is a request for placement rather than a configuration object. This improves API semantics. * Rename DefaultConfig to DefaultPlacementRequest Aligns with the PlacementRequest type naming for consistency * Address review comments from Gemini and CodeRabbit Fix HIGH issues: - Fix empty disk discovery: Now discovers all disks from VolumeInfos, not just from EC shards. This ensures disks without EC shards are still considered for placement. - Fix EC shard count calculation in detection.go: Now correctly filters by DiskId and sums actual shard counts using ShardBits.ShardIdCount() instead of just counting EcShardInfo entries. Fix MEDIUM issues: - Add disk ID to evacuation log messages for consistency with other logging - Remove unused serverToDisks variable in placement.go - Fix comment that incorrectly said 'ascending' when sorting is 'descending' * add ec tests * Update ec-integration-tests.yml * Update ec_integration_test.go * Fix EC integration tests CI: build weed binary and update actions - Add 'Build weed binary' step before running tests - Update actions/setup-go from v4 to v6 (Node20 compatibility) - Update actions/checkout from v2 to v4 (Node20 compatibility) - Move working-directory to test step only * Add disk-aware EC rebalancing integration tests - Add TestDiskAwareECRebalancing test with multi-disk cluster setup - Test EC encode with disk awareness (shows disk ID in output) - Test EC balance with disk-level shard distribution - Add helper functions for disk-level verification: - startMultiDiskCluster: 3 servers x 4 disks each - countShardsPerDisk: track shards per disk per server - calculateDiskShardVariance: measure distribution balance - Verify no single disk is overloaded with shards	3 weeks ago
Chris Lu	733ca8e6df	Fix SSE-S3 copy: preserve encryption metadata and set chunk SSE type (#7598) * Fix SSE-S3 copy: preserve encryption metadata and set chunk SSE type Fixes GitHub #7562: Copying objects between encrypted buckets was failing. Root causes: 1. processMetadataBytes was re-adding SSE headers from source entry, undoing the encryption header filtering. Now uses dstEntry.Extended which is already filtered. 2. SSE-S3 streaming copy returned nil metadata. Now properly generates and returns SSE-S3 destination metadata (SeaweedFSSSES3Key, AES256 header) via ExecuteStreamingCopyWithMetadata. 3. Chunks created during streaming copy didn't have SseType set. Now sets SseType and per-chunk SseMetadata with chunk-specific IVs for SSE-S3, enabling proper decryption on GetObject. * Address review: make SSE-S3 metadata serialization failures fatal errors - In executeEncryptCopy: return error instead of just logging if SerializeSSES3Metadata fails - In createChunkFromData: return error if chunk SSE-S3 metadata serialization fails This ensures objects/chunks are never created without proper encryption metadata, preventing unreadable/corrupted data. * fmt * Refactor: reuse function names instead of creating WithMetadata variants - Change ExecuteStreamingCopy to return (*EncryptionSpec, error) directly - Remove ExecuteStreamingCopyWithMetadata wrapper - Change executeStreamingReencryptCopy to return (*EncryptionSpec, error) - Remove executeStreamingReencryptCopyWithMetadata wrapper - Update callers to ignore encryption spec with _ where not needed * Add TODO documenting large file SSE-S3 copy limitation The streaming copy approach encrypts the entire stream with a single IV but stores data in chunks with per-chunk IVs. This causes decryption issues for large files. Small inline files work correctly. This is a known architectural issue that needs separate work to fix. * Use chunk-by-chunk encryption for SSE-S3 copy (consistent with SSE-C/SSE-KMS) Instead of streaming encryption (which had IV mismatch issues for multi-chunk files), SSE-S3 now uses the same chunk-by-chunk approach as SSE-C and SSE-KMS: 1. Extended copyMultipartCrossEncryption to handle SSE-S3: - Added SSE-S3 source decryption in copyCrossEncryptionChunk - Added SSE-S3 destination encryption with per-chunk IVs - Added object-level metadata generation for SSE-S3 destinations 2. Updated routing in executeEncryptCopy/executeDecryptCopy/executeReencryptCopy to use copyMultipartCrossEncryption for all SSE-S3 scenarios 3. Removed streaming copy functions (shouldUseStreamingCopy, executeStreamingReencryptCopy) as they're no longer used 4. Added large file (1MB) integration test to verify chunk-by-chunk copy works This ensures consistent behavior across all SSE types and fixes data corruption that occurred with large files in the streaming copy approach. * fmt * fmt * Address review: fail explicitly if SSE-S3 metadata is missing Instead of silently ignoring missing SSE-S3 metadata (which could create unreadable objects), now explicitly fail the copy operation with a clear error message if: - First chunk is missing - First chunk doesn't have SSE-S3 type - First chunk has empty SSE metadata - Deserialization fails * Address review: improve comment to reflect full scope of chunk creation * Address review: fail explicitly if baseIV is empty for SSE-S3 chunk encryption If DestinationIV is not set when encrypting SSE-S3 chunks, the chunk would be created without SseMetadata, causing GetObject decryption to fail later. Now fails explicitly with a clear error message. Note: calculateIVWithOffset returns ([]byte, int) not ([]byte, error) - the int is a skip amount for intra-block alignment, not an error code. * Address review: handle 0-byte files in SSE-S3 copy For 0-byte files, there are no chunks to get metadata from. Generate an IV for the object-level metadata to ensure even empty files are properly marked as SSE-S3 encrypted. Also validate that we don't have a non-empty file with no chunks (which would indicate an internal error).	3 weeks ago
Chris Lu	208d008fe3	Fix tagging test pattern to run our comprehensive tests instead of basic tests	3 weeks ago
Chris Lu	ec41795594	Update s3-tagging-tests to use Makefile server management like other S3 tests	3 weeks ago
Chris Lu	a33e5a9e6a	Add S3 object tagging tests to CI workflow - Modified test/s3/tagging/s3_tagging_test.go to use environment variables for configurable endpoint and credentials - Added s3-tagging-tests job to .github/workflows/s3-go-tests.yml to run tagging tests in CI - Tests will now run automatically on pull requests	3 weeks ago
Chris Lu	8c585a9682	Fix S3 object tagging issue #7589 - Add X-Amz-Tagging header parsing in putToFiler function for PUT object operations - Store tags with X-Amz-Tagging- prefix in entry.Extended metadata - Add comprehensive test suite for S3 object tagging functionality - Tests cover upload tagging, API operations, special characters, and edge cases	3 weeks ago
dependabot[bot]	5602f98c47	chore(deps): bump github.com/prometheus/procfs from 0.19.1 to 0.19.2 (#7577) * chore(deps): bump github.com/prometheus/procfs from 0.19.1 to 0.19.2 Bumps [github.com/prometheus/procfs](https://github.com/prometheus/procfs) from 0.19.1 to 0.19.2. - [Release notes](https://github.com/prometheus/procfs/releases) - [Commits](https://github.com/prometheus/procfs/compare/v0.19.1...v0.19.2) --- updated-dependencies: - dependency-name: github.com/prometheus/procfs dependency-version: 0.19.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * go mod --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	3 weeks ago
dependabot[bot]	8a6fcc1df9	chore(deps): bump github.com/klauspost/compress from 1.18.1 to 1.18.2 (#7576) * chore(deps): bump github.com/klauspost/compress from 1.18.1 to 1.18.2 Bumps [github.com/klauspost/compress](https://github.com/klauspost/compress) from 1.18.1 to 1.18.2. - [Release notes](https://github.com/klauspost/compress/releases) - [Commits](https://github.com/klauspost/compress/compare/v1.18.1...v1.18.2) --- updated-dependencies: - dependency-name: github.com/klauspost/compress dependency-version: 1.18.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * go mod --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	3 weeks ago
dependabot[bot]	ab222709e3	chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.10 to 4.25.11 (#7579) * chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.10 to 4.25.11 Bumps [github.com/shirou/gopsutil/v4](https://github.com/shirou/gopsutil) from 4.25.10 to 4.25.11. - [Release notes](https://github.com/shirou/gopsutil/releases) - [Commits](https://github.com/shirou/gopsutil/compare/v4.25.10...v4.25.11) --- updated-dependencies: - dependency-name: github.com/shirou/gopsutil/v4 dependency-version: 4.25.11 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * go mod --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	3 weeks ago
Chris Lu	bd419fda51	fix: copy to bucket with default SSE-S3 encryption fails (#7562) (#7568) * filer use context without cancellation * pass along context * fix: copy to bucket with default SSE-S3 encryption fails (#7562) When copying an object from an encrypted bucket to a temporary unencrypted bucket, then to another bucket with default SSE-S3 encryption, the operation fails with 'invalid SSE-S3 source key type' error. Root cause: When objects are copied from an SSE-S3 encrypted bucket to an unencrypted bucket, the 'X-Amz-Server-Side-Encryption: AES256' header is preserved but the actual encryption key (SeaweedFSSSES3Key) is stripped. This creates an 'orphaned' SSE-S3 header that causes IsSSES3EncryptedInternal() to return true, triggering decryption logic with a nil key. Fix: 1. Modified IsSSES3EncryptedInternal() to require BOTH the AES256 header AND the SeaweedFSSSES3Key to be present before returning true 2. Added isOrphanedSSES3Header() to detect orphaned SSE-S3 headers 3. Updated copy handler to strip orphaned headers during copy operations Fixes #7562 * fmt * refactor: simplify isOrphanedSSES3Header function logic Remove redundant existence check since the caller iterates through metadata map, making the check unnecessary. Improves readability while maintaining the same functionality.	3 weeks ago
Chris Lu	865fc88b3b	java 4.00	3 weeks ago
tam-i13	b669607fcd	Add error list each entry func (#7485) * added error return in type ListEachEntryFunc * return error if errClose * fix fmt.Errorf * fix return errClose * use %w fmt.Errorf * added entry in messege error * add callbackErr in ListDirectoryEntries * fix error * add log * clear err when the scanner stops on io.EOF, so returning err doesn’t surface EOF as a failure. * more info in error * add ctx to logs, error handling * fix return eachEntryFunc * fix * fix log * fix return * fix foundationdb test s * fix eachEntryFunc * fix return resEachEntryFuncErr * Update weed/filer/filer.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update weed/filer/elastic/v7/elastic_store.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update weed/filer/hbase/hbase_store.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update weed/filer/foundationdb/foundationdb_store.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update weed/filer/ydb/ydb_store.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix * add scanErr --------- Co-authored-by: Roman Tamarov <r.tamarov@kryptonite.ru> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: chrislu <chris.lu@gmail.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	4 weeks ago
dependabot[bot]	2712a0daeb	chore(deps): bump github.com/linkedin/goavro/v2 from 2.14.0 to 2.14.1 (#7537) * chore(deps): bump github.com/linkedin/goavro/v2 from 2.14.0 to 2.14.1 Bumps [github.com/linkedin/goavro/v2](https://github.com/linkedin/goavro) from 2.14.0 to 2.14.1. - [Release notes](https://github.com/linkedin/goavro/releases) - [Changelog](https://github.com/linkedin/goavro/blob/master/debug_release.go) - [Commits](https://github.com/linkedin/goavro/compare/v2.14.0...v2.14.1) --- updated-dependencies: - dependency-name: github.com/linkedin/goavro/v2 dependency-version: 2.14.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * go mod tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chrislu <chris.lu@gmail.com>	4 weeks ago
Chris Lu	9f413de6a9	HDFS: Java client replication configuration (#7526) * more flexible replication configuration * remove hdfs-over-ftp * Fix keepalive mismatch * NPE * grpc-java 1.75.0 → 1.77.0 * grpc-go 1.75.1 → 1.77.0 * Retry logic * Connection pooling, HTTP/2 tuning, keepalive * Complete Spark integration test suite * CI/CD workflow * Update dependency-reduced-pom.xml * add comments * docker compose * build clients * go mod tidy * fix building * mod * java: fix NPE in SeaweedWrite and Makefile env var scope - Add null check for HttpEntity in SeaweedWrite.multipartUpload() to prevent NPE when response.getEntity() returns null - Fix Makefile test target to properly export SEAWEEDFS_TEST_ENABLED by setting it on the same command line as mvn test - Update docker-compose commands to use V2 syntax (docker compose) for consistency with GitHub Actions workflow * spark: update compiler source/target from Java 8 to Java 11 - Fix inconsistency between maven.compiler.source/target (1.8) and surefire JVM args (Java 9+ module flags like --add-opens) - Update to Java 11 to match CI environment (GitHub Actions uses Java 11) - Docker environment uses Java 17 which is also compatible - Java 11+ is required for the --add-opens/--add-exports flags used in the surefire configuration * spark: fix flaky test by sorting DataFrame before first() - In testLargeDataset(), add orderBy("value") before calling first() - Parquet files don't guarantee row order, so first() on unordered DataFrame can return any row, making assertions flaky - Sorting by 'value' ensures the first row is always the one with value=0, making the test deterministic and reliable * ci: refactor Spark workflow for DRY and robustness 1. Add explicit permissions (least privilege): - contents: read - checks: write (for test reports) - pull-requests: write (for PR comments) 2. Extract duplicate build steps into shared 'build-deps' job: - Eliminates duplication between spark-tests and spark-example - Build artifacts are uploaded and reused by dependent jobs - Reduces CI time and ensures consistency 3. Fix spark-example service startup verification: - Match robust approach from spark-tests job - Add explicit timeout and failure handling - Verify all services (master, volume, filer) - Include diagnostic logging on failure - Prevents silent failures and obscure errors These changes improve maintainability, security, and reliability of the Spark integration test workflow. * ci: update actions/cache from v3 to v4 - Update deprecated actions/cache@v3 to actions/cache@v4 - Ensures continued support and bug fixes - Cache key and path remain compatible with v4 * ci: fix Maven artifact restoration in workflow - Add step to restore Maven artifacts from download to ~/.m2/repository - Restructure artifact upload to use consistent directory layout - Remove obsolete 'version' field from docker-compose.yml to eliminate warnings - Ensures SeaweedFS Java dependencies are available during test execution * ci: fix SeaweedFS binary permissions after artifact download - Add step to chmod +x the weed binary after downloading artifacts - Artifacts lose executable permissions during upload/download - Prevents 'Permission denied' errors when Docker tries to run the binary * ci: fix artifact download path to avoid checkout conflicts - Download artifacts to 'build-artifacts' directory instead of '.' - Prevents checkout from overwriting downloaded files - Explicitly copy weed binary from build-artifacts to docker/ directory - Update Maven artifact restoration to use new path * fix: add -peers=none to master command for standalone mode - Ensures master runs in standalone single-node mode - Prevents master from trying to form a cluster - Required for proper initialization in test environment * test: improve docker-compose config for Spark tests - Add -volumeSizeLimitMB=50 to master (consistent with other integration tests) - Add -defaultReplication=000 to master for explicit single-copy storage - Add explicit -port and -port.grpc flags to all services - Add -preStopSeconds=1 to volume for faster shutdown - Add healthchecks to master and volume services - Use service_healthy conditions for proper startup ordering - Improve healthcheck intervals and timeouts for faster startup - Use -ip flag instead of -ip.bind for service identity * fix: ensure weed binary is executable in Docker image - Add chmod +x for weed binaries in Dockerfile.local - Artifact upload/download doesn't preserve executable permissions - Ensures binaries are executable regardless of source file permissions * refactor: remove unused imports in FilerGrpcClient - Remove unused io.grpc.Deadline import - Remove unused io.netty.handler.codec.http2.Http2Settings import - Clean up linter warnings * refactor: eliminate code duplication in channel creation - Extract common gRPC channel configuration to createChannelBuilder() method - Reduce code duplication from 3 branches to single configuration - Improve maintainability by centralizing channel settings - Add Javadoc for the new helper method * fix: align maven-compiler-plugin with compiler properties - Change compiler plugin source/target from hardcoded 1.8 to use properties - Ensures consistency with maven.compiler.source/target set to 11 - Prevents version mismatch between properties and plugin configuration - Aligns with surefire Java 9+ module arguments * fix: improve binary copy and chmod in Dockerfile - Copy weed binary explicitly to /usr/bin/weed - Run chmod +x immediately after COPY to ensure executable - Add ls -la to verify binary exists and has correct permissions - Make weed_pub* and weed_sub* copies optional with || true - Simplify RUN commands for better layer caching * fix: remove invalid shell operators from Dockerfile COPY - Remove '|| true' from COPY commands (not supported in Dockerfile) - Remove optional weed_pub* and weed_sub* copies (not needed for tests) - Simplify Dockerfile to only copy required files - Keep chmod +x and ls -la verification for main binary * ci: add debugging and force rebuild of Docker images - Add ls -la to show build-artifacts/docker/ contents - Add file command to verify binary type - Add --no-cache to docker compose build to prevent stale cache issues - Ensures fresh build with current binary * ci: add comprehensive failure diagnostics - Add container status (docker compose ps -a) on startup failure - Add detailed logs for all three services (master, volume, filer) - Add container inspection to verify binary exists - Add debugging info for spark-example job - Helps diagnose startup failures before containers are torn down * fix: build statically linked binary for Alpine Linux - Add CGO_ENABLED=0 to go build command - Creates statically linked binary compatible with Alpine (musl libc) - Fixes 'not found' error caused by missing glibc dynamic linker - Add file command to verify static linking in build output * security: add dependencyManagement to fix vulnerable transitives - Pin Jackson to 2.15.3 (fixes multiple CVEs in older versions) - Pin Netty to 4.1.100.Final (fixes CVEs in transport/codec) - Pin Apache Avro to 1.11.4 (fixes deserialization CVEs) - Pin Apache ZooKeeper to 3.9.1 (fixes authentication bypass) - Pin commons-compress to 1.26.0 (fixes zip slip vulnerabilities) - Pin commons-io to 2.15.1 (fixes path traversal) - Pin Guava to 32.1.3-jre (fixes temp directory vulnerabilities) - Pin SnakeYAML to 2.2 (fixes arbitrary code execution) - Pin Jetty to 9.4.53 (fixes multiple HTTP vulnerabilities) - Overrides vulnerable versions from Spark/Hadoop transitives * refactor: externalize seaweedfs-hadoop3-client version to property - Add seaweedfs.hadoop3.client.version property set to 3.80 - Replace hardcoded version with ${seaweedfs.hadoop3.client.version} - Enables easier version management from single location - Follows Maven best practices for dependency versioning * refactor: extract surefire JVM args to property - Move multi-line argLine to surefire.jvm.args property - Reference property in argLine for cleaner configuration - Improves maintainability and readability - Follows Maven best practices for JVM argument management - Avoids potential whitespace parsing issues * fix: add publicUrl to volume server for host network access - Add -publicUrl=localhost:8080 to volume server command - Ensures filer returns localhost URL instead of Docker service name - Fixes UnknownHostException when tests run on host network - Volume server is accessible via localhost from CI runner * security: upgrade Netty to 4.1.115.Final to fix CVE - Upgrade netty.version from 4.1.100.Final to 4.1.115.Final - Fixes GHSA-prj3-ccx8-p6x4: MadeYouReset HTTP/2 DDoS vulnerability - Netty 4.1.115.Final includes patches for high severity DoS attack - Addresses GitHub dependency review security alert * fix: suppress verbose Parquet DEBUG logging - Set org.apache.parquet to WARN level - Set org.apache.parquet.io to ERROR level - Suppress RecordConsumerLoggingWrapper and MessageColumnIO DEBUG logs - Reduces CI log noise from thousands of record-level messages - Keeps important error messages visible * fix: use 127.0.0.1 for volume server IP registration - Change volume -ip from seaweedfs-volume to 127.0.0.1 - Change -publicUrl from localhost:8080 to 127.0.0.1:8080 - Volume server now registers with master using 127.0.0.1 - Filer will return 127.0.0.1:8080 URL that's resolvable from host - Fixes UnknownHostException for seaweedfs-volume hostname * security: upgrade Netty to 4.1.118.Final - Upgrade from 4.1.115.Final to 4.1.118.Final - Fixes CVE-2025-24970: improper validation in SslHandler - Fixes CVE-2024-47535: unsafe environment file reading on Windows - Fixes CVE-2024-29025: HttpPostRequestDecoder resource exhaustion - Addresses GHSA-prj3-ccx8-p6x4 and related vulnerabilities * security: upgrade Netty to 4.1.124.Final (patched version) - Upgrade from 4.1.118.Final to 4.1.124.Final - Fixes GHSA-prj3-ccx8-p6x4: MadeYouReset HTTP/2 DDoS vulnerability - 4.1.124.Final is the confirmed patched version per GitHub advisory - All versions <= 4.1.123.Final are vulnerable * ci: skip central-publishing plugin during build - Add -Dcentral.publishing.skip=true to all Maven builds - Central publishing plugin is only needed for Maven Central releases - Prevents plugin resolution errors during CI builds - Complements existing -Dgpg.skip=true flag * fix: aggressively suppress Parquet DEBUG logging - Set Parquet I/O loggers to OFF (completely disabled) - Add log4j.configuration system property to ensure config is used - Override Spark's default log4j configuration - Prevents thousands of record-level DEBUG messages in CI logs * security: upgrade Apache ZooKeeper to 3.9.3 - Upgrade from 3.9.1 to 3.9.3 - Fixes GHSA-g93m-8x6h-g5gv: Authentication bypass in Admin Server - Fixes GHSA-r978-9m6m-6gm6: Information disclosure in persistent watchers - Fixes GHSA-2hmj-97jw-28jh: Insufficient permission check in snapshot/restore - Addresses high and moderate severity vulnerabilities * security: upgrade Apache ZooKeeper to 3.9.4 - Upgrade from 3.9.3 to 3.9.4 (latest stable) - Ensures all known security vulnerabilities are patched - Fixes GHSA-g93m-8x6h-g5gv, GHSA-r978-9m6m-6gm6, GHSA-2hmj-97jw-28jh * fix: add -max=0 to volume server for unlimited volumes - Add -max=0 flag to volume server command - Allows volume server to create unlimited 50MB volumes - Fixes 'No writable volumes' error during Spark tests - Volume server will create new volumes as needed for writes - Consistent with other integration test configurations * security: upgrade Jetty from 9.4.53 to 12.0.16 - Upgrade from 9.4.53.v20231009 to 12.0.16 (meets requirement >12.0.9) - Addresses security vulnerabilities in older Jetty versions - Externalized version to jetty.version property for easier maintenance - Added jetty-util, jetty-io, jetty-security to dependencyManagement - Ensures all Jetty transitive dependencies use secure version * fix: add persistent volume data directory for volume server - Add -dir=/data flag to volume server command - Mount Docker volume seaweedfs-volume-data to /data - Ensures volume server has persistent storage for volume files - Fixes issue where volume server couldn't create writable volumes - Volume data persists across container restarts during tests * fmt * fix: remove Jetty dependency management due to unavailable versions - Jetty 12.0.x versions greater than 12.0.9 do not exist in Maven Central - Attempted 12.0.10, 12.0.12, 12.0.16 - none are available - Next available versions are in 12.1.x series - Remove Jetty dependency management to rely on transitive resolution - Allows build to proceed with Jetty versions from Spark/Hadoop dependencies - Can revisit with explicit version pinning if CVE concerns arise * 4.1.125.Final * fix: restore Jetty dependency management with version 12.0.12 - Restore explicit Jetty version management in dependencyManagement - Pin Jetty 12.0.12 for transitive dependencies from Spark/Hadoop - Remove misleading comment about Jetty versions availability - Include jetty-server, jetty-http, jetty-servlet, jetty-util, jetty-io, jetty-security - Use jetty.version property for consistency across all Jetty artifacts - Update Netty to 4.1.125.Final (latest security patch) * security: add dependency overrides for vulnerable transitive deps - Add commons-beanutils 1.11.0 (fixes CVE in 1.9.4) - Add protobuf-java 3.25.5 (compatible with Spark/Hadoop ecosystem) - Add nimbus-jose-jwt 9.37.2 (minimum secure version) - Add snappy-java 1.1.10.4 (fixes compression vulnerabilities) - Add dnsjava 3.6.0 (fixes DNS security issues) All dependencies are pulled transitively from Hadoop/Spark: - commons-beanutils: hadoop-common - protobuf-java: hadoop-common - nimbus-jose-jwt: hadoop-auth - snappy-java: spark-core - dnsjava: hadoop-common Verified with mvn dependency:tree that overrides are applied correctly. * security: upgrade nimbus-jose-jwt to 9.37.4 (patched version) - Update from 9.37.2 to 9.37.4 to address CVE - 9.37.2 is vulnerable, 9.37.4 is the patched version for 9.x line - Verified with mvn dependency:tree that override is applied * Update pom.xml * security: upgrade nimbus-jose-jwt to 10.0.2 to fix GHSA-xwmg-2g98-w7v9 - Update nimbus-jose-jwt from 9.37.4 to 10.0.2 - Fixes CVE: GHSA-xwmg-2g98-w7v9 (DoS via deeply nested JSON) - 9.38.0 doesn't exist in Maven Central; 10.0.2 is the patched version - Remove Jetty dependency management (12.0.12 doesn't exist) - Verified with mvn -U clean verify that all dependencies resolve correctly - Build succeeds with all security patches applied * ci: add volume cleanup and verification steps - Add 'docker compose down -v' before starting services to clean up stale volumes - Prevents accumulation of data/buckets from previous test runs - Add volume registration verification after service startup - Check that volume server has registered with master and volumes are available - Helps diagnose 'No writable volumes' errors - Shows volume count and waits up to 30 seconds for volumes to be created - Both spark-tests and spark-example jobs updated with same improvements * ci: add volume.list diagnostic for troubleshooting 'No writable volumes' - Add 'weed shell' execution to run 'volume.list' on failure - Shows which volumes exist, their status, and available space - Add cluster status JSON output for detailed topology view - Helps diagnose volume allocation issues and full volumes - Added to both spark-tests and spark-example jobs - Diagnostic runs only when tests fail (if: failure()) * fix: force volume creation before tests to prevent 'No writable volumes' error Root cause: With -max=0 (unlimited volumes), volumes are created on-demand, but no volumes existed when tests started, causing first write to fail. Solution: - Explicitly trigger volume growth via /vol/grow API - Create 3 volumes with replication=000 before running tests - Verify volumes exist before proceeding - Fail early with clear message if volumes can't be created Changes: - POST to http://localhost:9333/vol/grow?replication=000&count=3 - Wait up to 10 seconds for volumes to appear - Show volume count and layout status - Exit with error if no volumes after 10 attempts - Applied to both spark-tests and spark-example jobs This ensures writable volumes exist before Spark tries to write data. * fix: use container hostname for volume server to enable automatic volume creation Root cause identified: - Volume server was using -ip=127.0.0.1 - Master couldn't reach volume server at 127.0.0.1 from its container - When Spark requested assignment, master tried to create volume via gRPC - Master's gRPC call to 127.0.0.1:18080 failed (reached itself, not volume server) - Result: 'No writable volumes' error Solution: - Change volume server to use -ip=seaweedfs-volume (container hostname) - Master can now reach volume server at seaweedfs-volume:18080 - Automatic volume creation works as designed - Kept -publicUrl=127.0.0.1:8080 for external clients (host network) Workflow changes: - Remove forced volume creation (curl POST to /vol/grow) - Volumes will be created automatically on first write request - Keep diagnostic output for troubleshooting - Simplified startup verification This matches how other SeaweedFS tests work with Docker networking. * fix: use localhost publicUrl and -max=100 for host-based Spark tests The previous fix enabled master-to-volume communication but broke client writes. Problem: - Volume server uses -ip=seaweedfs-volume (Docker hostname) - Master can reach it ✓ - Spark tests run on HOST (not in Docker container) - Host can't resolve 'seaweedfs-volume' → UnknownHostException ✗ Solution: - Keep -ip=seaweedfs-volume for master gRPC communication - Change -publicUrl to 'localhost:8080' for host-based clients - Change -max=0 to -max=100 (matches other integration tests) Why -max=100: - Pre-allocates volume capacity at startup - Volumes ready immediately for writes - Consistent with other test configurations - More reliable than on-demand (-max=0) This configuration allows: - Master → Volume: seaweedfs-volume:18080 (Docker network) - Clients → Volume: localhost:8080 (host network via port mapping) * refactor: run Spark tests fully in Docker with bridge network Better approach than mixing host and container networks. Changes to docker-compose.yml: - Remove 'network_mode: host' from spark-tests container - Add spark-tests to seaweedfs-spark bridge network - Update SEAWEEDFS_FILER_HOST from 'localhost' to 'seaweedfs-filer' - Add depends_on to ensure services are healthy before tests - Update volume publicUrl from 'localhost:8080' to 'seaweedfs-volume:8080' Changes to workflow: - Remove separate build and test steps - Run tests via 'docker compose up spark-tests' - Use --abort-on-container-exit and --exit-code-from for proper exit codes - Simpler: one step instead of two Benefits: ✓ All components use Docker DNS (seaweedfs-master, seaweedfs-volume, seaweedfs-filer) ✓ No host/container network split or DNS resolution issues ✓ Consistent with how other SeaweedFS integration tests work ✓ Tests are fully containerized and reproducible ✓ Volume server accessible via seaweedfs-volume:8080 for all clients ✓ Automatic volume creation works (master can reach volume via gRPC) ✓ Data writes work (Spark can reach volume via Docker network) This matches the architecture of other integration tests and is cleaner. * debug: add DNS verification and disable Java DNS caching Troubleshooting 'seaweedfs-volume: Temporary failure in name resolution': docker-compose.yml changes: - Add MAVEN_OPTS to disable Java DNS caching (ttl=0) Java caches DNS lookups which can cause stale results - Add ping tests before mvn test to verify DNS resolution Tests: ping -c 1 seaweedfs-volume && ping -c 1 seaweedfs-filer - This will show if DNS works before tests run workflow changes: - List Docker networks before running tests - Shows network configuration for debugging - Helps verify spark-tests joins correct network If ping succeeds but tests fail, it's a Java/Maven DNS issue. If ping fails, it's a Docker networking configuration issue. Note: Previous test failures may be from old code before Docker networking fix. * fix: add file sync and cache settings to prevent EOF on read Issue: Files written successfully but truncated when read back Error: 'EOFException: Reached the end of stream. Still have: 78 bytes left' Root cause: Potential race condition between write completion and read - File metadata updated before all chunks fully flushed - Spark immediately reads after write without ensuring sync - Parquet reader gets incomplete file Solutions applied: 1. Disable filesystem cache to avoid stale file handles - spark.hadoop.fs.seaweedfs.impl.disable.cache=true 2. Enable explicit flush/sync on write (if supported by client) - spark.hadoop.fs.seaweed.write.flush.sync=true 3. Add SPARK_SUBMIT_OPTS for cache disabling These settings ensure: - Files are fully flushed before close() returns - No cached file handles with stale metadata - Fresh reads always get current file state Note: If issue persists, may need to add explicit delay between write and read, or investigate seaweedfs-hadoop3-client flush behavior. * fix: remove ping command not available in Maven container The maven:3.9-eclipse-temurin-17 image doesn't include ping utility. DNS resolution was already confirmed working in previous runs. Remove diagnostic ping commands - not needed anymore. * workaround: increase Spark task retries for eventual consistency Issue: EOF exceptions when reading immediately after write - Files appear truncated by ~78 bytes on first read - SeaweedOutputStream.close() does wait for all chunks via Future.get() - But distributed file systems can have eventual consistency delays Workaround: - Increase spark.task.maxFailures from default 1 to 4 - Allows Spark to automatically retry failed read tasks - If file becomes consistent after 1-2 seconds, retry succeeds This is a pragmatic solution for testing. The proper fix would be: 1. Ensure SeaweedOutputStream.close() waits for volume server acknowledgment 2. Or add explicit sync/flush mechanism in SeaweedFS client 3. Or investigate if metadata is updated before data is fully committed For CI tests, automatic retries should mask the consistency delay. * debug: enable detailed logging for SeaweedFS client file operations Enable DEBUG logging for: - SeaweedRead: Shows fileSize calculations from chunks - SeaweedOutputStream: Shows write/flush/close operations - SeaweedInputStream: Shows read operations and content length This will reveal: 1. What file size is calculated from Entry chunks metadata 2. What actual chunk sizes are written 3. If there's a mismatch between metadata and actual data 4. Whether the '78 bytes' missing is consistent pattern Looking for clues about the EOF exception root cause. * debug: add detailed chunk size logging to diagnose EOF issue Added INFO-level logging to track: 1. Every chunk write: offset, size, etag, target URL 2. Metadata update: total chunks count and calculated file size 3. File size calculation: breakdown of chunks size vs attr size This will reveal: - If chunks are being written with correct sizes - If metadata file size matches sum of chunks - If there's a mismatch causing the '78 bytes left' EOF Example output expected: ✓ Wrote chunk to http://volume:8080/3,xxx at offset 0 size 1048576 bytes ✓ Wrote chunk to http://volume:8080/3,yyy at offset 1048576 size 524288 bytes ✓ Writing metadata with 2 chunks, total size: 1572864 bytes Calculated file size: 1572864 (chunks: 1572864, attr: 0, #chunks: 2) If we see size=X in write but size=X-78 in read, that's the smoking gun. * fix: replace deprecated slf4j-log4j12 with slf4j-reload4j Maven warning: 'The artifact org.slf4j:slf4j-log4j12:jar:1.7.36 has been relocated to org.slf4j:slf4j-reload4j:jar:1.7.36' slf4j-log4j12 was replaced by slf4j-reload4j due to log4j vulnerabilities. The reload4j project is a fork of log4j 1.2.17 with security fixes. This is a drop-in replacement with the same API. * debug: add detailed buffer tracking to identify lost 78 bytes Issue: Parquet expects 1338 bytes but SeaweedFS only has 1260 bytes (78 missing) Added logging to track: - Buffer position before every write - Bytes submitted for write - Whether buffer is skipped (position==0) This will show if: 1. The last 78 bytes never entered the buffer (Parquet bug) 2. The buffer had 78 bytes but weren't written (flush bug) 3. The buffer was written but data was lost (volume server bug) Next step: Force rebuild in CI to get these logs. * debug: track position and buffer state at close time Added logging to show: 1. totalPosition: Total bytes ever written to stream 2. buffer.position(): Bytes still in buffer before flush 3. finalPosition: Position after flush completes This will reveal if: - Parquet wrote 1338 bytes → position should be 1338 - Only 1260 bytes reached write() → position would be 1260 - 78 bytes stuck in buffer → buffer.position() would be 78 Expected output: close: path=...parquet totalPosition=1338 buffer.position()=78 → Shows 78 bytes in buffer need flushing OR: close: path=...parquet totalPosition=1260 buffer.position()=0 → Shows Parquet never wrote the 78 bytes! * fix: force Maven clean build to pick up updated Java client JARs Issue: mvn test was using cached compiled classes - Changed command from 'mvn test' to 'mvn clean test' - Forces recompilation of test code - Ensures updated seaweedfs-client JAR with new logging is used This should now show the INFO logs: - close: path=X totalPosition=Y buffer.position()=Z - writeCurrentBufferToService: buffer.position()=X - ✓ Wrote chunk to URL at offset X size Y bytes * fix: force Maven update and verify JAR contains updated code Added -U flag to mvn install to force dependency updates Added verification step using javap to check compiled bytecode This will show if the JAR actually contains the new logging code: - If 'totalPosition' string is found → JAR is updated - If not found → Something is wrong with the build The verification output will help diagnose why INFO logs aren't showing. * fix: use SNAPSHOT version to force Maven to use locally built JARs ROOT CAUSE: Maven was downloading seaweedfs-client:3.80 from Maven Central instead of using the locally built version in CI! Changes: - Changed all versions from 3.80 to 3.80.1-SNAPSHOT - other/java/client/pom.xml: 3.80 → 3.80.1-SNAPSHOT - other/java/hdfs2/pom.xml: property 3.80 → 3.80.1-SNAPSHOT - other/java/hdfs3/pom.xml: property 3.80 → 3.80.1-SNAPSHOT - test/java/spark/pom.xml: property 3.80 → 3.80.1-SNAPSHOT Maven behavior: - Release versions (3.80): Downloaded from remote repos if available - SNAPSHOT versions: Prefer local builds, can be updated This ensures the CI uses the locally built JARs with our debug logging! Also added unique [DEBUG-2024] markers to verify in logs. * fix: use explicit $HOME path for Maven mount and add verification Issue: docker-compose was using ~ which may not expand correctly in CI Changes: 1. docker-compose.yml: Changed ~/.m2 to ${HOME}/.m2 - Ensures proper path expansion in GitHub Actions - $HOME is /home/runner in GitHub Actions runners 2. Added verification step in workflow: - Lists all SNAPSHOT artifacts before tests - Shows what's available in Maven local repo - Will help diagnose if artifacts aren't being restored correctly This should ensure the Maven container can access the locally built 3.80.1-SNAPSHOT JARs with our debug logging code. * fix: copy Maven artifacts into workspace instead of mounting $HOME/.m2 Issue: Docker volume mount from $HOME/.m2 wasn't working in GitHub Actions - Container couldn't access the locally built SNAPSHOT JARs - Maven failed with 'Could not find artifact seaweedfs-hadoop3-client:3.80.1-SNAPSHOT' Solution: Copy Maven repository into workspace 1. In CI: Copy ~/.m2/repository/com/seaweedfs to test/java/spark/.m2/repository/com/ 2. docker-compose.yml: Mount ./.m2 (relative path in workspace) 3. .gitignore: Added .m2/ to ignore copied artifacts Why this works: - Workspace directory (.) is successfully mounted as /workspace - ./.m2 is inside workspace, so it gets mounted too - Container sees artifacts at /root/.m2/repository/com/seaweedfs/... - Maven finds the 3.80.1-SNAPSHOT JARs with our debug logging! Next run should finally show the [DEBUG-2024] logs! 🎯 * debug: add detailed verification for Maven artifact upload The Maven artifacts are not appearing in the downloaded artifacts! Only 'docker' directory is present, '.m2' is missing. Added verification to show: 1. Does ~/.m2/repository/com/seaweedfs exist? 2. What files are being copied? 3. What SNAPSHOT artifacts are in the upload? 4. Full structure of artifacts/ before upload This will reveal if: - Maven install didn't work (artifacts not created) - Copy command failed (wrong path) - Upload excluded .m2 somehow (artifact filter issue) The next run will show exactly where the Maven artifacts are lost! * refactor: merge workflow jobs into single job Benefits: - Eliminates artifact upload/download complexity - Maven artifacts stay in ~/.m2 throughout - Simpler debugging (all logs in one place) - Faster execution (no transfer overhead) - More reliable (no artifact transfer failures) Structure: 1. Build SeaweedFS binary + Java dependencies 2. Run Spark integration tests (Docker) 3. Run Spark example (host-based, push/dispatch only) 4. Upload results & diagnostics Trade-off: Example runs sequentially after tests instead of parallel, but overall runtime is likely faster without artifact transfers. * debug: add critical diagnostics for EOFException (78 bytes missing) The persistent EOFException shows Parquet expects 78 more bytes than exist. This suggests a mismatch between what was written vs what's in chunks. Added logging to track: 1. Buffer state at close (position before flush) 2. Stream position when flushing metadata 3. Chunk count vs file size in attributes 4. Explicit fileSize setting from stream position Key hypothesis: - Parquet writes N bytes total (e.g., 762) - Stream.position tracks all writes - But only (N-78) bytes end up in chunks - This causes Parquet read to fail with 'Still have: 78 bytes left' If buffer.position() = 78 at close, the buffer wasn't flushed. If position != chunk total, write submission failed. If attr.fileSize != position, metadata is inconsistent. Next run will show which scenario is happening. * debug: track stream lifecycle and total bytes written Added comprehensive logging to identify why Parquet files fail with 'EOFException: Still have: 78 bytes left'. Key additions: 1. SeaweedHadoopOutputStream constructor logging with 🔧 marker - Shows when output streams are created - Logs path, position, bufferSize, replication 2. totalBytesWritten counter in SeaweedOutputStream - Tracks cumulative bytes written via write() calls - Helps identify if Parquet wrote 762 bytes but only 684 reached chunks 3. Enhanced close() logging with 🔒 and ✅ markers - Shows totalBytesWritten vs position vs buffer.position() - If totalBytesWritten=762 but position=684, write submission failed - If buffer.position()=78 at close, buffer wasn't flushed Expected scenarios in next run: A) Stream never created → No 🔧 log for .parquet files B) Write failed → totalBytesWritten=762 but position=684 C) Buffer not flushed → buffer.position()=78 at close D) All correct → totalBytesWritten=position=684, but Parquet expects 762 This will pinpoint whether the issue is in: - Stream creation/lifecycle - Write submission - Buffer flushing - Or Parquet's internal state * debug: add getPos() method to track position queries Added getPos() to SeaweedOutputStream to understand when and how Hadoop/Parquet queries the output stream position. Current mystery: - Files are written correctly (totalBytesWritten=position=chunks) - But Parquet expects 78 more bytes when reading - year=2020: wrote 696, expects 774 (missing 78) - year=2021: wrote 684, expects 762 (missing 78) The consistent 78-byte discrepancy suggests either: A) Parquet calculates row group size before finalizing footer B) FSDataOutputStream tracks position differently than our stream C) Footer is written with stale/incorrect metadata D) File size is cached/stale during rename operation getPos() logging will show if Parquet/Hadoop queries position and what value is returned vs what was actually written. * docs: comprehensive analysis of 78-byte EOFException Documented all findings, hypotheses, and debugging approach. Key insight: 78 bytes is likely the Parquet footer size. The file has data pages (684 bytes) but missing footer (78 bytes). Next run will show if getPos() reveals the cause. * Revert "docs: comprehensive analysis of 78-byte EOFException" This reverts commit 94ab173eb0. * fmt * debug: track ALL writes to Parquet files CRITICAL FINDING from previous run: - getPos() was NEVER called by Parquet/Hadoop! - This eliminates position tracking mismatch hypothesis - Bytes are genuinely not reaching our write() method Added detailed write() logging to track: - Every write call for .parquet files - Cumulative totalBytesWritten after each write - Buffer state during writes This will show the exact write pattern and reveal: A) If Parquet writes 762 bytes but only 684 reach us → FSDataOutputStream buffering issue B) If Parquet only writes 684 bytes → Parquet calculates size incorrectly C) Number and size of write() calls for a typical Parquet file Expected patterns: - Parquet typically writes in chunks: header, data pages, footer - For small files: might be 2-3 write calls - Footer should be ~78 bytes if that's what's missing Next run will show EXACT write sequence. * fmt * fix: reduce write() logging verbosity, add summary stats Previous run showed Parquet writes byte-by-byte (hundreds of 1-byte writes), flooding logs and getting truncated. This prevented seeing the full picture. Changes: 1. Only log writes >= 20 bytes (skip byte-by-byte metadata writes) 2. Track writeCallCount to see total number of write() invocations 3. Show writeCallCount in close() summary logs This will show: - Large data writes clearly (26, 34, 41, 67 bytes, etc.) - Total bytes written vs total calls (e.g., 684 bytes in 200+ calls) - Whether ALL bytes Parquet wrote actually reached close() If totalBytesWritten=684 at close, Parquet only sent 684 bytes. If totalBytesWritten=762 at close, Parquet sent all 762 bytes but we lost 78. Next run will definitively answer: Does Parquet write 684 or 762 bytes total? * fmt * feat: upgrade Apache Parquet to 1.16.0 to fix EOFException Upgrading from Parquet 1.13.1 (bundled with Spark 3.5.0) to 1.16.0. Root cause analysis showed: - Parquet writes 684/696 bytes total (confirmed via totalBytesWritten) - But Parquet's footer claims file should be 762/774 bytes - Consistent 78-byte discrepancy across all files - This is a Parquet writer bug in file size calculation Parquet 1.16.0 changelog includes: - Multiple fixes for compressed file handling - Improved footer metadata accuracy - Better handling of column statistics - Fixes for Snappy compression edge cases Test approach: 1. Keep Spark 3.5.0 (stable, known good) 2. Override transitive Parquet dependencies to 1.16.0 3. If this fixes the issue, great! 4. If not, consider upgrading Spark to 4.0.1 References: - Latest Parquet: https://downloads.apache.org/parquet/apache-parquet-1.16.0/ - Parquet format: 2.12.0 (latest) This should resolve the 'Still have: 78 bytes left' EOFException. * docs: add Parquet 1.16.0 upgrade summary and testing guide * debug: enhance logging to capture footer writes and getPos calls Added targeted logging to answer the key question: "Are the missing 78 bytes the Parquet footer that never got written?" Changes: 1. Log ALL writes after call 220 (likely footer-related) - Previous: only logged writes >= 20 bytes - Now: also log small writes near end marked [FOOTER?] 2. Enhanced getPos() logging with writeCalls context - Shows relationship between getPos() and actual writes - Helps identify if Parquet calculates size before writing footer This will reveal: A) What the last ~14 write calls contain (footer structure) B) If getPos() is called before/during footer writes C) If there's a mismatch between calculated size and actual writes Expected pattern if footer is missing: - Large writes up to ~600 bytes (data pages) - Small writes for metadata - getPos() called to calculate footer offset - Footer writes (78 bytes) that either: * Never happen (bug in Parquet) * Get lost in FSDataOutputStream * Are written but lost in flush Next run will show the exact write sequence! * debug parquet footer writing * docs: comprehensive analysis of persistent 78-byte Parquet issue After Parquet 1.16.0 upgrade: - Error persists (EOFException: 78 bytes left) - File sizes changed (684→693, 696→705) but SAME 78-byte gap - Footer IS being written (logs show complete write sequence) - All bytes ARE stored correctly (perfect consistency) Conclusion: This is a systematic offset calculation error in how Parquet calculates expected file size, not a missing data problem. Possible causes: 1. Page header size mismatch with Snappy compression 2. Column chunk metadata offset error in footer 3. FSDataOutputStream position tracking issue 4. Dictionary page size accounting problem Recommended next steps: 1. Try uncompressed Parquet (remove Snappy) 2. Examine actual file bytes with parquet-tools 3. Test with different Spark version (4.0.1) 4. Compare with known-working FS (HDFS, S3A) The 78-byte constant suggests a fixed structure size that Parquet accounts for but isn't actually written or is written differently. * test: add Parquet file download and inspection on failure Added diagnostic step to download and examine actual Parquet files when tests fail. This will definitively answer: 1. Is the file complete? (Check PAR1 magic bytes at start/end) 2. What size is it? (Compare actual vs expected) 3. Can parquet-tools read it? (Reader compatibility test) 4. What does the footer contain? (Hex dump last 200 bytes) Steps performed: - List files in SeaweedFS - Download first Parquet file - Check magic bytes (PAR1 at offset 0 and EOF-4) - Show file size from filesystem - Hex dump header (first 100 bytes) - Hex dump footer (last 200 bytes) - Run parquet-tools inspect/show - Upload file as artifact for local analysis This will reveal if the issue is: A) File is incomplete (missing trailer) → SeaweedFS write problem B) File is complete but unreadable → Parquet format problem C) File is complete and readable → SeaweedFS read problem D) File size doesn't match metadata → Footer offset problem The downloaded file will be available as 'failed-parquet-file' artifact. * Revert "docs: comprehensive analysis of persistent 78-byte Parquet issue" This reverts commit 8e5f1d60ee. * docs: push summary for Parquet diagnostics All diagnostic code already in place from previous commits: - Enhanced write logging with footer tracking - Parquet 1.16.0 upgrade - File download & inspection on failure (b767825ba) This push just adds documentation explaining what will happen when CI runs and what the file analysis will reveal. Ready to get definitive answer about the 78-byte discrepancy! * fix: restart SeaweedFS services before downloading files on test failure Problem: --abort-on-container-exit stops ALL containers when tests fail, so SeaweedFS services are down when file download step runs. Solution: 1. Use continue-on-error: true to capture test failure 2. Store exit code in GITHUB_OUTPUT for later checking 3. Add new step to restart SeaweedFS services if tests failed 4. Download step runs after services are back up 5. Final step checks test exit code and fails workflow This ensures: ✅ Services keep running for file analysis ✅ Parquet files are accessible via filer API ✅ Workflow still fails if tests failed ✅ All diagnostics can complete Now we'll actually be able to download and examine the Parquet files! * fix: restart SeaweedFS services before downloading files on test failure Problem: --abort-on-container-exit stops ALL containers when tests fail, so SeaweedFS services are down when file download step runs. Solution: 1. Use continue-on-error: true to capture test failure 2. Store exit code in GITHUB_OUTPUT for later checking 3. Add new step to restart SeaweedFS services if tests failed 4. Download step runs after services are back up 5. Final step checks test exit code and fails workflow This ensures: ✅ Services keep running for file analysis ✅ Parquet files are accessible via filer API ✅ Workflow still fails if tests failed ✅ All diagnostics can complete Now we'll actually be able to download and examine the Parquet files! * debug: improve file download with better diagnostics and fallbacks Problem: File download step shows 'No Parquet files found' even though ports are exposed (8888:8888) and services are running. Improvements: 1. Show raw curl output to see actual API response 2. Use improved grep pattern with -oP for better parsing 3. Add fallback to fetch file via docker exec if HTTP fails 4. If no files found via HTTP, try docker exec curl 5. If still no files, use weed shell 'fs.ls' to list files This will help us understand: - Is the HTTP API returning files in unexpected format? - Are files accessible from inside the container but not outside? - Are files in a different path than expected? One of these methods WILL find the files! * refactor: remove emojis from logging and workflow messages Removed all emoji characters from: 1. SeaweedOutputStream.java - write() logs - close() logs - getPos() logs - flushWrittenBytesToServiceInternal() logs - writeCurrentBufferToService() logs 2. SeaweedWrite.java - Chunk write logs - Metadata write logs - Mismatch warnings 3. SeaweedHadoopOutputStream.java - Constructor logs 4. spark-integration-tests.yml workflow - Replaced checkmarks with 'OK' - Replaced X marks with 'FAILED' - Replaced error marks with 'ERROR' - Replaced warning marks with 'WARNING:' All functionality remains the same, just cleaner ASCII-only output. * fix: run Spark integration tests on all branches Removed branch restrictions from workflow triggers. Now the tests will run on ANY branch when relevant files change: - test/java/spark/** - other/java/hdfs2/** - other/java/hdfs3/** - other/java/client/** - workflow file itself This fixes the issue where tests weren't running on feature branches. * fix: replace heredoc with echo pipe to fix YAML syntax The heredoc syntax (<<'SHELL_EOF') in the workflow was breaking YAML parsing and preventing the workflow from running. Changed from: weed shell <<'SHELL_EOF' fs.ls /test-spark/employees/ exit SHELL_EOF To: echo -e 'fs.ls /test-spark/employees/\nexit' | weed shell This achieves the same result but is YAML-compatible. * debug: add directory structure inspection before file download Added weed shell commands to inspect the directory structure: - List /test-spark/ to see what directories exist - List /test-spark/employees/ to see what files are there This will help diagnose why the HTTP API returns empty: - Are files there but HTTP not working? - Are files in a different location? - Were files cleaned up after the test? - Did the volume data persist after container restart? Will show us exactly what's in SeaweedFS after test failure. * debug: add comprehensive volume and container diagnostics Added checks to diagnose why files aren't accessible: 1. Container status before restart - See if containers are still running or stopped - Check exit codes 2. Volume inspection - List all docker volumes - Inspect seaweedfs-volume-data volume - Check if volume data persisted 3. Access from inside container - Use curl from inside filer container - This bypasses host networking issues - Shows if files exist but aren't exposed 4. Direct filesystem check - Try to ls the directory from inside container - See if filer has filesystem access This will definitively show: - Did data persist through container restart? - Are files there but not accessible via HTTP from host? - Is the volume getting cleaned up somehow? * fix: download Parquet file immediately after test failure ROOT CAUSE FOUND: Files disappear after docker compose stops containers. The data doesn't persist because: - docker compose up --abort-on-container-exit stops ALL containers when tests finish - When containers stop, the data in SeaweedFS is lost (even with named volumes, the metadata/index is lost when master/filer stop) - By the time we tried to download files, they were gone SOLUTION: Download file IMMEDIATELY after test failure, BEFORE docker compose exits and stops containers. Changes: 1. Moved file download INTO the test-run step 2. Download happens right after TEST_EXIT_CODE is captured 3. File downloads while containers are still running 4. Analysis step now just uses the already-downloaded file 5. Removed all the restart/diagnostics complexity This should finally get us the Parquet file for analysis! * fix: keep containers running during file download REAL ROOT CAUSE: --abort-on-container-exit stops ALL containers immediately when the test container exits, including the filer. So we couldn't download files because filer was already stopped. SOLUTION: Run tests in detached mode, wait for completion, then download while filer is still running. Changes: 1. docker compose up -d spark-tests (detached mode) 2. docker wait seaweedfs-spark-tests (wait for completion) 3. docker inspect to get exit code 4. docker compose logs to show test output 5. Download file while all services still running 6. Then exit with test exit code Improved grep pattern to be more specific: part-[a-f0-9-]+\.c000\.snappy\.parquet This MUST work - filer is guaranteed to be running during download! * fix: add comprehensive diagnostics for file location The directory is empty, which means tests are failing BEFORE writing files. Enhanced diagnostics: 1. List /test-spark/ root to see what directories exist 2. Grep test logs for 'employees', 'people_partitioned', '.parquet' 3. Try multiple possible locations: employees, people_partitioned, people 4. Show WHERE the test actually tried to write files This will reveal: - If test fails before writing (connection error, etc.) - What path the test is actually using - Whether files exist in a different location * fix: download Parquet file in real-time when EOF error occurs ROOT CAUSE: Spark cleans up files after test completes (even on failure). By the time we try to download, files are already deleted. SOLUTION: Monitor test logs in real-time and download file THE INSTANT we see the EOF error (meaning file exists and was just read). Changes: 1. Start tests in detached mode 2. Background process monitors logs for 'EOFException.*78 bytes' 3. When detected, extract filename from error message 4. Download IMMEDIATELY (file still exists!) 5. Quick analysis with parquet-tools 6. Main process waits for test completion This catches the file at the exact moment it exists and is causing the error! * chore: trigger new workflow run with real-time monitoring * fix: download Parquet data directly from volume server BREAKTHROUGH: Download chunk data directly from volume server, bypassing filer! The issue: Even real-time monitoring is too slow - Spark deletes filer metadata instantly after the EOF error. THE SOLUTION: Extract chunk ID from logs and download directly from volume server. Volume keeps data even after filer metadata is deleted! From logs we see: file_id: "7,d0364fd01" size: 693 We can download this directly: curl http://localhost:8080/7,d0364fd01 Changes: 1. Extract chunk file_id from logs (format: "volume,filekey") 2. Download directly from volume server port 8080 3. Volume data persists longer than filer metadata 4. Comprehensive analysis with parquet-tools, hexdump, magic bytes This WILL capture the actual file data! * fix: extract correct chunk ID (not source_file_id) The grep was matching 'source_file_id' instead of 'file_id'. Fixed pattern to look for ' file_id: ' (with spaces) which excludes 'source_file_id:' line. Now will correctly extract: file_id: "7,d0cdf5711" ← THIS ONE Instead of: source_file_id: "0,000000000" ← NOT THIS The correct chunk ID should download successfully from volume server! * feat: add detailed offset analysis for 78-byte discrepancy SUCCESS: File downloaded and readable! Now analyzing WHY Parquet expects 78 more bytes. Added analysis: 1. Parse footer length from last 8 bytes 2. Extract column chunk offsets from parquet-tools meta 3. Compare actual file size with expected size from metadata 4. Identify if offsets are pointing beyond actual data This will reveal: - Are column chunk offsets incorrectly calculated during write? - Is the footer claiming data that doesn't exist? - Where exactly are the missing 78 bytes supposed to be? The file is already uploaded as artifact for deeper local analysis. * fix: extract chunk ID for the EXACT file causing EOF error CRITICAL FIX: We were downloading the wrong file! The issue: - EOF error is for: test-spark/employees/part-00000-xxx.parquet - But logs contain MULTIPLE files (employees_window with 1275 bytes, etc.) - grep -B 50 was matching chunk info from OTHER files The solution: 1. Extract the EXACT failing filename from EOF error message 2. Search logs for chunk info specifically for THAT file 3. Download the correct chunk Example: - EOF error mentions: part-00000-32cafb4f-82c4-436e-a22a-ebf2f5cb541e-c000.snappy.parquet - Find chunk info for this specific file, not other files in logs Now we'll download the actual problematic file, not a random one! * fix: search for failing file in read context (SeaweedInputStream) The issue: We're not finding the correct file because: 1. Error mentions: test-spark/employees/part-00000-xxx.parquet 2. But we downloaded chunk from employees_window (different file!) The problem: - File is already written when error occurs - Error happens during READ, not write - Need to find when SeaweedInputStream opens this file for reading New approach: 1. Extract filename from EOF error message 2. Search for 'new path:' + filename (when file is opened for read) 3. Get chunk info from the entry details logged at that point 4. Download the ACTUAL failing chunk This should finally get us the right file with the 78-byte issue! * fix: search for filename in 'Encountered error' message The issue: grep pattern was wrong and looking in wrong place - EOF exception is in the 'Caused by' section - Filename is in the outer exception message The fix: - Search for 'Encountered error while reading file' line - Extract filename: part-00000-xxx-c000.snappy.parquet - Fixed regex pattern (was missing dash before c000) Example from logs: 'Encountered error while reading file seaweedfs://...part-00000-c5a41896-5221-4d43-a098-d0839f5745f6-c000.snappy.parquet' This will finally extract the right filename! * feat: proactive download - grab files BEFORE Spark deletes them BREAKTHROUGH STRATEGY: Don't wait for error, download files proactively! The problem: - Waiting for EOF error is too slow - By the time we extract chunk ID, Spark has deleted the file - Volume garbage collection removes chunks quickly The solution: 1. Monitor for 'Running seaweed.spark.SparkSQLTest' in logs 2. Sleep 5 seconds (let test write files) 3. Download ALL files from /test-spark/employees/ immediately 4. Keep files for analysis when EOF occurs This downloads files while they still exist, BEFORE Spark cleanup! Timeline: Write → Download (NEW!) → Read → EOF Error → Analyze Instead of: Write → Read → EOF Error → Try to download (file gone!) ❌ This will finally capture the actual problematic file! * fix: poll for files to appear instead of fixed sleep The issue: Fixed 5-second sleep was too short - files not written yet The solution: Poll every second for up to 30 seconds - Check if files exist in employees directory - Download immediately when they appear - Log progress every 5 seconds This gives us a 30-second window to catch the file between: - Write (file appears) - Read (EOF error) The file should appear within a few seconds of SparkSQLTest starting, and we'll grab it immediately! * feat: add explicit logging when employees Parquet file is written PRECISION TRIGGER: Log exactly when the file we need is written! Changes: 1. SeaweedOutputStream.close(): Add WARN log for /test-spark/employees/*.parquet - Format: '=== PARQUET FILE WRITTEN TO EMPLOYEES: filename (size bytes) ===' - Uses WARN level so it stands out in logs 2. Workflow: Trigger download on this exact log message - Instead of 'Running seaweed.spark.SparkSQLTest' (too early) - Now triggers on 'PARQUET FILE WRITTEN TO EMPLOYEES' (exact moment!) Timeline: File write starts ↓ close() called → LOG APPEARS ↓ Workflow detects log → DOWNLOAD NOW! ← We're here instantly! ↓ Spark reads file → EOF error ↓ Analyze downloaded file ✅ This gives us the EXACT moment to download, with near-zero latency! * fix: search temporary directories for Parquet files The issue: Files written to employees/ but immediately moved/deleted by Spark Spark's file commit process: 1. Write to: employees/_temporary/0/_temporary/attempt_xxx/part-xxx.parquet 2. Commit/rename to: employees/part-xxx.parquet 3. Read and delete (on failure) By the time we check employees/, the file is already gone! Solution: Search multiple locations - employees/ (final location) - employees/_temporary/ (intermediate) - employees/_temporary/0/_temporary/ (write location) - Recursive search as fallback Also: - Extract exact filename from write log - Try all locations until we find the file - Show directory listings for debugging This should catch files in their temporary location before Spark moves them! * feat: extract chunk IDs from write log and download from volume ULTIMATE SOLUTION: Bypass filer entirely, download chunks directly! The problem: Filer metadata is deleted instantly after write - Directory listings return empty - HTTP API can't find the file - Even temporary paths are cleaned up The breakthrough: Get chunk IDs from the WRITE operation itself! Changes: 1. SeaweedOutputStream: Log chunk IDs in write message Format: 'CHUNKS: [id1,id2,...]' 2. Workflow: Extract chunk IDs from log, download from volume - Parse 'CHUNKS: [...]' from write log - Download directly: http://localhost:8080/CHUNK_ID - Volume keeps chunks even after filer metadata deleted Why this MUST work: - Chunk IDs logged at write time (not dependent on reads) - Volume server persistence (chunks aren't deleted immediately) - Bypasses filer entirely (no metadata lookups) - Direct data access (raw chunk bytes) Timeline: Write → Log chunk ID → Extract ID → Download chunk → Success! ✅ * fix: don't split chunk ID on comma - comma is PART of the ID! CRITICAL BUG FIX: Chunk ID format is 'volumeId,fileKey' (e.g., '3,0307c52bab') The problem: - Log shows: CHUNKS: [3,0307c52bab] - Script was splitting on comma: IFS=',' - Tried to download: '3' (404) and '0307c52bab' (404) - Both failed! The fix: - Chunk ID is a SINGLE string with embedded comma - Don't split it! - Download directly: http://localhost:8080/3,0307c52bab This should finally work! * Update SeaweedOutputStream.java * fix: Override FSDataOutputStream.getPos() to use SeaweedOutputStream position CRITICAL FIX for Parquet 78-byte EOF error! Root Cause Analysis: - Hadoop's FSDataOutputStream tracks position with an internal counter - It does NOT call SeaweedOutputStream.getPos() by default - When Parquet writes data and calls getPos() to record column chunk offsets, it gets FSDataOutputStream's counter, not SeaweedOutputStream's actual position - This creates a 78-byte mismatch between recorded offsets and actual file size - Result: EOFException when reading (tries to read beyond file end) The Fix: - Override getPos() in the anonymous FSDataOutputStream subclass - Delegate to SeaweedOutputStream.getPos() which returns 'position + buffer.position()' - This ensures Parquet gets the correct position when recording metadata - Column chunk offsets in footer will now match actual data positions This should fix the consistent 78-byte discrepancy we've been seeing across all Parquet file writes (regardless of file size: 684, 693, 1275 bytes, etc.) * docs: add detailed analysis of Parquet EOF fix * docs: push instructions for Parquet EOF fix * debug: add aggressive logging to FSDataOutputStream getPos() override This will help determine: 1. If the anonymous FSDataOutputStream subclass is being created 2. If the getPos() override is actually being called by Parquet 3. What position value is being returned If we see 'Creating FSDataOutputStream' but NOT 'getPos() override called', it means FSDataOutputStream is using a different mechanism for position tracking. If we don't see either log, it means the code path isn't being used at all. * fix: make path variable final for anonymous inner class Java compilation error: - 'local variables referenced from an inner class must be final or effectively final' - The 'path' variable was being reassigned (path = qualify(path)) - This made it non-effectively-final Solution: - Create 'final Path finalPath = path' after qualification - Use finalPath in the anonymous FSDataOutputStream subclass - Applied to both create() and append() methods * debug: change logs to WARN level to ensure visibility INFO logs from seaweed.hdfs package may be filtered. Changed all diagnostic logs to WARN level to match the 'PARQUET FILE WRITTEN' log which DOES appear in test output. This will definitively show: 1. Whether our code path is being used 2. Whether the getPos() override is being called 3. What position values are being returned * fix: enable DEBUG logging for seaweed.hdfs package Added explicit log4j configuration: log4j.logger.seaweed.hdfs=DEBUG This ensures ALL logs from SeaweedFileSystem and SeaweedHadoopOutputStream will appear in test output, including our diagnostic logs for position tracking. Without this, the generic 'seaweed=INFO' setting might filter out DEBUG level logs from the HDFS integration layer. * debug: add logging to SeaweedFileSystemStore.createFile() Critical diagnostic: Our FSDataOutputStream.getPos() override is NOT being called! Adding WARN logs to SeaweedFileSystemStore.createFile() to determine: 1. Is createFile() being called at all? 2. If yes, but FSDataOutputStream override not called, then streams are being returned WITHOUT going through SeaweedFileSystem.create/append 3. This would explain why our position tracking fix has no effect Hypothesis: SeaweedFileSystemStore.createFile() returns SeaweedHadoopOutputStream directly, and it gets wrapped by something else (not our custom FSDataOutputStream). * debug: add WARN logging to SeaweedOutputStream base constructor CRITICAL: None of our higher-level logging is appearing! - NO SeaweedFileSystemStore.createFile logs - NO SeaweedHadoopOutputStream constructor logs - NO FSDataOutputStream.getPos() override logs But we DO see: - WARN SeaweedOutputStream: PARQUET FILE WRITTEN (from close()) Adding WARN log to base SeaweedOutputStream constructor will tell us: 1. IF streams are being created through our code at all 2. If YES, we can trace the call stack 3. If NO, streams are being created through a completely different mechanism (maybe Hadoop is caching/reusing FileSystem instances with old code) * debug: verify JARs contain latest code before running tests CRITICAL ISSUE: Our constructor logs aren't appearing! Adding verification step to check if SeaweedOutputStream JAR contains the new 'BASE constructor called' log message. This will tell us: 1. If verification FAILS → Maven is building stale JARs (caching issue) 2. If verification PASSES but logs still don't appear → Docker isn't using the JARs 3. If verification PASSES and logs appear → Fix is working! Using 'strings' on the .class file to grep for the log message. * Update SeaweedOutputStream.java * debug: add logging to SeaweedInputStream constructor to track contentLength CRITICAL FINDING: File is PERFECT but Spark fails to read it! The downloaded Parquet file (1275 bytes): - ✅ Valid header/trailer (PAR1) - ✅ Complete metadata - ✅ parquet-tools reads it successfully (all 4 rows) - ❌ Spark gets 'Still have: 78 bytes left' EOF error This proves the bug is in READING, not writing! Hypothesis: SeaweedInputStream.contentLength is set to 1197 (1275-78) instead of 1275 when opening the file for reading. Adding WARN logs to track: - When SeaweedInputStream is created - What contentLength is calculated as - How many chunks the entry has This will show if the metadata is being read incorrectly when Spark opens the file, causing contentLength to be 78 bytes short. * fix: SeaweedInputStream returning 0 bytes for inline content reads ROOT CAUSE IDENTIFIED: In SeaweedInputStream.read(ByteBuffer buf), when reading inline content (stored directly in the protobuf entry), the code was copying data to the buffer but NOT updating bytesRead, causing it to return 0. This caused Parquet's H2SeekableInputStream.readFully() to fail with: "EOFException: Still have: 78 bytes left" The readFully() method calls read() in a loop until all requested bytes are read. When read() returns 0 or -1 prematurely, it throws EOF. CHANGES: 1. SeaweedInputStream.java: - Fixed inline content read to set bytesRead = len after copying - Added debug logging to track position, len, and bytesRead - This ensures read() always returns the actual number of bytes read 2. SeaweedStreamIntegrationTest.java: - Added comprehensive testRangeReads() that simulates Parquet behavior: * Seeks to specific offsets (like reading footer at end) * Reads specific byte ranges (like reading column chunks) * Uses readFully() pattern with multiple sequential read() calls * Tests the exact scenario that was failing (78-byte read at offset 1197) - This test will catch any future regressions in range read behavior VERIFICATION: Local testing showed: - contentLength correctly set to 1275 bytes - Chunk download retrieved all 1275 bytes from volume server - BUT read() was returning -1 before fulfilling Parquet's request - After fix, test compiles successfully Related to: Spark integration test failures with Parquet files * debug: add detailed getPos() tracking with caller stack trace Added comprehensive logging to track: 1. Who is calling getPos() (using stack trace) 2. The position values being returned 3. Buffer flush operations 4. Total bytes written at each getPos() call This helps diagnose if Parquet is recording incorrect column chunk offsets in the footer metadata, which would cause seek-to-wrong-position errors when reading the file back. Key observations from testing: - getPos() is called frequently by Parquet writer - All positions appear correct (0, 4, 59, 92, 139, 172, 203, 226, 249, 272, etc.) - Buffer flushes are logged to track when position jumps - No EOF errors observed in recent test run Next: Analyze if the fix resolves the issue completely * docs: add comprehensive debugging analysis for EOF exception fix Documents the complete debugging journey from initial symptoms through to the root cause discovery and fix. Key finding: SeaweedInputStream.read() was returning 0 bytes when copying inline content, causing Parquet's readFully() to throw EOF exceptions. The fix ensures read() always returns the actual number of bytes copied. * debug: add logging to EOF return path - FOUND ROOT CAUSE! Added logging to the early return path in SeaweedInputStream.read() that returns -1 when position >= contentLength. KEY FINDING: Parquet is trying to read 78 bytes from position 1275, but the file ends at 1275! This proves the Parquet footer metadata has INCORRECT offsets or sizes, making it think there's data at bytes [1275-1353) which don't exist. Since getPos() returned correct values during write (383, 1267), the issue is likely: 1. Parquet 1.16.0 has different footer format/calculation 2. There's a mismatch between write-time and read-time offset calculations 3. Column chunk sizes in footer are off by 78 bytes Next: Investigate if downgrading Parquet or fixing footer size calculations resolves the issue. * debug: confirmed root cause - Parquet tries to read 78 bytes past EOF **KEY FINDING:** Parquet is trying to read 78 bytes starting at position 1275, but the file ends at 1275! This means: 1. The Parquet footer metadata contains INCORRECT offsets or sizes 2. It thinks there's a column chunk or row group at bytes [1275-1353) 3. But the actual file is only 1275 bytes During write, getPos() returned correct values (0, 190, 231, 262, etc., up to 1267). Final file size: 1275 bytes (1267 data + 8-byte footer). During read: - Successfully reads [383, 1267) → 884 bytes ✅ - Successfully reads [1267, 1275) → 8 bytes ✅ - Successfully reads [4, 1275) → 1271 bytes ✅ - FAILS trying to read [1275, 1353) → 78 bytes ❌ The '78 bytes' is ALWAYS constant across all test runs, indicating a systematic offset calculation error, not random corruption. Files modified: - SeaweedInputStream.java - Added EOF logging to early return path - ROOT_CAUSE_CONFIRMED.md - Analysis document - ParquetReproducerTest.java - Attempted standalone reproducer (incomplete) - pom.xml - Downgraded Parquet to 1.13.1 (didn't fix issue) Next: The issue is likely in how getPos() is called during column chunk writes. The footer records incorrect offsets, making it expect data beyond EOF. * docs: comprehensive issue summary - getPos() buffer flush timing issue Added detailed analysis showing: - Root cause: Footer metadata has incorrect offsets - Parquet tries to read [1275-1353) but file ends at 1275 - The '78 bytes' constant indicates buffered data size at footer write time - Most likely fix: Flush buffer before getPos() returns position Next step: Implement buffer flush in getPos() to ensure returned position reflects all written data, not just flushed data. * test: add GetPosBufferTest to reproduce Parquet issue - ALL TESTS PASS! Created comprehensive unit tests that specifically test the getPos() behavior with buffered data, including the exact 78-byte scenario from the Parquet bug. KEY FINDING: All tests PASS! ✅ - getPos() correctly returns position + buffer.position() - Files are written with correct sizes - Data can be read back at correct positions This proves the issue is NOT in the basic getPos() implementation, but something SPECIFIC to how Spark/Parquet uses the FSDataOutputStream. Tests include: 1. testGetPosWithBufferedData() - Basic multi-chunk writes 2. testGetPosWithSmallWrites() - Simulates Parquet's pattern 3. testGetPosWithExactly78BytesBuffered() - The exact bug scenario Next: Analyze why Spark behaves differently than our unit tests. * docs: comprehensive test results showing unit tests PASS but Spark fails KEY FINDINGS: - Unit tests: ALL 3 tests PASS ✅ including exact 78-byte scenario - getPos() works correctly: returns position + buffer.position() - FSDataOutputStream override IS being called in Spark - But EOF exception still occurs at position=1275 trying to read 78 bytes This proves the bug is NOT in getPos() itself, but in HOW/WHEN Parquet uses the returned positions. Hypothesis: Parquet footer has positions recorded BEFORE final flush, causing a 78-byte offset error in column chunk metadata. * docs: BREAKTHROUGH - found the bug in Spark local reproduction! KEY FINDINGS from local Spark test: 1. flushedPosition=0 THE ENTIRE TIME during writes! - All data stays in buffer until close - getPos() returns bufferPosition (0 + bufferPos) 2. Critical sequence discovered: - Last getPos(): bufferPosition=1252 (Parquet records this) - close START: buffer.position()=1260 (8 MORE bytes written!) - File size: 1260 bytes 3. The Gap: - Parquet calls getPos() and gets 1252 - Parquet writes 8 MORE bytes (footer metadata) - File ends at 1260 - But Parquet footer has stale positions from when getPos() was 1252 4. Why unit tests pass but Spark fails: - Unit tests: write, getPos(), close (no more writes) - Spark: write chunks, getPos(), write footer, close The Parquet footer metadata is INCORRECT because Parquet writes additional data AFTER the last getPos() call but BEFORE close. Next: Download actual Parquet file and examine footer with parquet-tools. * docs: complete local reproduction analysis with detailed findings Successfully reproduced the EOF exception locally and traced the exact issue: FINDINGS: - Unit tests pass (all 3 including 78-byte scenario) - Spark test fails with same EOF error - flushedPosition=0 throughout entire write (all data buffered) - 8-byte gap between last getPos()(1252) and close(1260) - Parquet writes footer AFTER last getPos() call KEY INSIGHT: getPos() implementation is CORRECT (position + buffer.position()). The issue is the interaction between Parquet's footer writing sequence and SeaweedFS's buffering strategy. Parquet sequence: 1. Write chunks, call getPos() → records 1252 2. Write footer metadata → +8 bytes 3. Close → flush 1260 bytes total 4. Footer says data ends at 1252, but tries to read at 1260+ Next: Compare with HDFS behavior and examine actual Parquet footer metadata. * feat: add comprehensive debug logging to track Parquet write sequence Added extensive WARN-level debug messages to trace the exact sequence of: - Every write() operation with position tracking - All getPos() calls with caller stack traces - flush() and flushInternal() operations - Buffer flushes and position updates - Metadata updates BREAKTHROUGH FINDING: - Last getPos() call: returns 1252 bytes (at writeCall #465) - 5 more writes happen: add 8 bytes → buffer.position()=1260 - close() flushes all 1260 bytes to disk - But Parquet footer records offsets based on 1252! Result: 8-byte offset mismatch in Parquet footer metadata → Causes EOFException: 'Still have: 78 bytes left' The 78 bytes is NOT missing data - it's a metadata calculation error due to Parquet footer offsets being stale by 8 bytes. * docs: comprehensive analysis of Parquet EOF root cause and fix strategies Documented complete technical analysis including: ROOT CAUSE: - Parquet writes footer metadata AFTER last getPos() call - 8 bytes written without getPos() being called - Footer records stale offsets (1252 instead of 1260) - Results in metadata mismatch → EOF exception on read FIX OPTIONS (4 approaches analyzed): 1. Flush on getPos() - simple but slow 2. Track virtual position - RECOMMENDED 3. Defer footer metadata - complex 4. Force flush before close - workaround RECOMMENDED: Option 2 (Virtual Position) - Add virtualPosition field - getPos() returns virtualPosition (not position) - Aligns with Hadoop FSDataOutputStream semantics - No performance impact Ready to implement the fix. * feat: implement virtual position tracking in SeaweedOutputStream Added virtualPosition field to track total bytes written including buffered data. Updated getPos() to return virtualPosition instead of position + buffer.position(). RESULT: - getPos() now always returns accurate total (1260 bytes) ✓ - File size metadata is correct (1260 bytes) ✓ - EOF exception STILL PERSISTS ❌ ROOT CAUSE (deeper analysis): Parquet calls getPos() → gets 1252 → STORES this value Then writes 8 more bytes (footer metadata) Then writes footer containing the stored offset (1252) Result: Footer has stale offsets, even though getPos() is correct THE FIX DOESN'T WORK because Parquet uses getPos() return value IMMEDIATELY, not at close time. Virtual position tracking alone can't solve this. NEXT: Implement flush-on-getPos() to ensure offsets are always accurate. * feat: implement flush-on-getPos() to ensure accurate offsets IMPLEMENTATION: - Added buffer flush in getPos() before returning position - Every getPos() call now flushes buffered data - Updated FSDataOutputStream wrappers to handle IOException - Extensive debug logging added RESULT: - Flushing is working ✓ (logs confirm) - File size is correct (1260 bytes) ✓ - EOF exception STILL PERSISTS ❌ DEEPER ROOT CAUSE DISCOVERED: Parquet records offsets when getPos() is called, THEN writes more data, THEN writes footer with those recorded (now stale) offsets. Example: 1. Write data → getPos() returns 100 → Parquet stores '100' 2. Write dictionary (no getPos()) 3. Write footer containing '100' (but actual offset is now 110) Flush-on-getPos() doesn't help because Parquet uses the RETURNED VALUE, not the current position when writing footer. NEXT: Need to investigate Parquet's footer writing or disable buffering entirely. * docs: complete debug session summary and findings Comprehensive documentation of the entire debugging process: PHASES: 1. Debug logging - Identified 8-byte gap between getPos() and actual file size 2. Virtual position tracking - Ensured getPos() returns correct total 3. Flush-on-getPos() - Made position always reflect committed data RESULT: All implementations correct, but EOF exception persists! ROOT CAUSE IDENTIFIED: Parquet records offsets when getPos() is called, then writes more data, then writes footer with those recorded (now stale) offsets. This is a fundamental incompatibility between: - Parquet's assumption: getPos() = exact file offset - Buffered streams: Data buffered, offsets recorded, then flushed NEXT STEPS: 1. Check if Parquet uses Syncable.hflush() 2. If yes: Implement hflush() properly 3. If no: Disable buffering for Parquet files The debug logging successfully identified the issue. The fix requires architectural changes to how SeaweedFS handles Parquet writes. * feat: comprehensive Parquet EOF debugging with multiple fix attempts IMPLEMENTATIONS TRIED: 1. ✅ Virtual position tracking 2. ✅ Flush-on-getPos() 3. ✅ Disable buffering (bufferSize=1) 4. ✅ Return virtualPosition from getPos() 5. ✅ Implement hflush() logging CRITICAL FINDINGS: - Parquet does NOT call hflush() or hsync() - Last getPos() always returns 1252 - Final file size always 1260 (8-byte gap) - EOF exception persists in ALL approaches - Even with bufferSize=1 (completely unbuffered), problem remains ROOT CAUSE (CONFIRMED): Parquet's write sequence is incompatible with ANY buffered stream: 1. Writes data (1252 bytes) 2. Calls getPos() → records offset (1252) 3. Writes footer metadata (8 bytes) WITHOUT calling getPos() 4. Writes footer containing recorded offset (1252) 5. Close → flushes all 1260 bytes 6. Result: Footer says offset 1252, but actual is 1260 The 78-byte error is Parquet's calculation based on incorrect footer offsets. CONCLUSION: This is not a SeaweedFS bug. It's a fundamental incompatibility with how Parquet writes files. The problem requires either: - Parquet source code changes (to call hflush/getPos properly) - Or SeaweedFS to handle Parquet as a special case differently All our implementations were correct but insufficient to fix the core issue. * fix: implement flush-before-getPos() for Parquet compatibility After analyzing Parquet-Java source code, confirmed that: 1. Parquet calls out.getPos() before writing each page to record offsets 2. These offsets are stored in footer metadata 3. Footer length (4 bytes) + MAGIC (4 bytes) are written after last page 4. When reading, Parquet seeks to recorded offsets IMPLEMENTATION: - getPos() now flushes buffer before returning position - This ensures recorded offsets match actual file positions - Added comprehensive debug logging RESULT: - Offsets are now correctly recorded (verified in logs) - Last getPos() returns 1252 ✓ - File ends at 1260 (1252 + 8 footer bytes) ✓ - Creates 17 chunks instead of 1 (side effect of many flushes) - EOF exception STILL PERSISTS ❌ ANALYSIS: The EOF error persists despite correct offset recording. The issue may be: 1. Too many small chunks (17 chunks for 1260 bytes) causing fragmentation 2. Chunks being assembled incorrectly during read 3. Or a deeper issue in how Parquet footer is structured The implementation is CORRECT per Parquet's design, but something in the chunk assembly or read path is still causing the 78-byte EOF error. Next: Investigate chunk assembly in SeaweedRead or consider atomic writes. * docs: comprehensive recommendation for Parquet EOF fix After exhaustive investigation and 6 implementation attempts, identified that: ROOT CAUSE: - Parquet footer metadata expects 1338 bytes - Actual file size is 1260 bytes - Discrepancy: 78 bytes (the EOF error) - All recorded offsets are CORRECT - But Parquet's internal size calculations are WRONG when using many small chunks APPROACHES TRIED (ALL FAILED): 1. Virtual position tracking 2. Flush-on-getPos() (creates 17 chunks/1260 bytes, offsets correct, footer wrong) 3. Disable buffering (261 chunks, same issue) 4. Return flushed position 5. Syncable.hflush() (Parquet never calls it) RECOMMENDATION: Implement atomic Parquet writes: - Buffer entire file in memory (with disk spill) - Write as single chunk on close() - Matches local filesystem behavior - Guaranteed to work This is the ONLY viable solution without: - Modifying Apache Parquet source code - Or accepting the incompatibility Trade-off: Memory buffering vs. correct Parquet support. * experiment: prove chunk count irrelevant to 78-byte EOF error Tested 4 different flushing strategies: - Flush on every getPos() → 17 chunks → 78 byte error - Flush every 5 calls → 10 chunks → 78 byte error - Flush every 20 calls → 10 chunks → 78 byte error - NO intermediate flushes (single chunk) → 1 chunk → 78 byte error CONCLUSION: The 78-byte error is CONSTANT regardless of: - Number of chunks (1, 10, or 17) - Flush strategy - getPos() timing - Write pattern This PROVES: ✅ File writing is correct (1260 bytes, complete) ✅ Chunk assembly is correct ✅ SeaweedFS chunked storage works fine ❌ The issue is in Parquet's footer metadata calculation The problem is NOT how we write files - it's how Parquet interprets our file metadata to calculate expected file size. Next: Examine what metadata Parquet reads from entry.attributes and how it differs from actual file content. * test: prove Parquet works perfectly when written directly (not via Spark) Created ParquetMemoryComparisonTest that writes identical Parquet data to: 1. Local filesystem 2. SeaweedFS RESULTS: ✅ Both files are 643 bytes ✅ Files are byte-for-byte IDENTICAL ✅ Both files read successfully with ParquetFileReader ✅ NO EOF errors! CONCLUSION: The 78-byte EOF error ONLY occurs when Spark writes Parquet files. Direct Parquet writes work perfectly on SeaweedFS. This proves: - SeaweedFS file storage is correct - Parquet library works fine with SeaweedFS - The issue is in SPARK's Parquet writing logic The problem is likely in how Spark's ParquetOutputFormat or ParquetFileWriter interacts with our getPos() implementation during the multi-stage write/commit process. * test: prove Spark CAN read Parquet files (both direct and Spark-written) Created SparkReadDirectParquetTest with two tests: TEST 1: Spark reads directly-written Parquet - Direct write: 643 bytes - Spark reads it: ✅ SUCCESS (3 rows) - Proves: Spark's READ path works fine TEST 2: Spark writes then reads Parquet - Spark writes via INSERT: 921 bytes (3 rows) - Spark reads it: ✅ SUCCESS (3 rows) - Proves: Some Spark write paths work fine COMPARISON WITH FAILING TEST: - SparkSQLTest (FAILING): df.write().parquet() → 1260 bytes (4 rows) → EOF error - SparkReadDirectParquetTest (PASSING): INSERT INTO → 921 bytes (3 rows) → works CONCLUSION: The issue is SPECIFIC to Spark's DataFrame.write().parquet() code path, NOT a general Spark+SeaweedFS incompatibility. Different Spark write methods: 1. Direct ParquetWriter: 643 bytes → ✅ works 2. Spark INSERT INTO: 921 bytes → ✅ works 3. Spark df.write().parquet(): 1260 bytes → ❌ EOF error The 78-byte error only occurs with DataFrame.write().parquet()! * test: prove I/O operations identical between local and SeaweedFS Created ParquetOperationComparisonTest to log and compare every read/write operation during Parquet file operations. WRITE TEST RESULTS: - Local: 643 bytes, 6 operations - SeaweedFS: 643 bytes, 6 operations - Comparison: IDENTICAL (except name prefix) READ TEST RESULTS: - Local: 643 bytes in 3 chunks - SeaweedFS: 643 bytes in 3 chunks - Comparison: IDENTICAL (except name prefix) CONCLUSION: When using direct ParquetWriter (not Spark's DataFrame.write): ✅ Write operations are identical ✅ Read operations are identical ✅ File sizes are identical ✅ NO EOF errors This definitively proves: 1. SeaweedFS I/O operations work correctly 2. Parquet library integration is perfect 3. The 78-byte EOF error is ONLY in Spark's DataFrame.write().parquet() 4. Not a general SeaweedFS or Parquet issue The problem is isolated to a specific Spark API interaction. * test: comprehensive I/O comparison reveals timing/metadata issue Created SparkDataFrameWriteComparisonTest to compare Spark operations between local and SeaweedFS filesystems. BREAKTHROUGH FINDING: - Direct df.write().parquet() → ✅ WORKS (1260 bytes) - Direct df.read().parquet() → ✅ WORKS (4 rows) - SparkSQLTest write → ✅ WORKS - SparkSQLTest read → ❌ FAILS (78-byte EOF) The issue is NOT in the write path - writes succeed perfectly! The issue appears to be in metadata visibility/timing when Spark reads back files it just wrote. This suggests: 1. Metadata not fully committed/visible 2. File handle conflicts 3. Distributed execution timing issues 4. Spark's task scheduler reading before full commit The 78-byte error is consistent with Parquet footer metadata being stale or not yet visible to the reader. * docs: comprehensive analysis of I/O comparison findings Created BREAKTHROUGH_IO_COMPARISON.md documenting: KEY FINDINGS: 1. I/O operations IDENTICAL between local and SeaweedFS 2. Spark df.write() WORKS perfectly (1260 bytes) 3. Spark df.read() WORKS in isolation 4. Issue is metadata visibility/timing, not data corruption ROOT CAUSE: - Writes complete successfully - File data is correct (1260 bytes) - Metadata may not be immediately visible after write - Spark reads before metadata fully committed - Results in 78-byte EOF error (stale metadata) SOLUTION: Implement explicit metadata sync/commit operation to ensure metadata visibility before close() returns. This is a solvable metadata consistency issue, not a fundamental I/O or Parquet integration problem. * WIP: implement metadata visibility check in close() Added ensureMetadataVisible() method that: - Performs lookup after flush to verify metadata is visible - Retries with exponential backoff if metadata is stale - Logs all attempts for debugging STATUS: Method is being called but EOF error still occurs. Need to investigate: 1. What metadata values are being returned 2. Whether the issue is in write or read path 3. Timing of when Spark reads vs when metadata is visible The method is confirmed to execute (logs show it's called) but the 78-byte EOF error persists, suggesting the issue may be more complex than simple metadata visibility timing. * docs: final investigation summary - issue is in rename operation After extensive testing and debugging: PROVEN TO WORK: ✅ Direct Parquet writes to SeaweedFS ✅ Spark reads Parquet from SeaweedFS ✅ Spark df.write() in isolation ✅ I/O operations identical to local filesystem ✅ Spark INSERT INTO STILL FAILS: ❌ SparkSQLTest with DataFrame.write().parquet() ROOT CAUSE IDENTIFIED: The issue is in Spark's file commit protocol: 1. Spark writes to _temporary directory (succeeds) 2. Spark renames to final location 3. Metadata after rename is stale/incorrect 4. Spark reads final file, gets 78-byte EOF error ATTEMPTED FIX: - Added ensureMetadataVisible() in close() - Result: Method HANGS when calling lookupEntry() - Reason: Cannot lookup from within close() (deadlock) CONCLUSION: The issue is NOT in write path, it's in RENAME operation. Need to investigate SeaweedFS rename() to ensure metadata is correctly preserved/updated when moving files from temporary to final locations. Removed hanging metadata check, documented findings. * debug: add rename logging - proves metadata IS preserved correctly CRITICAL FINDING: Rename operation works perfectly: - Source: size=1260 chunks=1 - Destination: size=1260 chunks=1 - Metadata is correctly preserved! The EOF error occurs DURING READ, not after rename. Parquet tries to read at position=1260 with bufRemaining=78, meaning it expects file to be 1338 bytes but it's only 1260. This proves the issue is in how Parquet WRITES the file, not in how SeaweedFS stores or renames it. The Parquet footer contains incorrect offsets that were calculated during the write phase. * fix: implement flush-on-getPos() - still fails with 78-byte error Implemented proper flush before returning position in getPos(). This ensures Parquet's recorded offsets match actual file layout. RESULT: Still fails with same 78-byte EOF error! FINDINGS: - Flush IS happening (17 chunks created) - Last getPos() returns 1252 - 8 more bytes written after last getPos() (writes #466-470) - Final file size: 1260 bytes (correct!) - But Parquet expects: 1338 bytes (1260 + 78) The 8 bytes after last getPos() are the footer length + magic bytes. But this doesn't explain the 78-byte discrepancy. Need to investigate further - the issue is more complex than simple flush timing. * fixing hdfs3 * tests not needed now * clean up tests * clean * remove hdfs2 * less logs * less logs * disable * security fix * Update pom.xml * Update pom.xml * purge * Update pom.xml * Update SeaweedHadoopInputStream.java * Update spark-integration-tests.yml * Update spark-integration-tests.yml * treat as root * clean up * clean up * remove try catch	4 weeks ago
Chris Lu	64dcbbb25b	test read write by s3fs and PyArrow native file system for s3 (#7520) * test read write by s3fs and PyArrow native file system for s3 * address comments * add github action	4 weeks ago
Chris Lu	c1b8d4bf0d	S3: adds FilerClient to use cached volume id (#7518) * adds FilerClient to use cached volume id * refactor: MasterClient embeds vidMapClient to eliminate ~150 lines of duplication - Create masterVolumeProvider that implements VolumeLocationProvider - MasterClient now embeds vidMapClient instead of maintaining duplicate cache logic - Removed duplicate methods: LookupVolumeIdsWithFallback, getStableVidMap, etc. - MasterClient still receives real-time updates via KeepConnected streaming - Updates call inherited addLocation/deleteLocation from vidMapClient - Benefits: DRY principle, shared singleflight, cache chain logic reused - Zero behavioral changes - only architectural improvement * refactor: mount uses FilerClient for efficient volume location caching - Add configurable vidMap cache size (default: 5 historical snapshots) - Add FilerClientOption struct for clean configuration * GrpcTimeout: default 5 seconds (prevents hanging requests) * UrlPreference: PreferUrl or PreferPublicUrl * CacheSize: number of historical vidMap snapshots (for volume moves) - NewFilerClient uses option struct for better API extensibility - Improved error handling in filerVolumeProvider.LookupVolumeIds: * Distinguish genuine 'not found' from communication failures * Log volumes missing from filer response * Return proper error context with volume count * Document that filer Locations lacks Error field (unlike master) - FilerClient.GetLookupFileIdFunction() handles URL preference automatically - Mount (WFS) creates FilerClient with appropriate options - Benefits for weed mount: * Singleflight: Deduplicates concurrent volume lookups * Cache history: Old volume locations available briefly when volumes move * Configurable cache depth: Tune for different deployment environments * Battle-tested vidMap cache with cache chain * Better concurrency handling with timeout protection * Improved error visibility and debugging - Old filer.LookupFn() kept for backward compatibility - Performance improvement for mount operations with high concurrency * fix: prevent vidMap swap race condition in LookupFileIdWithFallback - Hold vidMapLock.RLock() during entire vm.LookupFileId() call - Prevents resetVidMap() from swapping vidMap mid-operation - Ensures atomic access to the current vidMap instance - Added documentation warnings to getStableVidMap() about swap risks - Enhanced withCurrentVidMap() documentation for clarity This fixes a subtle race condition where: 1. Thread A: acquires lock, gets vm pointer, releases lock 2. Thread B: calls resetVidMap(), swaps vc.vidMap 3. Thread A: calls vm.LookupFileId() on old/stale vidMap While the old vidMap remains valid (in cache chain), holding the lock ensures we consistently use the current vidMap for the entire operation. * fix: FilerClient supports multiple filer addresses for high availability Critical fix: FilerClient now accepts []ServerAddress instead of single address - Prevents mount failure when first filer is down (regression fix) - Implements automatic failover to remaining filers - Uses round-robin with atomic index tracking (same pattern as WFS.WithFilerClient) - Retries all configured filers before giving up - Updates successful filer index for future requests Changes: - NewFilerClient([]pb.ServerAddress, ...) instead of (pb.ServerAddress, ...) - filerVolumeProvider references FilerClient for failover access - LookupVolumeIds tries all filers with util.Retry pattern - Mount passes all option.FilerAddresses for HA - S3 wraps single filer in slice for API consistency This restores the high availability that existed in the old implementation where mount would automatically failover between configured filers. * fix: restore leader change detection in KeepConnected stream loop Critical fix: Leader change detection was accidentally removed from the streaming loop - Master can announce leader changes during an active KeepConnected stream - Without this check, client continues talking to non-leader until connection breaks - This can lead to stale data or operational errors The check needs to be in TWO places: 1. Initial response (lines 178-187): Detect redirect on first connect 2. Stream loop (lines 203-209): Detect leader changes during active stream Restored the loop check that was accidentally removed during refactoring. This ensures the client immediately reconnects to new leader when announced. * improve: address code review findings on error handling and documentation 1. Master provider now preserves per-volume errors - Surface detailed errors from master (e.g., misconfiguration, deletion) - Return partial results with aggregated errors using errors.Join - Callers can now distinguish specific volume failures from general errors - Addresses issue of losing vidLoc.Error details 2. Document GetMaster initialization contract - Add comprehensive documentation explaining blocking behavior - Clarify that KeepConnectedToMaster must be started first - Provide typical initialization pattern example - Prevent confusing timeouts during warm-up 3. Document partial results API contract - LookupVolumeIdsWithFallback explicitly documents partial results - Clear examples of how to handle result + error combinations - Helps prevent callers from discarding valid partial results 4. Add safeguards to legacy filer.LookupFn - Add deprecation warning with migration guidance - Implement simple 10,000 entry cache limit - Log warning when limit reached - Recommend wdclient.FilerClient for new code - Prevents unbounded memory growth in long-running processes These changes improve API clarity and operational safety while maintaining backward compatibility. * fix: handle partial results correctly in LookupVolumeIdsWithFallback callers Two callers were discarding partial results by checking err before processing the result map. While these are currently single-volume lookups (so partial results aren't possible), the code was fragile and would break if we ever batched multiple volumes together. Changes: - Check result map FIRST, then conditionally check error - If volume is found in result, use it (ignore errors about other volumes) - If volume is NOT found and err != nil, include error context with %w - Add defensive comments explaining the pattern for future maintainers This makes the code: 1. Correct for future batched lookups 2. More informative (preserves underlying error details) 3. Consistent with filer_grpc_server.go which already handles this correctly Example: If looking up ["1", "2", "999"] and only 999 fails, callers looking for volumes 1 or 2 will succeed instead of failing unnecessarily. * improve: address remaining code review findings 1. Lazy initialize FilerClient in mount for proxy-only setups - Only create FilerClient when VolumeServerAccess != "filerProxy" - Avoids wasted work when all reads proxy through filer - filerClient is nil for proxy mode, initialized for direct access 2. Fix inaccurate deprecation comment in filer.LookupFn - Updated comment to reflect current behavior (10k bounded cache) - Removed claim of "unbounded growth" after adding size limit - Still directs new code to wdclient.FilerClient for better features 3. Audit all MasterClient usages for KeepConnectedToMaster - Verified all production callers start KeepConnectedToMaster early - Filer, Shell, Master, Broker, Benchmark, Admin all correct - IAM creates MasterClient but never uses it (harmless) - Test code doesn't need KeepConnectedToMaster (mocks) All callers properly follow the initialization pattern documented in GetMaster(), preventing unexpected blocking or timeouts. * fix: restore observability instrumentation in MasterClient During the refactoring, several important stats counters and logging statements were accidentally removed from tryConnectToMaster. These are critical for monitoring and debugging the health of master client connections. Restored instrumentation: 1. stats.MasterClientConnectCounter("total") - tracks all connection attempts 2. stats.MasterClientConnectCounter(FailedToKeepConnected) - when KeepConnected stream fails 3. stats.MasterClientConnectCounter(FailedToReceive) - when Recv() fails in loop 4. stats.MasterClientConnectCounter(Failed) - when overall gprcErr occurs 5. stats.MasterClientConnectCounter(OnPeerUpdate) - when peer updates detected Additionally restored peer update logging: - "+ filer@host noticed group.type address" for node additions - "- filer@host noticed group.type address" for node removals - Only logs updates matching the client's FilerGroup for noise reduction This information is valuable for: - Monitoring cluster health and connection stability - Debugging cluster membership changes - Tracking master failover and reconnection patterns - Identifying network issues between clients and masters No functional changes - purely observability restoration. * improve: implement gRPC-aware retry for FilerClient volume lookups The previous implementation used util.Retry which only retries errors containing the string "transport". This is insufficient for handling the full range of transient gRPC errors. Changes: 1. Added isRetryableGrpcError() to properly inspect gRPC status codes - Retries: Unavailable, DeadlineExceeded, ResourceExhausted, Aborted - Falls back to string matching for non-gRPC network errors 2. Replaced util.Retry with custom retry loop - 3 attempts with exponential backoff (1s, 1.5s, 2.25s) - Tries all N filers on each attempt (N*3 total attempts max) - Fast-fails on non-retryable errors (NotFound, PermissionDenied, etc.) 3. Improved logging - Shows both filer attempt (x/N) and retry attempt (y/3) - Logs retry reason and wait time for debugging Benefits: - Better handling of transient gRPC failures (server restarts, load spikes) - Faster failure for permanent errors (no wasted retries) - More informative logs for troubleshooting - Maintains existing HA failover across multiple filers Example: If all 3 filers return Unavailable (server overload): - Attempt 1: try all 3 filers, wait 1s - Attempt 2: try all 3 filers, wait 1.5s - Attempt 3: try all 3 filers, fail Example: If filer returns NotFound (volume doesn't exist): - Attempt 1: try all 3 filers, fast-fail (no retry) * fmt * improve: add circuit breaker to skip known-unhealthy filers The previous implementation tried all filers on every failure, including known-unhealthy ones. This wasted time retrying permanently down filers. Problem scenario (3 filers, filer0 is down): - Last successful: filer1 (saved as filerIndex=1) - Next lookup when filer1 fails: Retry 1: filer1(fail) → filer2(fail) → filer0(fail, wastes 5s timeout) Retry 2: filer1(fail) → filer2(fail) → filer0(fail, wastes 5s timeout) Retry 3: filer1(fail) → filer2(fail) → filer0(fail, wastes 5s timeout) Total wasted: 15 seconds on known-bad filer! Solution: Circuit breaker pattern - Track consecutive failures per filer (atomic int32) - Skip filers with 3+ consecutive failures - Re-check unhealthy filers every 30 seconds - Reset failure count on success New behavior: - filer0 fails 3 times → marked unhealthy - Future lookups skip filer0 for 30 seconds - After 30s, re-check filer0 (allows recovery) - If filer0 succeeds, reset failure count to 0 Benefits: 1. Avoids wasting time on known-down filers 2. Still sticks to last healthy filer (via filerIndex) 3. Allows recovery (30s re-check window) 4. No configuration needed (automatic) Implementation details: - filerHealth struct tracks failureCount (atomic) + lastFailureTime - shouldSkipUnhealthyFiler(): checks if we should skip this filer - recordFilerSuccess(): resets failure count to 0 - recordFilerFailure(): increments count, updates timestamp - Logs when skipping unhealthy filers (V(2) level) Example with circuit breaker: - filer0 down, saved filerIndex=1 (filer1 healthy) - Lookup 1: filer1(ok) → Done (0.01s) - Lookup 2: filer1(fail) → filer2(ok) → Done, save filerIndex=2 (0.01s) - Lookup 3: filer2(fail) → skip filer0 (unhealthy) → filer1(ok) → Done (0.01s) Much better than wasting 15s trying filer0 repeatedly! * fix: OnPeerUpdate should only process updates for matching FilerGroup Critical bug: The OnPeerUpdate callback was incorrectly moved outside the FilerGroup check when restoring observability instrumentation. This caused clients to process peer updates for ALL filer groups, not just their own. Problem: Before: mc.OnPeerUpdate only called for update.FilerGroup == mc.FilerGroup Bug: mc.OnPeerUpdate called for ALL updates regardless of FilerGroup Impact: - Multi-tenant deployments with separate filer groups would see cross-group updates (e.g., group A clients processing group B updates) - Could cause incorrect cluster membership tracking - OnPeerUpdate handlers (like Filer's DLM ring updates) would receive irrelevant updates from other groups Example scenario: Cluster has two filer groups: "production" and "staging" Production filer connects with FilerGroup="production" Incorrect behavior (bug): - Receives "staging" group updates - Incorrectly adds staging filers to production DLM ring - Cross-tenant data access issues Correct behavior (fixed): - Only receives "production" group updates - Only adds production filers to production DLM ring - Proper isolation between groups Fix: Moved mc.OnPeerUpdate(update, time.Now()) back INSIDE the FilerGroup check where it belongs, matching the original implementation. The logging and stats counter were already correctly scoped to matching FilerGroup, so they remain inside the if block as intended. * improve: clarify Aborted error handling in volume lookups Added documentation and logging to address the concern that codes.Aborted might not always be retryable in all contexts. Context-specific justification for treating Aborted as retryable: Volume location lookups (LookupVolume RPC) are simple, read-only operations: - No transactions - No write conflicts - No application-level state changes - Idempotent (safe to retry) In this context, Aborted is most likely caused by: - Filer restarting/recovering (transient) - Connection interrupted mid-request (transient) - Server-side resource cleanup (transient) NOT caused by: - Application-level conflicts (no writes) - Transaction failures (no transactions) - Logical errors (read-only lookup) Changes: 1. Added detailed comment explaining the context-specific reasoning 2. Added V(1) logging when treating Aborted as retryable - Helps detect misclassification if it occurs - Visible in verbose logs for troubleshooting 3. Split switch statement for clarity (one case per line) If future analysis shows Aborted should not be retried, operators will now have visibility via logs to make that determination. The logging provides evidence for future tuning decisions. Alternative approaches considered but not implemented: - Removing Aborted entirely (too conservative for read-only ops) - Message content inspection (adds complexity, no known patterns yet) - Different handling per RPC type (premature optimization) * fix: IAM server must start KeepConnectedToMaster for masterClient usage The IAM server creates and uses a MasterClient but never started KeepConnectedToMaster, which could cause blocking if IAM config files have chunks requiring volume lookups. Problem flow: NewIamApiServerWithStore() → creates masterClient → ❌ NEVER starts KeepConnectedToMaster GetS3ApiConfigurationFromFiler() → filer.ReadEntry(iama.masterClient, ...) → StreamContent(masterClient, ...) if file has chunks → masterClient.GetLookupFileIdFunction() → GetMaster(ctx) ← BLOCKS indefinitely waiting for connection! While IAM config files (identity & policies) are typically small and stored inline without chunks, the code path exists and would block if the files ever had chunks. Fix: Start KeepConnectedToMaster in background goroutine right after creating masterClient, following the documented pattern: mc := wdclient.NewMasterClient(...) go mc.KeepConnectedToMaster(ctx) This ensures masterClient is usable if ReadEntry ever needs to stream chunked content from volume servers. Note: This bug was dormant because IAM config files are small (<256 bytes) and SeaweedFS stores small files inline in Entry.Content, not as chunks. The bug would only manifest if: - IAM config grew > 256 bytes (inline threshold) - Config was stored as chunks on volume servers - ReadEntry called StreamContent - GetMaster blocked indefinitely Now all 9 production MasterClient instances correctly follow the pattern. * fix: data race on filerHealth.lastFailureTime in circuit breaker The circuit breaker tracked lastFailureTime as time.Time, which was written in recordFilerFailure and read in shouldSkipUnhealthyFiler without synchronization, causing a data race. Data race scenario: Goroutine 1: recordFilerFailure(0) health.lastFailureTime = time.Now() // ❌ unsynchronized write Goroutine 2: shouldSkipUnhealthyFiler(0) time.Since(health.lastFailureTime) // ❌ unsynchronized read → RACE DETECTED by -race detector Fix: Changed lastFailureTime from time.Time to int64 (lastFailureTimeNs) storing Unix nanoseconds for atomic access: Write side (recordFilerFailure): atomic.StoreInt64(&health.lastFailureTimeNs, time.Now().UnixNano()) Read side (shouldSkipUnhealthyFiler): lastFailureNs := atomic.LoadInt64(&health.lastFailureTimeNs) if lastFailureNs == 0 { return false } // Never failed lastFailureTime := time.Unix(0, lastFailureNs) time.Since(lastFailureTime) > 30*time.Second Benefits: - Atomic reads/writes (no data race) - Efficient (int64 is 8 bytes, always atomic on 64-bit systems) - Zero value (0) naturally means "never failed" - No mutex needed (lock-free circuit breaker) Note: sync/atomic was already imported for failureCount, so no new import needed. * fix: create fresh timeout context for each filer retry attempt The timeout context was created once at function start and reused across all retry attempts, causing subsequent retries to run with progressively shorter (or expired) deadlines. Problem flow: Line 244: timeoutCtx, cancel := context.WithTimeout(ctx, 5s) defer cancel() Retry 1, filer 0: client.LookupVolume(timeoutCtx, ...) ← 5s available ✅ Retry 1, filer 1: client.LookupVolume(timeoutCtx, ...) ← 3s left Retry 1, filer 2: client.LookupVolume(timeoutCtx, ...) ← 0.5s left Retry 2, filer 0: client.LookupVolume(timeoutCtx, ...) ← EXPIRED! ❌ Result: Retries always fail with DeadlineExceeded, defeating the purpose of retries. Fix: Moved context.WithTimeout inside the per-filer loop, creating a fresh timeout context for each attempt: for x := 0; x < n; x++ { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) err := pb.WithGrpcFilerClient(..., func(client) { resp, err := client.LookupVolume(timeoutCtx, ...) ... }) cancel() // Clean up immediately after call } Benefits: - Each filer attempt gets full fc.grpcTimeout (default 5s) - Retries actually have time to complete - No context leaks (cancel called after each attempt) - More predictable timeout behavior Example with fix: Retry 1, filer 0: fresh 5s timeout ✅ Retry 1, filer 1: fresh 5s timeout ✅ Retry 2, filer 0: fresh 5s timeout ✅ Total max time: 3 retries × 3 filers × 5s = 45s (plus backoff) Note: The outer ctx (from caller) still provides overall cancellation if the caller cancels or times out the entire operation. * fix: always reset vidMap cache on master reconnection The previous refactoring removed the else block that resets vidMap when the first message from a newly connected master is not a VolumeLocation. Problem scenario: 1. Client connects to master-1 and builds vidMap cache 2. Master-1 fails, client connects to master-2 3. First message from master-2 is a ClusterNodeUpdate (not VolumeLocation) 4. Old code: vidMap is reset and updated ✅ 5. New code: vidMap is NOT reset ❌ 6. Result: Client uses stale cache from master-1 → data access errors Example flow with bug: Connect to master-2 First message: ClusterNodeUpdate {filer.x added} → No resetVidMap() call → vidMap still has master-1's stale volume locations → Client reads from wrong volume servers → 404 errors Fix: Restored the else block that resets vidMap when first message is not a VolumeLocation: if resp.VolumeLocation != nil { // ... check leader, reset, and update ... } else { // First message is ClusterNodeUpdate or other type // Must still reset to avoid stale data mc.resetVidMap() } This ensures the cache is always cleared when establishing a new master connection, regardless of what the first message type is. Root cause: During the vidMapClient refactoring, this else block was accidentally dropped, making failover behavior fragile and non-deterministic (depends on which message type arrives first from the new master). Impact: - High severity for master failover scenarios - Could cause read failures, 404s, or wrong data access - Only manifests when first message is not VolumeLocation * fix: goroutine and connection leak in IAM server shutdown The IAM server's KeepConnectedToMaster goroutine used context.Background(), which is non-cancellable, causing the goroutine and its gRPC connections to leak on server shutdown. Problem: go masterClient.KeepConnectedToMaster(context.Background()) - context.Background() never cancels - KeepConnectedToMaster goroutine runs forever - gRPC connection to master stays open - No way to stop cleanly on server shutdown Result: Resource leaks when IAM server is stopped Fix: 1. Added shutdownContext and shutdownCancel to IamApiServer struct 2. Created cancellable context in NewIamApiServerWithStore: shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) 3. Pass shutdownCtx to KeepConnectedToMaster: go masterClient.KeepConnectedToMaster(shutdownCtx) 4. Added Shutdown() method to invoke cancel: func (iama *IamApiServer) Shutdown() { if iama.shutdownCancel != nil { iama.shutdownCancel() } } 5. Stored masterClient reference on IamApiServer for future use Benefits: - Goroutine stops cleanly when Shutdown() is called - gRPC connections are closed properly - No resource leaks on server restart/stop - Shutdown() is idempotent (safe to call multiple times) Usage (for future graceful shutdown): iamServer, _ := iamapi.NewIamApiServer(...) defer iamServer.Shutdown() // or in signal handler: sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGTERM, syscall.SIGINT) go func() { <-sigChan iamServer.Shutdown() os.Exit(0) }() Note: Current command implementations (weed/command/iam.go) don't have shutdown paths yet, but this makes IAM server ready for proper lifecycle management when that infrastructure is added. * refactor: remove unnecessary KeepMasterClientConnected wrapper in filer The Filer.KeepMasterClientConnected() method was an unnecessary wrapper that just forwarded to MasterClient.KeepConnectedToMaster(). This wrapper added no value and created inconsistency with other components that call KeepConnectedToMaster directly. Removed: filer.go:178-180 func (fs *Filer) KeepMasterClientConnected(ctx context.Context) { fs.MasterClient.KeepConnectedToMaster(ctx) } Updated caller: filer_server.go:181 - go fs.filer.KeepMasterClientConnected(context.Background()) + go fs.filer.MasterClient.KeepConnectedToMaster(context.Background()) Benefits: - Consistent with other components (S3, IAM, Shell, Mount) - Removes unnecessary indirection - Clearer that KeepConnectedToMaster runs in background goroutine - Follows the documented pattern from MasterClient.GetMaster() Note: shell/commands.go was verified and already correctly starts KeepConnectedToMaster in a background goroutine (shell_liner.go:51): go commandEnv.MasterClient.KeepConnectedToMaster(ctx) * fix: use client ID instead of timeout for gRPC signature parameter The pb.WithGrpcFilerClient signature parameter is meant to be a client identifier for logging and tracking (added as 'sw-client-id' gRPC metadata in streaming mode), not a timeout value. Problem: timeoutMs := int32(fc.grpcTimeout.Milliseconds()) // 5000 (5 seconds) err := pb.WithGrpcFilerClient(false, timeoutMs, filerAddress, ...) - Passing timeout (5000ms) as signature/client ID - Misuse of API: signature should be a unique client identifier - Timeout is already handled by timeoutCtx passed to gRPC call - Inconsistent with other callers (all use 0 or proper client ID) How WithGrpcFilerClient uses signature parameter: func WithGrpcClient(..., signature int32, ...) { if streamingMode && signature != 0 { md := metadata.New(map[string]string{"sw-client-id": fmt.Sprintf("%d", signature)}) ctx = metadata.NewOutgoingContext(ctx, md) } ... } It's for client identification, not timeout control! Fix: 1. Added clientId int32 field to FilerClient struct 2. Initialize with rand.Int31() in NewFilerClient for unique ID 3. Removed timeoutMs variable (and misleading comment) 4. Use fc.clientId in pb.WithGrpcFilerClient call Before: err := pb.WithGrpcFilerClient(false, timeoutMs, ...) ^^^^^^^^^ Wrong! (5000) After: err := pb.WithGrpcFilerClient(false, fc.clientId, ...) ^^^^^^^^^^^^ Correct! (random int31) Benefits: - Correct API usage (signature = client ID, not timeout) - Timeout still works via timeoutCtx (unchanged) - Consistent with other pb.WithGrpcFilerClient callers - Enables proper client tracking on filer side via gRPC metadata - Each FilerClient instance has unique ID for debugging Examples of correct usage elsewhere: weed/iamapi/iamapi_server.go:145 pb.WithGrpcFilerClient(false, 0, ...) weed/command/s3.go:215 pb.WithGrpcFilerClient(false, 0, ...) weed/shell/commands.go:110 pb.WithGrpcFilerClient(streamingMode, 0, ...) All use 0 (or a proper signature), not a timeout value. * fix: add timeout to master volume lookup to prevent indefinite blocking The masterVolumeProvider.LookupVolumeIds method was using the context directly without a timeout, which could cause it to block indefinitely if the master is slow to respond or unreachable. Problem: err := pb.WithMasterClient(false, p.masterClient.GetMaster(ctx), ...) resp, err := client.LookupVolume(ctx, &master_pb.LookupVolumeRequest{...}) - No timeout on gRPC call to master - Could block indefinitely if master is unresponsive - Inconsistent with FilerClient which uses 5s timeout - This is a fallback path (cache miss) but still needs protection Scenarios where this could hang: 1. Master server under heavy load (slow response) 2. Network issues between client and master 3. Master server hung or deadlocked 4. Master in process of shutting down Fix: timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() err := pb.WithMasterClient(false, p.masterClient.GetMaster(timeoutCtx), ...) resp, err := client.LookupVolume(timeoutCtx, &master_pb.LookupVolumeRequest{...}) Benefits: - Prevents indefinite blocking on master lookup - Consistent with FilerClient timeout pattern (5 seconds) - Faster failure detection when master is unresponsive - Caller's context still honored (timeout is in addition, not replacement) - Improves overall system resilience Note: 5 seconds is a reasonable default for volume lookups: - Long enough for normal master response (~10-50ms) - Short enough to fail fast on issues - Matches FilerClient's grpcTimeout default * purge * refactor: address code review feedback on comments and style Fixed several code quality issues identified during review: 1. Corrected backoff algorithm description in filer_client.go: - Changed "Exponential backoff" to "Multiplicative backoff with 1.5x factor" - The formula waitTime * 3/2 produces 1s, 1.5s, 2.25s, not exponential 2^n - More accurate terminology prevents confusion 2. Removed redundant nil check in vidmap_client.go: - After the for loop, node is guaranteed to be non-nil - Loop either returns early or assigns non-nil value to node - Simplified: if node != nil { node.cache.Store(nil) } → node.cache.Store(nil) 3. Added startup logging to IAM server for consistency: - Log when master client connection starts - Matches pattern in S3ApiServer (line 100 in s3api_server.go) - Improves operational visibility during startup - Added missing glog import 4. Fixed indentation in filer/reader_at.go: - Lines 76-91 had incorrect indentation (extra tab level) - Line 93 also misaligned - Now properly aligned with surrounding code 5. Updated deprecation comment to follow Go convention: - Changed "DEPRECATED:" to "Deprecated:" (standard Go format) - Tools like staticcheck and IDEs recognize the standard format - Enables automated deprecation warnings in tooling - Better developer experience All changes are cosmetic and do not affect functionality. * fmt * refactor: make circuit breaker parameters configurable in FilerClient The circuit breaker failure threshold (3) and reset timeout (30s) were hardcoded, making it difficult to tune the client's behavior in different deployment environments without modifying the code. Problem: func shouldSkipUnhealthyFiler(index int32) bool { if failureCount < 3 { // Hardcoded threshold return false } if time.Since(lastFailureTime) > 30*time.Second { // Hardcoded timeout return false } } Different environments have different needs: - High-traffic production: may want lower threshold (2) for faster failover - Development/testing: may want higher threshold (5) to tolerate flaky networks - Low-latency services: may want shorter reset timeout (10s) - Batch processing: may want longer reset timeout (60s) Solution: 1. Added fields to FilerClientOption: - FailureThreshold int32 (default: 3) - ResetTimeout time.Duration (default: 30s) 2. Added fields to FilerClient: - failureThreshold int32 - resetTimeout time.Duration 3. Applied defaults in NewFilerClient with option override: failureThreshold := int32(3) resetTimeout := 30 * time.Second if opt.FailureThreshold > 0 { failureThreshold = opt.FailureThreshold } if opt.ResetTimeout > 0 { resetTimeout = opt.ResetTimeout } 4. Updated shouldSkipUnhealthyFiler to use configurable values: if failureCount < fc.failureThreshold { ... } if time.Since(lastFailureTime) > fc.resetTimeout { ... } Benefits: ✓ Tunable for different deployment environments ✓ Backward compatible (defaults match previous hardcoded values) ✓ No breaking changes to existing code ✓ Better maintainability and flexibility Example usage: // Aggressive failover for low-latency production fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ FailureThreshold: 2, ResetTimeout: 10 * time.Second, }) // Tolerant of flaky networks in development fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ FailureThreshold: 5, ResetTimeout: 60 * time.Second, }) * retry parameters * refactor: make retry and timeout parameters configurable Made retry logic and gRPC timeouts configurable across FilerClient and MasterClient to support different deployment environments and network conditions. Problem 1: Hardcoded retry parameters in FilerClient waitTime := time.Second // Fixed at 1s maxRetries := 3 // Fixed at 3 attempts waitTime = waitTime * 3 / 2 // Fixed 1.5x multiplier Different environments have different needs: - Unstable networks: may want more retries (5) with longer waits (2s) - Low-latency production: may want fewer retries (2) with shorter waits (500ms) - Batch processing: may want exponential backoff (2x) instead of 1.5x Problem 2: Hardcoded gRPC timeout in MasterClient timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second) Master lookups may need different timeouts: - High-latency cross-region: may need 10s timeout - Local network: may use 2s timeout for faster failure detection Solution for FilerClient: 1. Added fields to FilerClientOption: - MaxRetries int (default: 3) - InitialRetryWait time.Duration (default: 1s) - RetryBackoffFactor float64 (default: 1.5) 2. Added fields to FilerClient: - maxRetries int - initialRetryWait time.Duration - retryBackoffFactor float64 3. Updated LookupVolumeIds to use configurable values: waitTime := fc.initialRetryWait maxRetries := fc.maxRetries for retry := 0; retry < maxRetries; retry++ { ... waitTime = time.Duration(float64(waitTime) * fc.retryBackoffFactor) } Solution for MasterClient: 1. Added grpcTimeout field to MasterClient (default: 5s) 2. Initialize in NewMasterClient with 5 * time.Second default 3. Updated masterVolumeProvider to use p.masterClient.grpcTimeout Benefits: ✓ Tunable for different network conditions and deployment scenarios ✓ Backward compatible (defaults match previous hardcoded values) ✓ No breaking changes to existing code ✓ Consistent configuration pattern across FilerClient and MasterClient Example usage: // Fast-fail for low-latency production with stable network fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ MaxRetries: 2, InitialRetryWait: 500 * time.Millisecond, RetryBackoffFactor: 2.0, // Exponential backoff GrpcTimeout: 2 * time.Second, }) // Patient retries for unstable network or batch processing fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ MaxRetries: 5, InitialRetryWait: 2 * time.Second, RetryBackoffFactor: 1.5, GrpcTimeout: 10 * time.Second, }) Note: MasterClient timeout is currently set at construction time and not user-configurable via NewMasterClient parameters. Future enhancement could add a MasterClientOption struct similar to FilerClientOption. * fix: rename vicCacheLock to vidCacheLock for consistency Fixed typo in variable name for better code consistency and readability. Problem: vidCache := make(map[string]*filer_pb.Locations) var vicCacheLock sync.RWMutex // Typo: vic instead of vid vicCacheLock.RLock() locations, found := vidCache[vid] vicCacheLock.RUnlock() The variable name 'vicCacheLock' is inconsistent with 'vidCache'. Both should use 'vid' prefix (volume ID) not 'vic'. Fix: Renamed all 5 occurrences: - var vicCacheLock → var vidCacheLock (line 56) - vicCacheLock.RLock() → vidCacheLock.RLock() (line 62) - vicCacheLock.RUnlock() → vidCacheLock.RUnlock() (line 64) - vicCacheLock.Lock() → vidCacheLock.Lock() (line 81) - vicCacheLock.Unlock() → vidCacheLock.Unlock() (line 91) Benefits: ✓ Consistent variable naming convention ✓ Clearer intent (volume ID cache lock) ✓ Better code readability ✓ Easier code navigation * fix: use defer cancel() with anonymous function for proper context cleanup Fixed context cancellation to use defer pattern correctly in loop iteration. Problem: for x := 0; x < n; x++ { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) err := pb.WithGrpcFilerClient(...) cancel() // Only called on normal return, not on panic } Issues with original approach: 1. If pb.WithGrpcFilerClient panics, cancel() is never called → context leak 2. If callback returns early (though unlikely here), cleanup might be missed 3. Not following Go best practices for context.WithTimeout usage Problem with naive defer in loop: for x := 0; x < n; x++ { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) defer cancel() // ❌ WRONG: All defers accumulate until function returns } In Go, defer executes when the surrounding *function* returns, not when the loop iteration ends. This would accumulate n deferred cancel() calls and leak contexts until LookupVolumeIds returns. Solution: Wrap in anonymous function for x := 0; x < n; x++ { err := func() error { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) defer cancel() // ✅ Executes when anonymous function returns (per iteration) return pb.WithGrpcFilerClient(...) }() } Benefits: ✓ Context always cancelled, even on panic ✓ defer executes after each iteration (not accumulated) ✓ Follows Go best practices for context.WithTimeout ✓ No resource leaks during retry loop execution ✓ Cleaner error handling Reference: Go documentation for context.WithTimeout explicitly shows: ctx, cancel := context.WithTimeout(...) defer cancel() This is the idiomatic pattern that should always be followed. * Can't use defer directly in loop * improve: add data center preference and URL shuffling for consistent performance Added missing data center preference and load distribution (URL shuffling) to ensure consistent performance and behavior across all code paths. Problem 1: PreferPublicUrl path missing DC preference and shuffling Location: weed/wdclient/filer_client.go lines 184-192 The custom PreferPublicUrl implementation was simply iterating through locations and building URLs without considering: 1. Data center proximity (latency optimization) 2. Load distribution across volume servers Before: for _, loc := range locations { url := loc.PublicUrl if url == "" { url = loc.Url } fullUrls = append(fullUrls, "http://"+url+"/"+fileId) } return fullUrls, nil After: var sameDcUrls, otherDcUrls []string dataCenter := fc.GetDataCenter() for _, loc := range locations { url := loc.PublicUrl if url == "" { url = loc.Url } httpUrl := "http://" + url + "/" + fileId if dataCenter != "" && dataCenter == loc.DataCenter { sameDcUrls = append(sameDcUrls, httpUrl) } else { otherDcUrls = append(otherDcUrls, httpUrl) } } rand.Shuffle(len(sameDcUrls), ...) rand.Shuffle(len(otherDcUrls), ...) fullUrls = append(sameDcUrls, otherDcUrls...) Problem 2: Cache miss path missing URL shuffling Location: weed/wdclient/vidmap_client.go lines 95-108 The cache miss path (fallback lookup) was missing URL shuffling, while the cache hit path (vm.LookupFileId) already shuffles URLs. This inconsistency meant: - Cache hit: URLs shuffled → load distributed - Cache miss: URLs not shuffled → first server always hit Before: var sameDcUrls, otherDcUrls []string // ... build URLs ... fullUrls = append(sameDcUrls, otherDcUrls...) return fullUrls, nil After: var sameDcUrls, otherDcUrls []string // ... build URLs ... rand.Shuffle(len(sameDcUrls), ...) rand.Shuffle(len(otherDcUrls), ...) fullUrls = append(sameDcUrls, otherDcUrls...) return fullUrls, nil Benefits: ✓ Reduced latency by preferring same-DC volume servers ✓ Even load distribution across all volume servers ✓ Consistent behavior between cache hit/miss paths ✓ Consistent behavior between PreferUrl and PreferPublicUrl ✓ Matches behavior of existing vidMap.LookupFileId implementation Impact on performance: - Lower read latency (same-DC preference) - Better volume server utilization (load spreading) - No single volume server becomes a hotspot Note: Added math/rand import to vidmap_client.go for shuffle support. * Update weed/wdclient/masterclient.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * improve: call IAM server Shutdown() for best-effort cleanup Added call to iamApiServer.Shutdown() to ensure cleanup happens when possible, and documented the limitations of the current approach. Problem: The Shutdown() method was defined in IamApiServer but never called anywhere, meaning the KeepConnectedToMaster goroutine would continue running even when the IAM server stopped, causing resource leaks. Changes: 1. Store iamApiServer instance in weed/command/iam.go - Changed: _, iamApiServer_err := iamapi.NewIamApiServer(...) - To: iamApiServer, iamApiServer_err := iamapi.NewIamApiServer(...) 2. Added defer call for best-effort cleanup - defer iamApiServer.Shutdown() - This will execute if startIamServer() returns normally 3. Added logging in Shutdown() method - Log when shutdown is triggered for visibility 4. Documented limitations and future improvements - Added note that defer only works for normal function returns - SeaweedFS commands don't currently have signal handling - Suggested future enhancement: add SIGTERM/SIGINT handling Current behavior: - ✓ Cleanup happens if HTTP server fails to start (glog.Fatalf path) - ✓ Cleanup happens if Serve() returns with error (unlikely) - ✗ Cleanup does NOT happen on SIGTERM/SIGINT (process killed) The last case is a limitation of the current command architecture - all SeaweedFS commands (s3, filer, volume, master, iam) lack signal handling for graceful shutdown. This is a systemic issue that affects all services. Future enhancement: To properly handle SIGTERM/SIGINT, the command layer would need: sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGTERM, syscall.SIGINT) go func() { httpServer.Serve(listener) // Non-blocking }() <-sigChan glog.V(0).Infof("Received shutdown signal") iamApiServer.Shutdown() httpServer.Shutdown(context.Background()) This would require refactoring the command structure for all services, which is out of scope for this change. Benefits of current approach: ✓ Best-effort cleanup (better than nothing) ✓ Proper cleanup in error paths ✓ Documented for future improvement ✓ Consistent with how other SeaweedFS services handle lifecycle * data racing in test --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>	1 month ago
dependabot[bot]	7dce429e61	chore(deps): bump golang.org/x/crypto from 0.43.0 to 0.45.0 (#7511) * chore(deps): bump golang.org/x/crypto from 0.43.0 to 0.45.0 Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.43.0 to 0.45.0. - [Commits](https://github.com/golang/crypto/compare/v0.43.0...v0.45.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.45.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> * go mod tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chrislu <chris.lu@gmail.com>	1 month ago
dependabot[bot]	d2daea1552	chore(deps): bump golang.org/x/crypto from 0.43.0 to 0.45.0 in /test/kafka/kafka-client-loadtest (#7510) chore(deps): bump golang.org/x/crypto Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.43.0 to 0.45.0. - [Commits](https://github.com/golang/crypto/compare/v0.43.0...v0.45.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.45.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	1 month ago
Chris Lu	c6b6ea40e6	filer store: add foundationdb (#7178) * add foundationdb * Update foundationdb_store.go * fix * apply the patch * avoid panic on error * address comments * remove extra data * address comments * adds more debug messages * fix range listing * delete with prefix range; list with right start key * fix docker files * use the more idiomatic FoundationDB KeySelectors * address comments * proper errors * fix API versions * more efficient * recursive deletion * clean up * clean up * pagination, one transaction for deletion * error checking * Use fdb.Strinc() to compute the lexicographically next string and create a proper range * fix docker * Update README.md * delete in batches * delete in batches * fix build * add foundationdb build * Updated FoundationDB Version * Fixed glibc/musl Incompatibility (Alpine → Debian) * Update container_foundationdb_version.yml * build SeaweedFS * build tag * address comments * separate transaction * address comments * fix build * empty vs no data * fixes * add go test * Install FoundationDB client libraries * nil compare	1 month ago
Chris Lu	8be9e258fc	S3: Add tests for PyArrow with native S3 filesystem (#7508) * PyArrow native S3 filesystem * add sse-s3 tests * update * minor * ENABLE_SSE_S3 * Update test_pyarrow_native_s3.py * clean up * refactoring * Update test_pyarrow_native_s3.py	1 month ago
Chris Lu	ca84a8a713	S3: Directly read write volume servers (#7481) * Lazy Versioning Check, Conditional SSE Entry Fetch, HEAD Request Optimization * revert Reverted the conditional versioning check to always check versioning status Reverted the conditional SSE entry fetch to always fetch entry metadata Reverted the conditional versioning check to always check versioning status Reverted the conditional SSE entry fetch to always fetch entry metadata * Lazy Entry Fetch for SSE, Skip Conditional Header Check * SSE-KMS headers are present, this is not an SSE-C request (mutually exclusive) * SSE-C is mutually exclusive with SSE-S3 and SSE-KMS * refactor * Removed Premature Mutual Exclusivity Check * check for the presence of the X-Amz-Server-Side-Encryption header * not used * fmt * directly read write volume servers * HTTP Range Request Support * set header * md5 * copy object * fix sse * fmt * implement sse * sse continue * fixed the suffix range bug (bytes=-N for "last N bytes") * debug logs * Missing PartsCount Header * profiling * url encoding * test_multipart_get_part * headers * debug * adjust log level * handle part number * Update s3api_object_handlers.go * nil safety * set ModifiedTsNs * remove * nil check * fix sse header * same logic as filer * decode values * decode ivBase64 * s3: Fix SSE decryption JWT authentication and streaming errors Critical fix for SSE (Server-Side Encryption) test failures: 1. **JWT Authentication Bug** (Root Cause): - Changed from GenJwtForFilerServer to GenJwtForVolumeServer - S3 API now uses correct JWT when directly reading from volume servers - Matches filer's authentication pattern for direct volume access - Fixes 'unexpected EOF' and 500 errors in SSE tests 2. **Streaming Error Handling**: - Added error propagation in getEncryptedStreamFromVolumes goroutine - Use CloseWithError() to properly communicate stream failures - Added debug logging for streaming errors 3. **Response Header Timing**: - Removed premature WriteHeader(http.StatusOK) call - Let Go's http package write status automatically on first write - Prevents header lock when errors occur during streaming 4. **Enhanced SSE Decryption Debugging**: - Added IV/Key validation and logging for SSE-C, SSE-KMS, SSE-S3 - Better error messages for missing or invalid encryption metadata - Added glog.V(2) debugging for decryption setup This fixes SSE integration test failures where encrypted objects could not be retrieved due to volume server authentication failures. The JWT bug was causing volume servers to reject requests, resulting in truncated/empty streams (EOF) or internal errors. * s3: Fix SSE multipart upload metadata preservation Critical fix for SSE multipart upload test failures (SSE-C and SSE-KMS): **Root Cause - Incomplete SSE Metadata Copying**: The old code only tried to copy 'SeaweedFSSSEKMSKey' from the first part to the completed object. This had TWO bugs: 1. **Wrong Constant Name** (Key Mismatch Bug): - Storage uses: SeaweedFSSSEKMSKeyHeader = 'X-SeaweedFS-SSE-KMS-Key' - Old code read: SeaweedFSSSEKMSKey = 'x-seaweedfs-sse-kms-key' - Result: SSE-KMS metadata was NEVER copied → 500 errors 2. **Missing SSE-C and SSE-S3 Headers**: - SSE-C requires: IV, Algorithm, KeyMD5 - SSE-S3 requires: encrypted key data + standard headers - Old code: copied nothing for SSE-C/SSE-S3 → decryption failures **Fix - Complete SSE Header Preservation**: Now copies ALL SSE headers from first part to completed object: - SSE-C: SeaweedFSSSEIV, CustomerAlgorithm, CustomerKeyMD5 - SSE-KMS: SeaweedFSSSEKMSKeyHeader, AwsKmsKeyId, ServerSideEncryption - SSE-S3: SeaweedFSSSES3Key, ServerSideEncryption Applied consistently to all 3 code paths: 1. Versioned buckets (creates version file) 2. Suspended versioning (creates main object with null versionId) 3. Non-versioned buckets (creates main object) **Why This Is Correct**: The headers copied EXACTLY match what putToFiler stores during part upload (lines 496-521 in s3api_object_handlers_put.go). This ensures detectPrimarySSEType() can correctly identify encrypted multipart objects and trigger inline decryption with proper metadata. Fixes: TestSSEMultipartUploadIntegration (SSE-C and SSE-KMS subtests) * s3: Add debug logging for versioning state diagnosis Temporary debug logging to diagnose test_versioning_obj_plain_null_version_overwrite_suspended failure. Added glog.V(0) logging to show: 1. setBucketVersioningStatus: when versioning status is changed 2. PutObjectHandler: what versioning state is detected (Enabled/Suspended/none) 3. PutObjectHandler: which code path is taken (putVersionedObject vs putSuspendedVersioningObject) This will help identify if: - The versioning status is being set correctly in bucket config - The cache is returning stale/incorrect versioning state - The switch statement is correctly routing to suspended vs enabled handlers * s3: Enhanced versioning state tracing for suspended versioning diagnosis Added comprehensive logging across the entire versioning state flow: PutBucketVersioningHandler: - Log requested status (Enabled/Suspended) - Log when calling setBucketVersioningStatus - Log success/failure of status change setBucketVersioningStatus: - Log bucket and status being set - Log when config is updated - Log completion with error code updateBucketConfig: - Log versioning state being written to cache - Immediate cache verification after Set - Log if cache verification fails getVersioningState: - Log bucket name and state being returned - Log if object lock forces VersioningEnabled - Log errors This will reveal: 1. If PutBucketVersioning(Suspended) is reaching the handler 2. If the cache update succeeds 3. What state getVersioningState returns during PUT 4. Any cache consistency issues Expected to show why bucket still reports 'Enabled' after 'Suspended' call. * s3: Add SSE chunk detection debugging for multipart uploads Added comprehensive logging to diagnose why TestSSEMultipartUploadIntegration fails: detectPrimarySSEType now logs: 1. Total chunk count and extended header count 2. All extended headers with 'sse'/'SSE'/'encryption' in the name 3. For each chunk: index, SseType, and whether it has metadata 4. Final SSE type counts (SSE-C, SSE-KMS, SSE-S3) This will reveal if: - Chunks are missing SSE metadata after multipart completion - Extended headers are copied correctly from first part - The SSE detection logic is working correctly Expected to show if chunks have SseType=0 (none) or proper SSE types set. * s3: Trace SSE chunk metadata through multipart completion and retrieval Added end-to-end logging to track SSE chunk metadata lifecycle: **During Multipart Completion (filer_multipart.go)**: 1. Log finalParts chunks BEFORE mkFile - shows SseType and metadata 2. Log versionEntry.Chunks INSIDE mkFile callback - shows if mkFile preserves SSE info 3. Log success after mkFile completes **During GET Retrieval (s3api_object_handlers.go)**: 1. Log retrieved entry chunks - shows SseType and metadata after retrieval 2. Log detected SSE type result This will reveal at which point SSE chunk metadata is lost: - If finalParts have SSE metadata but versionEntry.Chunks don't → mkFile bug - If versionEntry.Chunks have SSE metadata but retrieved chunks don't → storage/retrieval bug - If chunks never have SSE metadata → multipart completion SSE processing bug Expected to show chunks with SseType=NONE during retrieval even though they were created with proper SseType during multipart completion. * s3: Fix SSE-C multipart IV base64 decoding bug **Critical Bug Found**: SSE-C multipart uploads were failing because: Root Cause: - entry.Extended[SeaweedFSSSEIV] stores base64-encoded IV (24 bytes for 16-byte IV) - SerializeSSECMetadata expects raw IV bytes (16 bytes) - During multipart completion, we were passing base64 IV directly → serialization error Error Message: "Failed to serialize SSE-C metadata for chunk in part X: invalid IV length: expected 16 bytes, got 24" Fix: - Base64-decode IV before passing to SerializeSSECMetadata - Added error handling for decode failures Impact: - SSE-C multipart uploads will now correctly serialize chunk metadata - Chunks will have proper SSE metadata for decryption during GET This fixes the SSE-C subtest of TestSSEMultipartUploadIntegration. SSE-KMS still has a separate issue (error code 23) being investigated. * fixes * kms sse * handle retry if not found in .versions folder and should read the normal object * quick check (no retries) to see if the .versions/ directory exists * skip retry if object is not found * explicit update to avoid sync delay * fix map update lock * Remove fmt.Printf debug statements * Fix SSE-KMS multipart base IV fallback to fail instead of regenerating * fmt * Fix ACL grants storage logic * header handling * nil handling * range read for sse content * test range requests for sse objects * fmt * unused code * upload in chunks * header case * fix url * bucket policy error vs bucket not found * jwt handling * fmt * jwt in request header * Optimize Case-Insensitive Prefix Check * dead code * Eliminated Unnecessary Stream Prefetch for Multipart SSE * range sse * sse * refactor * context * fmt * fix type * fix SSE-C IV Mismatch * Fix Headers Being Set After WriteHeader * fix url parsing * propergate sse headers * multipart sse-s3 * aws sig v4 authen * sse kms * set content range * better errors * Update s3api_object_handlers_copy.go * Update s3api_object_handlers.go * Update s3api_object_handlers.go * avoid magic number * clean up * Update s3api_bucket_policy_handlers.go * fix url parsing * context * data and metadata both use background context * adjust the offset * SSE Range Request IV Calculation * adjust logs * IV relative to offset in each part, not the whole file * collect logs * offset * fix offset * fix url * logs * variable * jwt * Multipart ETag semantics: conditionally set object-level Md5 for single-chunk uploads only. * sse * adjust IV and offset * multipart boundaries * ensures PUT and GET operations return consistent ETags * Metadata Header Case * CommonPrefixes Sorting with URL Encoding * always sort * remove the extra PathUnescape call * fix the multipart get part ETag * the FileChunk is created without setting ModifiedTsNs * Sort CommonPrefixes lexicographically to match AWS S3 behavior * set md5 for multipart uploads * prevents any potential data loss or corruption in the small-file inline storage path * compiles correctly * decryptedReader will now be properly closed after use * Fixed URL encoding and sort order for CommonPrefixes * Update s3api_object_handlers_list.go * SSE-x Chunk View Decryption * Different IV offset calculations for single-part vs multipart objects * still too verbose in logs * less logs * ensure correct conversion * fix listing * nil check * minor fixes * nil check * single character delimiter * optimize * range on empty object or zero-length * correct IV based on its position within that part, not its position in the entire object * adjust offset * offset Fetch FULL encrypted chunk (not just the range) Adjust IV by PartOffset/ChunkOffset only Decrypt full chunk Skip in the DECRYPTED stream to reach OffsetInChunk * look breaking * refactor * error on no content * handle intra-block byte skipping * Incomplete HTTP Response Error Handling * multipart SSE * Update s3api_object_handlers.go * address comments * less logs * handling directory * Optimized rejectDirectoryObjectWithoutSlash() to avoid unnecessary lookups * Revert "handling directory" This reverts commit 3a335f0ac3. * constant * Consolidate nil entry checks in GetObjectHandler * add range tests * Consolidate redundant nil entry checks in HeadObjectHandler * adjust logs * SSE type * large files * large files Reverted the plain-object range test * ErrNoEncryptionConfig * Fixed SSERangeReader Infinite Loop Vulnerability * Fixed SSE-KMS Multipart ChunkReader HTTP Body Leak * handle empty directory in S3, added PyArrow tests * purge unused code * Update s3_parquet_test.py * Update requirements.txt * According to S3 specifications, when both partNumber and Range are present, the Range should apply within the selected part's boundaries, not to the full object. * handle errors * errors after writing header * https * fix: Wait for volume assignment readiness before running Parquet tests The test-implicit-dir-with-server test was failing with an Internal Error because volume assignment was not ready when tests started. This fix adds a check that attempts a volume assignment and waits for it to succeed before proceeding with tests. This ensures that: 1. Volume servers are registered with the master 2. Volume growth is triggered if needed 3. The system can successfully assign volumes for writes Fixes the timeout issue where boto3 would retry 4 times and fail with 'We encountered an internal error, please try again.' * sse tests * store derived IV * fix: Clean up gRPC ports between tests to prevent port conflicts The second test (test-implicit-dir-with-server) was failing because the volume server's gRPC port (18080 = VOLUME_PORT + 10000) was still in use from the first test. The cleanup code only killed HTTP port processes, not gRPC port processes. Added cleanup for gRPC ports in all stop targets: - Master gRPC: MASTER_PORT + 10000 (19333) - Volume gRPC: VOLUME_PORT + 10000 (18080) - Filer gRPC: FILER_PORT + 10000 (18888) This ensures clean state between test runs in CI. * add import * address comments * docs: Add placeholder documentation files for Parquet test suite Added three missing documentation files referenced in test/s3/parquet/README.md: 1. TEST_COVERAGE.md - Documents 43 total test cases (17 Go unit tests, 6 Python integration tests, 20 Python end-to-end tests) 2. FINAL_ROOT_CAUSE_ANALYSIS.md - Explains the s3fs compatibility issue with PyArrow, the implicit directory problem, and how the fix works 3. MINIO_DIRECTORY_HANDLING.md - Compares MinIO's directory handling approach with SeaweedFS's implementation Each file contains: - Title and overview - Key technical details relevant to the topic - TODO sections for future expansion These placeholder files resolve the broken README links and provide structure for future detailed documentation. * clean up if metadata operation failed * Update s3_parquet_test.py * clean up * Update Makefile * Update s3_parquet_test.py * Update Makefile * Handle ivSkip for non-block-aligned offsets * Update README.md * stop volume server faster * stop volume server in 1 second * different IV for each chunk in SSE-S3 and SSE-KMS * clean up if fails * testing upload * error propagation * fmt * simplify * fix copying * less logs * endian * Added marshaling error handling * handling invalid ranges * error handling for adding to log buffer * fix logging * avoid returning too quickly and ensure proper cleaning up * Activity Tracking for Disk Reads * Cleanup Unused Parameters * Activity Tracking for Kafka Publishers * Proper Test Error Reporting * refactoring * less logs * less logs * go fmt * guard it with if entry.Attributes.TtlSec > 0 to match the pattern used elsewhere. * Handle bucket-default encryption config errors explicitly for multipart * consistent activity tracking * obsolete code for s3 on filer read/write handlers * Update weed/s3api/s3api_object_handlers_list.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	1 month ago