* opt: reduce ShardsInfo memory usage with bitmap and sorted slice - Replace map[ShardId]*ShardInfo with sorted []ShardInfo slice - Add ShardBits (uint32) bitmap for O(1) existence checks - Use binary search for O(log n) lookups by shard ID - Maintain sorted order for efficient iteration - Add comprehensive unit tests and benchmarks Memory savings: - Map overhead: ~48 bytes per entry eliminated - Pointers: 8 bytes per entry eliminated - Total: ~56 bytes per shard saved Performance improvements: - Has(): O(1) using bitmap - Size(): O(log n) using binary search (was O(1), acceptable tradeoff) - Count(): O(1) using popcount on bitmap - Iteration: Faster due to cache locality * refactor: add methods to ShardBits type - Add Has(), Set(), Clear(), and Count() methods to ShardBits - Simplify ShardsInfo methods by using ShardBits methods - Improves code readability and encapsulation * opt: use ShardBits directly in ShardsCountFromVolumeEcShardInformationMessage Avoid creating a full ShardsInfo object just to count shards. Directly cast vi.EcIndexBits to ShardBits and use Count() method. * opt: use strings.Builder in ShardsInfo.String() for efficiency * refactor: change AsSlice to return []ShardInfo (values instead of pointers) This completes the memory optimization by avoiding unnecessary pointer slices and potential allocations. * refactor: rename ShardsCountFromVolumeEcShardInformationMessage to GetShardCount * fix: prevent deadlock in Add and Subtract methods Copy shards data from 'other' before releasing its lock to avoid potential deadlock when a.Add(b) and b.Add(a) are called concurrently. The previous implementation held other's lock while calling si.Set/Delete, which acquires si's lock. This could deadlock if two goroutines tried to add/subtract each other concurrently. * opt: avoid unnecessary locking in constructor functions ShardsInfoFromVolume and ShardsInfoFromVolumeEcShardInformationMessage now build shards slice and bitmap directly without calling Set(), which acquires a lock on every call. Since the object is local and not yet shared, locking is unnecessary and adds overhead. This improves performance during object construction. * fix: rename 'copy' variable to avoid shadowing built-in function The variable name 'copy' in TestShardsInfo_Copy shadowed the built-in copy() function, which is confusing and bad practice. Renamed to 'siCopy'. * opt: use math/bits.OnesCount32 and reorganize types 1. Replace manual popcount loop with math/bits.OnesCount32 for better performance and idiomatic Go code 2. Move ShardSize type definition to ec_shards_info.go for better code organization since it's primarily used there * refactor: Set() now accepts ShardInfo for future extensibility Changed Set(id ShardId, size ShardSize) to Set(shard ShardInfo) to support future additions to ShardInfo without changing the API. This makes the code more extensible as new fields can be added to ShardInfo (e.g., checksum, location, etc.) without breaking the Set API. * refactor: move ShardInfo and ShardSize to separate file Created ec_shard_info.go to hold the basic shard types (ShardInfo and ShardSize) for better code organization and separation of concerns. * refactor: add ShardInfo constructor and helper functions Added NewShardInfo() constructor and IsValid() method to better encapsulate ShardInfo creation and validation. Updated code to use the constructor for cleaner, more maintainable code. * fix: update remaining Set() calls to use NewShardInfo constructor Fixed compilation errors in storage and shell packages where Set() calls were not updated to use the new NewShardInfo() constructor. * fix: remove unreachable code in filer backup commands Removed unreachable return statements after infinite loops in filer_backup.go and filer_meta_backup.go to fix compilation errors. * fix: rename 'new' variable to avoid shadowing built-in Renamed 'new' to 'result' in MinusParityShards, Plus, and Minus methods to avoid shadowing Go's built-in new() function. * fix: update remaining test files to use NewShardInfo constructor Fixed Set() calls in command_volume_list_test.go and ec_rebalance_slots_test.go to use NewShardInfo() constructor.
295 lines
9.9 KiB
Go
295 lines
9.9 KiB
Go
package shell
|
||
|
||
import (
|
||
"testing"
|
||
|
||
"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
|
||
"github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding"
|
||
"github.com/seaweedfs/seaweedfs/weed/storage/types"
|
||
)
|
||
|
||
// TestECRebalanceWithLimitedSlots tests that EC rebalance handles the scenario
|
||
// where there are limited free slots on volume servers.
|
||
//
|
||
// This is a regression test for the error:
|
||
//
|
||
// "no free ec shard slots. only 0 left"
|
||
//
|
||
// Scenario (from real usage):
|
||
// - 6 volume servers in 6 racks
|
||
// - Each server has max=10 volume slots
|
||
// - 7 volumes were EC encoded (7 × 14 = 98 EC shards)
|
||
// - All 14 shards per volume are on the original server (not yet distributed)
|
||
//
|
||
// Expected behavior:
|
||
// - The rebalance algorithm should distribute shards across servers
|
||
// - Even if perfect distribution isn't possible, it should do best-effort
|
||
// - Currently fails with "no free ec shard slots" because freeSlots calculation
|
||
//
|
||
// doesn't account for shards being moved (freed slots on source, used on target)
|
||
func TestECRebalanceWithLimitedSlots(t *testing.T) {
|
||
// Build a topology matching the problematic scenario:
|
||
// 6 servers, each with 2+ volumes worth of EC shards (all 14 shards per volume on same server)
|
||
topology := buildLimitedSlotsTopology()
|
||
|
||
// Collect EC nodes from the topology
|
||
ecNodes, totalFreeEcSlots := collectEcVolumeServersByDc(topology, "", types.HardDriveType)
|
||
|
||
t.Logf("Topology summary:")
|
||
t.Logf(" Number of EC nodes: %d", len(ecNodes))
|
||
t.Logf(" Total free EC slots: %d", totalFreeEcSlots)
|
||
|
||
// Log per-node details
|
||
for _, node := range ecNodes {
|
||
shardCount := 0
|
||
for _, diskInfo := range node.info.DiskInfos {
|
||
for _, ecShard := range diskInfo.EcShardInfos {
|
||
shardCount += erasure_coding.GetShardCount(ecShard)
|
||
}
|
||
}
|
||
t.Logf(" Node %s (rack %s): %d shards, %d free slots",
|
||
node.info.Id, node.rack, shardCount, node.freeEcSlot)
|
||
}
|
||
|
||
// Calculate total EC shards
|
||
totalEcShards := 0
|
||
for _, node := range ecNodes {
|
||
for _, diskInfo := range node.info.DiskInfos {
|
||
for _, ecShard := range diskInfo.EcShardInfos {
|
||
totalEcShards += erasure_coding.GetShardCount(ecShard)
|
||
}
|
||
}
|
||
}
|
||
t.Logf(" Total EC shards: %d", totalEcShards)
|
||
|
||
// Document the issue:
|
||
// With 98 EC shards (7 volumes × 14 shards) on 6 servers with max=10 each,
|
||
// total capacity is 60 slots. But shards already occupy slots on their current servers.
|
||
//
|
||
// The current algorithm calculates free slots as:
|
||
// freeSlots = maxVolumeCount - volumeCount - ecShardCount
|
||
//
|
||
// If all shards are on their original servers:
|
||
// - Server A has 28 shards (2 volumes × 14) → may have negative free slots
|
||
// - This causes totalFreeEcSlots to be 0 or negative
|
||
//
|
||
// The EXPECTED improvement:
|
||
// - Rebalance should recognize that moving a shard FREES a slot on the source
|
||
// - The algorithm should work iteratively, moving shards one at a time
|
||
// - Even if starting with 0 free slots, moving one shard opens a slot
|
||
|
||
if totalFreeEcSlots < 1 {
|
||
// This is the current (buggy) behavior we're documenting
|
||
t.Logf("")
|
||
t.Logf("KNOWN ISSUE: totalFreeEcSlots = %d (< 1)", totalFreeEcSlots)
|
||
t.Logf("")
|
||
t.Logf("This triggers the error: 'no free ec shard slots. only %d left'", totalFreeEcSlots)
|
||
t.Logf("")
|
||
t.Logf("Analysis:")
|
||
t.Logf(" - %d EC shards across %d servers", totalEcShards, len(ecNodes))
|
||
t.Logf(" - Shards are concentrated on original servers (not distributed)")
|
||
t.Logf(" - Current slot calculation doesn't account for slots freed by moving shards")
|
||
t.Logf("")
|
||
t.Logf("Expected fix:")
|
||
t.Logf(" 1. Rebalance should work iteratively, moving one shard at a time")
|
||
t.Logf(" 2. Moving a shard from A to B: frees 1 slot on A, uses 1 slot on B")
|
||
t.Logf(" 3. The 'free slots' check should be per-move, not global")
|
||
t.Logf(" 4. Or: calculate 'redistributable slots' = total capacity - shards that must stay")
|
||
|
||
// For now, document this is a known issue - don't fail the test
|
||
// When the fix is implemented, this test should be updated to verify the fix works
|
||
return
|
||
}
|
||
|
||
// If we get here, the issue might have been fixed
|
||
t.Logf("totalFreeEcSlots = %d, rebalance should be possible", totalFreeEcSlots)
|
||
}
|
||
|
||
// TestECRebalanceZeroFreeSlots tests the specific scenario where
|
||
// the topology appears to have free slots but rebalance fails.
|
||
//
|
||
// This can happen when the VolumeCount in the topology includes the original
|
||
// volumes that were EC-encoded, making the free slot calculation incorrect.
|
||
func TestECRebalanceZeroFreeSlots(t *testing.T) {
|
||
// Build a topology where volumes were NOT deleted after EC encoding
|
||
// (VolumeCount still reflects the original volumes)
|
||
topology := buildZeroFreeSlotTopology()
|
||
|
||
ecNodes, totalFreeEcSlots := collectEcVolumeServersByDc(topology, "", types.HardDriveType)
|
||
|
||
t.Logf("Zero free slots scenario:")
|
||
for _, node := range ecNodes {
|
||
shardCount := 0
|
||
for _, diskInfo := range node.info.DiskInfos {
|
||
for _, ecShard := range diskInfo.EcShardInfos {
|
||
shardCount += erasure_coding.GetShardCount(ecShard)
|
||
}
|
||
}
|
||
t.Logf(" Node %s: %d shards, %d free slots, volumeCount=%d, max=%d",
|
||
node.info.Id, shardCount, node.freeEcSlot,
|
||
node.info.DiskInfos[string(types.HardDriveType)].VolumeCount,
|
||
node.info.DiskInfos[string(types.HardDriveType)].MaxVolumeCount)
|
||
}
|
||
t.Logf(" Total free slots: %d", totalFreeEcSlots)
|
||
|
||
if totalFreeEcSlots == 0 {
|
||
t.Logf("")
|
||
t.Logf("SCENARIO REPRODUCED: totalFreeEcSlots = 0")
|
||
t.Logf("This would trigger: 'no free ec shard slots. only 0 left'")
|
||
}
|
||
}
|
||
|
||
// buildZeroFreeSlotTopology creates a topology where rebalance will fail
|
||
// because servers are at capacity (volumeCount equals maxVolumeCount)
|
||
func buildZeroFreeSlotTopology() *master_pb.TopologyInfo {
|
||
diskTypeKey := string(types.HardDriveType)
|
||
|
||
// Each server has max=10, volumeCount=10 (full capacity)
|
||
// Free capacity = (10-10) * 10 = 0 per server
|
||
// This will trigger "no free ec shard slots" error
|
||
return &master_pb.TopologyInfo{
|
||
Id: "test_zero_free_slots",
|
||
DataCenterInfos: []*master_pb.DataCenterInfo{
|
||
{
|
||
Id: "dc1",
|
||
RackInfos: []*master_pb.RackInfo{
|
||
{
|
||
Id: "rack0",
|
||
DataNodeInfos: []*master_pb.DataNodeInfo{
|
||
{
|
||
Id: "127.0.0.1:8080",
|
||
DiskInfos: map[string]*master_pb.DiskInfo{
|
||
diskTypeKey: {
|
||
Type: diskTypeKey,
|
||
MaxVolumeCount: 10,
|
||
VolumeCount: 10, // At full capacity
|
||
EcShardInfos: buildEcShards([]uint32{3, 4}),
|
||
},
|
||
},
|
||
},
|
||
},
|
||
},
|
||
{
|
||
Id: "rack1",
|
||
DataNodeInfos: []*master_pb.DataNodeInfo{
|
||
{
|
||
Id: "127.0.0.1:8081",
|
||
DiskInfos: map[string]*master_pb.DiskInfo{
|
||
diskTypeKey: {
|
||
Type: diskTypeKey,
|
||
MaxVolumeCount: 10,
|
||
VolumeCount: 10,
|
||
EcShardInfos: buildEcShards([]uint32{1, 7}),
|
||
},
|
||
},
|
||
},
|
||
},
|
||
},
|
||
{
|
||
Id: "rack2",
|
||
DataNodeInfos: []*master_pb.DataNodeInfo{
|
||
{
|
||
Id: "127.0.0.1:8082",
|
||
DiskInfos: map[string]*master_pb.DiskInfo{
|
||
diskTypeKey: {
|
||
Type: diskTypeKey,
|
||
MaxVolumeCount: 10,
|
||
VolumeCount: 10,
|
||
EcShardInfos: buildEcShards([]uint32{2}),
|
||
},
|
||
},
|
||
},
|
||
},
|
||
},
|
||
{
|
||
Id: "rack3",
|
||
DataNodeInfos: []*master_pb.DataNodeInfo{
|
||
{
|
||
Id: "127.0.0.1:8083",
|
||
DiskInfos: map[string]*master_pb.DiskInfo{
|
||
diskTypeKey: {
|
||
Type: diskTypeKey,
|
||
MaxVolumeCount: 10,
|
||
VolumeCount: 10,
|
||
EcShardInfos: buildEcShards([]uint32{5, 6}),
|
||
},
|
||
},
|
||
},
|
||
},
|
||
},
|
||
},
|
||
},
|
||
},
|
||
}
|
||
}
|
||
|
||
func buildEcShards(volumeIds []uint32) []*master_pb.VolumeEcShardInformationMessage {
|
||
var shards []*master_pb.VolumeEcShardInformationMessage
|
||
for _, vid := range volumeIds {
|
||
si := erasure_coding.NewShardsInfo()
|
||
for _, id := range erasure_coding.AllShardIds() {
|
||
si.Set(erasure_coding.NewShardInfo(id, 1234))
|
||
}
|
||
shards = append(shards, &master_pb.VolumeEcShardInformationMessage{
|
||
Id: vid,
|
||
Collection: "ectest",
|
||
EcIndexBits: si.Bitmap(),
|
||
ShardSizes: si.SizesInt64(),
|
||
})
|
||
}
|
||
return shards
|
||
}
|
||
|
||
// buildLimitedSlotsTopology creates a topology matching the problematic scenario:
|
||
// - 6 servers in 6 racks
|
||
// - Each server has max=10 volume slots
|
||
// - 7 volumes were EC encoded, shards distributed as follows:
|
||
// - rack0 (8080): volumes 3,4 → 28 shards
|
||
// - rack1 (8081): volumes 1,7 → 28 shards
|
||
// - rack2 (8082): volume 2 → 14 shards
|
||
// - rack3 (8083): volumes 5,6 → 28 shards
|
||
// - rack4 (8084): (no volumes originally)
|
||
// - rack5 (8085): (no volumes originally)
|
||
func buildLimitedSlotsTopology() *master_pb.TopologyInfo {
|
||
return &master_pb.TopologyInfo{
|
||
Id: "test_limited_slots",
|
||
DataCenterInfos: []*master_pb.DataCenterInfo{
|
||
{
|
||
Id: "dc1",
|
||
RackInfos: []*master_pb.RackInfo{
|
||
buildRackWithEcShards("rack0", "127.0.0.1:8080", 10, []uint32{3, 4}),
|
||
buildRackWithEcShards("rack1", "127.0.0.1:8081", 10, []uint32{1, 7}),
|
||
buildRackWithEcShards("rack2", "127.0.0.1:8082", 10, []uint32{2}),
|
||
buildRackWithEcShards("rack3", "127.0.0.1:8083", 10, []uint32{5, 6}),
|
||
buildRackWithEcShards("rack4", "127.0.0.1:8084", 10, []uint32{}),
|
||
buildRackWithEcShards("rack5", "127.0.0.1:8085", 10, []uint32{}),
|
||
},
|
||
},
|
||
},
|
||
}
|
||
}
|
||
|
||
// buildRackWithEcShards creates a rack with one data node containing EC shards
|
||
// for the specified volume IDs (all 14 shards per volume)
|
||
func buildRackWithEcShards(rackId, nodeId string, maxVolumes int64, volumeIds []uint32) *master_pb.RackInfo {
|
||
// Note: types.HardDriveType is "" (empty string), so we use "" as the key
|
||
diskTypeKey := string(types.HardDriveType)
|
||
|
||
return &master_pb.RackInfo{
|
||
Id: rackId,
|
||
DataNodeInfos: []*master_pb.DataNodeInfo{
|
||
{
|
||
Id: nodeId,
|
||
DiskInfos: map[string]*master_pb.DiskInfo{
|
||
diskTypeKey: {
|
||
Type: diskTypeKey,
|
||
MaxVolumeCount: maxVolumes,
|
||
VolumeCount: int64(len(volumeIds)), // Original volumes still counted
|
||
EcShardInfos: buildEcShards(volumeIds),
|
||
},
|
||
},
|
||
},
|
||
},
|
||
}
|
||
}
|