YARN-11967. Strict memory enforcement should not skip polling-based memory check when CGroups memory is disabled (branch-3.2)

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml

@@ -1513,6 +1513,10 @@
 
   <property>
     <description>Whether YARN CGroups strict memory enforcement is enabled.
+    This only takes effect when the CGroups memory controller is enabled via
+    yarn.nodemanager.resource.memory.enabled. When CGroups memory is disabled
+    (the default), this flag has no effect and container memory limits are
+    enforced (if at all) by the polling-based ContainersMonitor.
     </description>
     <name>yarn.nodemanager.resource.memory.enforced</name>
     <value>true</value>

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java

@@ -205,7 +205,18 @@ protected void serviceInit(Configuration myConf) throws Exception {
     elasticMemoryEnforcement = this.conf.getBoolean(
         YarnConfiguration.NM_ELASTIC_MEMORY_CONTROL_ENABLED,
         YarnConfiguration.DEFAULT_NM_ELASTIC_MEMORY_CONTROL_ENABLED);
+    // CGroup-based strict memory enforcement is only in effect when the
+    // CGroups memory controller is actually enabled via
+    // yarn.nodemanager.resource.memory.enabled. The "enforced" flag alone
+    // (which defaults to true) does not write any CGroups memory hard limit
+    // unless the memory controller is enabled. Gating strictMemoryEnforcement
+    // on both flags prevents the polling-based memory check from being skipped
+    // (see checkLimit) when no CGroups memory limit is actually applied, which
+    // would otherwise leave containers unbounded.
     strictMemoryEnforcement = conf.getBoolean(
+        YarnConfiguration.NM_MEMORY_RESOURCE_ENABLED,
+        YarnConfiguration.DEFAULT_NM_MEMORY_RESOURCE_ENABLED)
+        && conf.getBoolean(
         YarnConfiguration.NM_MEMORY_RESOURCE_ENFORCED,
         YarnConfiguration.DEFAULT_NM_MEMORY_RESOURCE_ENFORCED);
     LOG.info("Physical memory check enabled: " + pmemCheckEnabled);
@@ -1052,6 +1063,20 @@ public boolean isVmemCheckEnabled() {
     return this.vmemCheckEnabled;
   }
 
+  /**
+   * Is CGroup-based strict memory enforcement in effect? This is true only
+   * when both {@code yarn.nodemanager.resource.memory.enabled} and
+   * {@code yarn.nodemanager.resource.memory.enforced} are true. When it is
+   * true, the CGroups memory controller enforces the limit and the
+   * polling-based memory check is skipped.
+   *
+   * @return true if CGroup-based strict memory enforcement is in effect.
+   */
+  @VisibleForTesting
+  boolean isStrictMemoryEnforcementEnabled() {
+    return this.strictMemoryEnforcement;
+  }
+
   @Override
   public ResourceUtilization getContainersUtilization() {
     return this.containersUtilization;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainersMonitor.java

@@ -566,6 +566,52 @@ public void testContainerMonitorMemFlags() {
     assertEquals(true, cm.isVmemCheckEnabled());
   }
 
+  /**
+   * Verifies that CGroup-based strict memory enforcement (which skips the
+   * polling-based memory check) is only considered active when the CGroups
+   * memory controller is actually enabled via
+   * {@code yarn.nodemanager.resource.memory.enabled}. Historically,
+   * {@code yarn.nodemanager.resource.memory.enforced} alone (defaulting to
+   * true) was enough to skip the polling check, which left containers
+   * unbounded when CGroups memory was not enabled.
+   */
+  @Test(timeout = 20000)
+  public void testStrictMemoryEnforcementRequiresMemoryEnabled() {
+    // memory.enabled defaults to false, memory.enforced defaults to true.
+    // Strict enforcement must NOT be considered active in this case, otherwise
+    // the polling-based memory check would be wrongly skipped.
+    ContainersMonitorImpl cm =
+        new ContainersMonitorImpl(mock(ContainerExecutor.class),
+            mock(AsyncDispatcher.class), mock(Context.class));
+    cm.init(getConfForCM(true, false, 8192, 2.1f));
+    assertFalse("Strict memory enforcement must be off when "
+            + "yarn.nodemanager.resource.memory.enabled is false, "
+            + "even if yarn.nodemanager.resource.memory.enforced is true",
+        cm.isStrictMemoryEnforcementEnabled());
+
+    // memory.enabled = true, memory.enforced = true -> active.
+    cm = new ContainersMonitorImpl(mock(ContainerExecutor.class),
+        mock(AsyncDispatcher.class), mock(Context.class));
+    YarnConfiguration conf = getConfForCM(true, false, 8192, 2.1f);
+    conf.setBoolean(YarnConfiguration.NM_MEMORY_RESOURCE_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.NM_MEMORY_RESOURCE_ENFORCED, true);
+    cm.init(conf);
+    assertTrue("Strict memory enforcement must be on when both memory.enabled "
+            + "and memory.enforced are true",
+        cm.isStrictMemoryEnforcementEnabled());
+
+    // memory.enabled = true, memory.enforced = false -> not active.
+    cm = new ContainersMonitorImpl(mock(ContainerExecutor.class),
+        mock(AsyncDispatcher.class), mock(Context.class));
+    conf = getConfForCM(true, false, 8192, 2.1f);
+    conf.setBoolean(YarnConfiguration.NM_MEMORY_RESOURCE_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.NM_MEMORY_RESOURCE_ENFORCED, false);
+    cm.init(conf);
+    assertFalse("Strict memory enforcement must be off when "
+            + "memory.enforced is false",
+        cm.isStrictMemoryEnforcementEnabled());
+  }
+
   private YarnConfiguration getConfForCM(boolean pMemEnabled,
       boolean vMemEnabled, int nmPmem, float vMemToPMemRatio) {
     YarnConfiguration conf = new YarnConfiguration();