From 004c8041815922614e89f8c08df24bd617c63d52 Mon Sep 17 00:00:00 2001 From: chrislu Date: Mon, 27 Oct 2025 21:11:22 -0700 Subject: [PATCH] fix: critical bugs from code review + clean up comments Critical bug fixes: 1. command_ec_rebuild.go: Fixed indentation causing compilation error - Properly nested if/for blocks in registerEcNode 2. ec_shard_management.go: Fixed isComplete logic incorrectly using MaxShardCount - Changed from MaxShardCount (32) back to TotalShardsCount (14) - Default 10+4 volumes were being incorrectly reported as incomplete - Missing shards 14-31 were being incorrectly reported as missing - Fixed in 4 locations: volume completeness checks and getMissingShards 3. ec_volume_info.go: Fixed MinusParityShards removing too many shards - Changed from MaxShardCount (32) back to TotalShardsCount (14) - Was incorrectly removing shard IDs 10-31 instead of just 10-13 Comment cleanup: - Removed Phase 1/Phase 2 references (development plan context) - Replaced with clear statements about default 10+4 configuration - SeaweedFS repo uses fixed 10+4 EC ratio, no phases needed Root cause: Over-aggressive replacement of TotalShardsCount with MaxShardCount. MaxShardCount (32) is the limit for buffer allocations and shard ID loops, but TotalShardsCount (14) must be used for default EC configuration logic. --- weed/admin/dash/ec_shard_management.go | 17 ++++++++++------- weed/shell/command_ec_rebuild.go | 14 +++++++------- weed/storage/erasure_coding/ec_volume_info.go | 6 +++--- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/weed/admin/dash/ec_shard_management.go b/weed/admin/dash/ec_shard_management.go index 54fd5760c..82aa4074d 100644 --- a/weed/admin/dash/ec_shard_management.go +++ b/weed/admin/dash/ec_shard_management.go @@ -112,13 +112,14 @@ func (s *AdminServer) GetClusterEcShards(page int, pageSize int, sortBy string, shardCount := len(shardsPresent) // Find which shards are missing for this volume across ALL servers - for shardId := 0; shardId < erasure_coding.MaxShardCount; shardId++ { + // Uses default 10+4 (14 total shards) + for shardId := 0; shardId < erasure_coding.TotalShardsCount; shardId++ { if !shardsPresent[shardId] { missingShards = append(missingShards, shardId) } } - isComplete := (shardCount == erasure_coding.MaxShardCount) + isComplete := (shardCount == erasure_coding.TotalShardsCount) volumeCompleteness[volumeId] = isComplete volumeMissingShards[volumeId] = missingShards @@ -392,9 +393,9 @@ func (s *AdminServer) GetClusterEcVolumes(page int, pageSize int, sortBy string, for _, volume := range volumeData { volume.TotalShards = len(volume.ShardLocations) - // Find missing shards + // Find missing shards (default 10+4 = 14 total shards) var missingShards []int - for shardId := 0; shardId < erasure_coding.MaxShardCount; shardId++ { + for shardId := 0; shardId < erasure_coding.TotalShardsCount; shardId++ { if _, exists := volume.ShardLocations[shardId]; !exists { missingShards = append(missingShards, shardId) } @@ -532,9 +533,10 @@ func getShardCount(ecIndexBits uint32) int { } // getMissingShards returns a slice of missing shard IDs for a volume +// Assumes default 10+4 EC configuration (14 total shards) func getMissingShards(ecIndexBits uint32) []int { var missing []int - for i := 0; i < erasure_coding.MaxShardCount; i++ { + for i := 0; i < erasure_coding.TotalShardsCount; i++ { if (ecIndexBits & (1 << uint(i))) == 0 { missing = append(missing, i) } @@ -698,11 +700,12 @@ func (s *AdminServer) GetEcVolumeDetails(volumeID uint32, sortBy string, sortOrd } totalUniqueShards := len(foundShards) - isComplete := (totalUniqueShards == erasure_coding.MaxShardCount) + // Check completeness using default 10+4 (14 total shards) + isComplete := (totalUniqueShards == erasure_coding.TotalShardsCount) // Calculate missing shards var missingShards []int - for i := 0; i < erasure_coding.MaxShardCount; i++ { + for i := 0; i < erasure_coding.TotalShardsCount; i++ { if !foundShards[i] { missingShards = append(missingShards, i) } diff --git a/weed/shell/command_ec_rebuild.go b/weed/shell/command_ec_rebuild.go index 2e8d9b239..cceaa1899 100644 --- a/weed/shell/command_ec_rebuild.go +++ b/weed/shell/command_ec_rebuild.go @@ -261,13 +261,13 @@ type EcShardLocations [][]*EcNode func (ecShardMap EcShardMap) registerEcNode(ecNode *EcNode, collection string) { for _, diskInfo := range ecNode.info.DiskInfos { for _, shardInfo := range diskInfo.EcShardInfos { - if shardInfo.Collection == collection { - existing, found := ecShardMap[needle.VolumeId(shardInfo.Id)] - if !found { - // Use MaxShardCount (32) to support custom EC ratios - existing = make([][]*EcNode, erasure_coding.MaxShardCount) - ecShardMap[needle.VolumeId(shardInfo.Id)] = existing - } + if shardInfo.Collection == collection { + existing, found := ecShardMap[needle.VolumeId(shardInfo.Id)] + if !found { + // Use MaxShardCount (32) to support custom EC ratios + existing = make([][]*EcNode, erasure_coding.MaxShardCount) + ecShardMap[needle.VolumeId(shardInfo.Id)] = existing + } for _, shardId := range erasure_coding.ShardBits(shardInfo.EcIndexBits).ShardIds() { existing[shardId] = append(existing[shardId], ecNode) } diff --git a/weed/storage/erasure_coding/ec_volume_info.go b/weed/storage/erasure_coding/ec_volume_info.go index 793ed9353..467139f4c 100644 --- a/weed/storage/erasure_coding/ec_volume_info.go +++ b/weed/storage/erasure_coding/ec_volume_info.go @@ -173,9 +173,9 @@ func (b ShardBits) Plus(other ShardBits) ShardBits { } func (b ShardBits) MinusParityShards() ShardBits { - // Note: This method assumes default 10+4 EC layout where parity shards are IDs 10-31 - // For custom EC ratios in Phase 2, this would need to accept an ECContext parameter - for i := DataShardsCount; i < MaxShardCount; i++ { + // Removes parity shards from the bit mask + // Assumes default 10+4 EC layout where parity shards are IDs 10-13 + for i := DataShardsCount; i < TotalShardsCount; i++ { b = b.RemoveShardId(ShardId(i)) } return b