TaskMemoryMonitorThread might shoot down tasks even if their processes momentarily exceed the requested memory

• Key: HADOOP-5883
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Hemanth Yamijala
• Reporter: Hemanth Yamijala
• Fix Version/s: 0.20.1

Description

Currently the TaskMemoryMonitorThread kills tasks as soon as it detects they are consuming more memory than the max value specified. There are valid cases (see HADOOP-5059) where if a program is executed from the task, it might momentarily occupy twice the amount of memory for a short time. Ideally the monitoring thread should handle this case.

Comments

[Hemanth Yamijala]

As per some offline discussions with Eric, Arun and Vinod, we decided to follow the approach mentioned below for improving the monitoring logic:

The particular case in HADOOP-5059 indicates that any process that executes a program from the Java code could occupy (momentarily) twice the amount of memory, due to the JVM's fork()+exec() approach. This happens during the fork() and before the exec() completes.

• Hence, if a process tree is over twice the configured memory limit, then it is shot down immediately.
• Also, if there are processes in the tree older than a certain interval and these processes alone contribute to being over limit, then the task is shot down.

By corollary, this means that if there are no processes in the tree older than a certain interval and the tree is still over limit, we give it a benefit of doubt to accomodate the case in HADOOP-5059.

[Hemanth Yamijala]

The attached patch file incorporates the changes as mentioned in the earlier comment.

The key change was to determine processes older than a certain time. To do this, the process tree class keeps track of an 'age' of the process - which is how many time the process tree has seen a process with this PID. This count is updated every time the process tree is refreshed - which is once every monitoring iteration. The monitoring thread can now ask for cumulative virtual memory of processes over a certain 'age'. For the sake of simplicity, I've assumed the monitoring interval determines how aged processes are.

It is possible to do something more sophisticated - for e.g. we could determine the walltime of the process by making a system call. There doesn't seem to be a direct API for getting the 'walltime' of a process. One hack would be to see the created time of the pid directory in /proc and then subtract it from timeofday each time. However, it seems like this could be a costly operation, while not giving way too much more accuracy.

Summary of the changes:

• TaskMemoryManagerThread: changes to the logic to determine if a task is over limit.
• ProcfsBasedProcessTree: introduces age for processes and updates them
• The rest of the changes were a lot to do with enabling fast unit tests to be written. I think it is a good idea to move many of the tests for these two classes to use the testing mechanism I'm using. But that's the focus of another JIRA.
• Introduced new tests for testing just these changes.

I suppose this patch will need merging with HADOOP-5881, which is likely to go in first. I will update the patch with those changes once it's ready. This is just put up for early consumption.

Also missing is documentation updates for the new semantics of monitoring. Again, I will finish that after the HADOOP-5881 merge.

[Hemanth Yamijala]

Cancelling for updating with a new patch merging with HADOOP-5881.

[Hemanth Yamijala]

New patch merged with HADOOP-5881. This should be applied on top of HADOOP-5881, or after it is committed.

[Vinod K V]

Looked at the patch. Looks very good overall. Only few comments:

TaskMemoryMangerThread.java

• (+292) limit != JobConf.DISABLED_MEMORY_LIMIT This check is not needed anymore as JT doesn't access any jobs with no configuration with HADOOP-5881.
• (+207) We can avoid duplicate calls to ProcessTree.getCumulativeVmem() just before and inside ProcessTreeOverLimit(). For this, we can retain the current method for usage in testcases and add a new method that takes both the limits as parameters.

TestProcfsBasedProcessTree.setupProcfsRootDir() :

• Cant' we just clean the proc dir ourselves before the test?
• We can check for the return value of mkdirs instead of making a new File.exists() call.

Minor:

• Missing @throws IOException in javadoc of all the methods newly added in the two testcases.

[Hemanth Yamijala]

New patch incorporating Vinod's comments.

[Hemanth Yamijala]

Results of test-patch on the new patch.

[exec] +1 overall.
[exec]
[exec] +1 @author. The patch does not contain any @author tags.
[exec]
[exec] +1 tests included. The patch appears to include 6 new or modified tests.
[exec]
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec]
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec]
[exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
[exec]
[exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
[exec]
[exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

Also, ran all unit tests. TestQueueCapacities timed out. All other test cases passed.

[Hemanth Yamijala]

Patch generated against the 20 branch.

[Hemanth Yamijala]

The earlier patch had a compile time error. Updating a new patch to fix it.

[Hemanth Yamijala]

Ran tests against the 20 branch. TestDistributedFileSystem failed, but the patch in no way touches code in this test case. All other tests passed. I will commit this fix.

[Hemanth Yamijala]

I just committed this to trunk and branch 0.20.

[Hudson]

Integrated in Hadoop-trunk #863 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/863/)