From b078f86d69c53e9d6847c5941a43fe2f72730234 Mon Sep 17 00:00:00 2001
From: Peter Szucs <116345192+p-szucs@users.noreply.github.com>
Date: Tue, 24 Sep 2024 17:13:28 +0200
Subject: [PATCH] YARN-11733. Fix the order of updating CPU controls with
 cgroup v1 (#7069)

---
 .../resources/CGroupsCpuResourceHandlerImpl.java  |  8 ++++----
 .../TestCGroupsCpuResourceHandlerImpl.java        | 15 +++++++++------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsCpuResourceHandlerImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsCpuResourceHandlerImpl.java
index 54686fddb2..04b084852b 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsCpuResourceHandlerImpl.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsCpuResourceHandlerImpl.java
@@ -59,14 +59,14 @@ public class CGroupsCpuResourceHandlerImpl extends AbstractCGroupsCpuResourceHan
 
   @Override
   protected void updateCgroupMaxCpuLimit(String cgroupId, String quota, String period) throws ResourceHandlerException {
-    if (quota != null) {
-      cGroupsHandler
-          .updateCGroupParam(CPU, cgroupId, CGroupsHandler.CGROUP_CPU_QUOTA_US, quota);
-    }
     if (period != null) {
       cGroupsHandler
           .updateCGroupParam(CPU, cgroupId, CGroupsHandler.CGROUP_CPU_PERIOD_US, period);
     }
+    if (quota != null) {
+      cGroupsHandler
+          .updateCGroupParam(CPU, cgroupId, CGroupsHandler.CGROUP_CPU_QUOTA_US, quota);
+    }
   }
 
   @Override
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsCpuResourceHandlerImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsCpuResourceHandlerImpl.java
index 842fc6b437..9fbf9f03d2 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsCpuResourceHandlerImpl.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsCpuResourceHandlerImpl.java
@@ -31,6 +31,7 @@
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.InOrder;
 import org.mockito.Mockito;
 
 import java.io.File;
@@ -190,11 +191,12 @@ public void testPreStartStrictUsage() throws Exception {
             CGroupsHandler.CGROUP_CPU_SHARES,
             String.valueOf(CGroupsCpuResourceHandlerImpl.CPU_DEFAULT_WEIGHT));
     // set quota and period
-    verify(mockCGroupsHandler, times(1))
+    InOrder cpuLimitOrder = inOrder(mockCGroupsHandler);
+    cpuLimitOrder.verify(mockCGroupsHandler, times(1))
         .updateCGroupParam(CGroupsHandler.CGroupController.CPU, id,
             CGroupsHandler.CGROUP_CPU_PERIOD_US,
             String.valueOf(CGroupsCpuResourceHandlerImpl.MAX_QUOTA_US));
-    verify(mockCGroupsHandler, times(1))
+    cpuLimitOrder.verify(mockCGroupsHandler, times(1))
         .updateCGroupParam(CGroupsHandler.CGroupController.CPU, id,
             CGroupsHandler.CGROUP_CPU_QUOTA_US, String.valueOf(
                 (int) (CGroupsCpuResourceHandlerImpl.MAX_QUOTA_US * share)));
@@ -222,10 +224,11 @@ public void testPreStartRestrictedContainers() throws Exception {
     conf.setInt(YarnConfiguration.NM_RESOURCE_PERCENTAGE_PHYSICAL_CPU_LIMIT,
         cpuPerc);
     cGroupsCpuResourceHandler.bootstrap(plugin, conf);
-    verify(mockCGroupsHandler, times(1))
+    InOrder cpuLimitOrder = inOrder(mockCGroupsHandler);
+    cpuLimitOrder.verify(mockCGroupsHandler, times(1))
         .updateCGroupParam(CGroupsHandler.CGroupController.CPU, "",
             CGroupsHandler.CGROUP_CPU_PERIOD_US, String.valueOf("333333"));
-    verify(mockCGroupsHandler, times(1))
+    cpuLimitOrder.verify(mockCGroupsHandler, times(1))
         .updateCGroupParam(CGroupsHandler.CGroupController.CPU, "",
             CGroupsHandler.CGROUP_CPU_QUOTA_US,
             String.valueOf(CGroupsCpuResourceHandlerImpl.MAX_QUOTA_US));
@@ -262,10 +265,10 @@ public void testPreStartRestrictedContainers() throws Exception {
               CGroupsHandler.CGROUP_CPU_SHARES, String.valueOf(
               CGroupsCpuResourceHandlerImpl.CPU_DEFAULT_WEIGHT * cVcores));
       // set quota and period
-      verify(mockCGroupsHandler, times(1))
+      cpuLimitOrder.verify(mockCGroupsHandler, times(1))
           .updateCGroupParam(CGroupsHandler.CGroupController.CPU, id,
               CGroupsHandler.CGROUP_CPU_PERIOD_US, String.valueOf(periodUS));
-      verify(mockCGroupsHandler, times(1))
+      cpuLimitOrder.verify(mockCGroupsHandler, times(1))
           .updateCGroupParam(CGroupsHandler.CGroupController.CPU, id,
               CGroupsHandler.CGROUP_CPU_QUOTA_US, String.valueOf(quotaUS));
     }