singer-bin opened a new pull request, #8595:
URL: https://github.com/apache/hadoop/pull/8595

   ### Description of PR
   Follow-up / addendum to #8573 (YARN-11967).
   
   After #8573, `strictMemoryEnforcement` defaults to `false` — it now requires 
**both** `yarn.nodemanager.resource.memory.enabled` (default `false`) and 
`yarn.nodemanager.resource.memory.enforced` (default `true`). As a result, 
`ContainersMonitorImpl#checkLimit()` no longer takes the strict-only 
early-return path by default and the polling-based pmem/vmem check actually 
runs.
   
   `DistributedShellBaseTest#setupInternal` enables the polling checks:
   
   ```java
   conf.setBoolean(YarnConfiguration.NM_PMEM_CHECK_ENABLED, true);
   conf.setBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED, true);
   ```
   
   Historically these settings were **silent no-ops**, because strict 
enforcement was on by default and `checkLimit()` short-circuited before 
reaching the polling logic. After YARN-11967 they take effect, and 
MiniYARNCluster containers running under the tiny `MIN_ALLOCATION_MB` limit get 
killed for exceeding pmem/vmem, which is what @pan3793 observed on trunk after 
the merge.
   
   ### Fix
   Disable the polling pmem/vmem checks in the base test config. 
`ContainersMonitorImpl` itself is still enabled (governed by 
`NM_CONTAINER_MONITOR_ENABLED`, default `true`), so resource-utilization 
tracking used by the opportunistic-container setup below is preserved. These 
tests do not exercise memory enforcement (no `KILLED_EXCEEDED_PMEM` / 
`KILLED_EXCEEDED_VMEM` assertions anywhere in the module).
   
   ### Why the fix is safe
   - `isContainerMonitorEnabled()` reads only `NM_CONTAINER_MONITOR_ENABLED` — 
it does not depend on the pmem/vmem check flags, so `ContainersMonitorImpl` 
remains active and reports utilization for opportunistic scheduling.
   - The DistributedShell tests never asserted on memory-kill behaviour; the 
flags were effectively unused before this change.
   - Same-file behaviour is otherwise unchanged (comment updated to explain the 
rationale).
   
   ### How was this patch tested?
   Only Linux CI can reproduce the original failure (macOS has no `/proc`, so 
`ProcfsBasedProcessTree` cannot sample memory locally). Relying on GHA to 
validate on `apache/hadoop` trunk.
   
   ### For code changes:
   - [x] Does the title or this PR starts with the corresponding JIRA issue id 
(e.g. 'HADOOP-17799. Your PR title ...')?
   
   cc @pan3793 @Hexiaoqiao
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to