Merge pull request #656 from cheney-lin/fix/pool_size

fix(sysadvisor): fix isolacted size calculated incorrectly
kubewharf · Jul 26, 2024 · df45505 · df45505
2 parents d5ebb56 + 0807cf1
commit df45505
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 4 deletions.
diff --git a/.../sysadvisor/plugin/qosaware/resource/cpu/assembler/provisionassembler/assembler_common.go b/.../sysadvisor/plugin/qosaware/resource/cpu/assembler/provisionassembler/assembler_common.go
@@ -171,6 +171,24 @@ func (pa *ProvisionAssemblerCommon) AssembleProvision() (types.InternalCPUCalcul
 				// If there is a SNB pool with the same NUMA ID, it will be calculated while processing the SNB pool.
 				if shareRegions := pa.regionHelper.GetRegions(regionNuma, configapi.QoSRegionTypeShare); len(shareRegions) == 0 {
 					calculationResult.SetPoolEntry(r.Name(), regionNuma, int(controlKnob[configapi.ControlKnobNonReclaimedCPURequirementUpper].Value))
+
+					_, ok := calculationResult.GetPoolEntry(state.PoolNameReclaim, regionNuma)
+					if !ok {
+						available := getNUMAsResource(*pa.numaAvailable, r.GetBindingNumas())
+						reservedForReclaim := getNUMAsResource(*pa.reservedForReclaim, r.GetBindingNumas())
+
+						isolationRegions := pa.regionHelper.GetRegions(regionNuma, configapi.QoSRegionTypeIsolation)
+						isolationSizes := 0
+						for _, ir := range isolationRegions {
+							ck, err := ir.GetProvision()
+							if err != nil {
+								return types.InternalCPUCalculationResult{}, err
+							}
+							isolationSizes += int(ck[configapi.ControlKnobNonReclaimedCPURequirementUpper].Value)
+						}
+						reclaimedCoresSize := general.Max(available-isolationSizes, 0) + reservedForReclaim
+						calculationResult.SetPoolEntry(state.PoolNameReclaim, regionNuma, reclaimedCoresSize)
+					}
 				}
 			} else {
 				// save limits and requests for isolated region
@@ -242,12 +260,17 @@ func (pa *ProvisionAssemblerCommon) AssembleProvision() (types.InternalCPUCalcul
 
 	var reclaimPoolSizeOfNonBindingNUMAs int
 	if *pa.allowSharedCoresOverlapReclaimedCores {
-		isolated := shareAndIsolatedPoolAvailable
+		isolated := 0
 		sharePoolSizes := make(map[string]int)
-		for poolName := range sharePoolRequirements {
-			sharePoolSizes[poolName] = shareAndIsolatePoolSizes[poolName]
-			isolated -= shareAndIsolatePoolSizes[poolName]
+		for poolName, size := range shareAndIsolatePoolSizes {
+			_, ok := sharePoolRequirements[poolName]
+			if ok {
+				sharePoolSizes[poolName] = shareAndIsolatePoolSizes[poolName]
+			} else {
+				isolated += size
+			}
 		}
+
 		reclaimPoolSizeOfNonBindingNUMAs = general.Max(pa.getNumasReservedForReclaim(*pa.nonBindingNumas), shareAndIsolatedPoolAvailable-isolated-general.SumUpMapValues(sharePoolRequirements))
 		if !nodeEnableReclaim {
 			reclaimPoolSizeOfNonBindingNUMAs = pa.getNumasReservedForReclaim(*pa.nonBindingNumas)

diff --git a/...dvisor/plugin/qosaware/resource/cpu/assembler/provisionassembler/assembler_common_test.go b/...dvisor/plugin/qosaware/resource/cpu/assembler/provisionassembler/assembler_common_test.go
@@ -810,6 +810,20 @@ func TestAssembleProvision(t *testing.T) {
 				"reclaim": {-1: map[string]int{"share": 18}, 1: map[string]int{"share-NUMA1": 4}},
 			},
 		},
+		{
+			name:                                  "no share pool and isolated pool, allow shared_cores overlap reclaimed_cores",
+			enableReclaimed:                       true,
+			allowSharedCoresOverlapReclaimedCores: true,
+			poolInfos:                             []testCasePoolConfig{},
+			expectPoolEntries: map[string]map[int]int{
+				"reserve": {
+					-1: 0,
+				},
+				"reclaim": {
+					-1: 48,
+				},
+			},
+		},
 		{
 			name:                                  "share and isolated pool not throttled, overlap reclaimed cores, reclaim disabled",
 			enableReclaimed:                       false,
@@ -879,6 +893,48 @@ func TestAssembleProvision(t *testing.T) {
 				"reclaim": {-1: map[string]int{"share": 4}, 1: map[string]int{"share-NUMA1": 4}},
 			},
 		},
+		{
+			name:                                  "isolated pools only, with numa binding",
+			enableReclaimed:                       true,
+			allowSharedCoresOverlapReclaimedCores: true,
+			poolInfos: []testCasePoolConfig{
+				{
+					poolName:      "isolation-NUMA1",
+					poolType:      configapi.QoSRegionTypeIsolation,
+					numa:          machine.NewCPUSet(1),
+					isNumaBinding: true,
+					provision: types.ControlKnob{
+						configapi.ControlKnobNonReclaimedCPURequirementUpper: {Value: 8},
+						configapi.ControlKnobNonReclaimedCPURequirementLower: {Value: 4},
+					},
+				},
+				{
+					poolName:      "isolation-NUMA1-pod2",
+					poolType:      configapi.QoSRegionTypeIsolation,
+					numa:          machine.NewCPUSet(1),
+					isNumaBinding: true,
+					provision: types.ControlKnob{
+						configapi.ControlKnobNonReclaimedCPURequirementUpper: {Value: 8},
+						configapi.ControlKnobNonReclaimedCPURequirementLower: {Value: 4},
+					},
+				},
+			},
+			expectPoolEntries: map[string]map[int]int{
+				"isolation-NUMA1": {
+					1: 8,
+				},
+				"isolation-NUMA1-pod2": {
+					1: 8,
+				},
+				"reserve": {
+					-1: 0,
+				},
+				"reclaim": {
+					1:  8,
+					-1: 24,
+				},
+			},
+		},
 		{
 			name:                                  "share and bach pool non binding NUMAs, overlap reclaimed cores",
 			enableReclaimed:                       true,