[
https://issues.apache.org/jira/browse/MAPREDUCE-6628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15256933#comment-15256933
]
Mariappan Asokan commented on MAPREDUCE-6628:
---------------------------------------------
I created a test which prints out memory statistics (works only on Linux)
before and after the sort phase in the map task. I specified an encryption
buffer size of 64 MB. I did this to keep the number of reducers small so that
the test would finish fast. I could have left the buffer size as default (128
KB) and increased the number of reducers to more than 5000 to get the same
effect. In any case, following are the statistics before the patch.
{panel:title=Memory statistics before the patch}
MapOutputCopier.init(): Memory usage:
VmPeak: 1326992 kB
VmSize: 1326992 kB
VmLck: 0 kB
VmPin: 0 kB
VmHWM: 228188 kB
VmRSS: 228188 kB
VmData: 1256456 kB
VmStk: 156 kB
VmExe: 4 kB
VmLib: 15908 kB
VmPTE: 676 kB
VmSwap: 0 kB
MapOutputCopier.close(): Memory usage:
VmPeak: 2049988 kB
VmSize: 1722288 kB
VmLck: 0 kB
VmPin: 0 kB
VmHWM: 1335640 kB
VmRSS: 1069044 kB
VmData: 1651752 kB
VmStk: 156 kB
VmExe: 4 kB
VmLib: 15908 kB
VmPTE: 2352 kB
VmSwap: 0 kB
{panel}
After the patch was applied, I reran the test and here are the statistics:
{panel:title=Memory statistics after the patch}
MapOutputCopier.init(): Memory usage:
VmPeak: 1325348 kB
VmSize: 1325348 kB
VmLck: 0 kB
VmPin: 0 kB
VmHWM: 228308 kB
VmRSS: 228308 kB
VmData: 1254812 kB
VmStk: 156 kB
VmExe: 4 kB
VmLib: 15908 kB
VmPTE: 676 kB
VmSwap: 0 kB
MapOutputCopier.close(): Memory usage:
VmPeak: 1460344 kB
VmSize: 1329264 kB
VmLck: 0 kB
VmPin: 0 kB
VmHWM: 922732 kB
VmRSS: 795800 kB
VmData: 1258728 kB
VmStk: 156 kB
VmExe: 4 kB
VmLib: 15908 kB
VmPTE: 1800 kB
VmSwap: 0 kB
{panel}
It can be noticed that after the sort completes, the physical as well virtual
memory usage are much higher in the case before the patch.
I am including the test as part of the patch. However, the test itself cannot
verify on each run that memory was freed properly since there is no specific
value to check against. The memory statistics are output in stdout of the map
task.
> Potential memory leak in CryptoOutputStream
> -------------------------------------------
>
> Key: MAPREDUCE-6628
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-6628
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: security
> Reporter: Mariappan Asokan
> Assignee: Mariappan Asokan
> Attachments: MAPREDUCE-6628.001.patch, MAPREDUCE-6628.002.patch,
> MAPREDUCE-6628.003.patch, MAPREDUCE-6628.004.patch
>
>
> There is a potential memory leak in {{CryptoOutputStream.java.}} It
> allocates two direct byte buffers ({{inBuffer}} and {{outBuffer}}) that get
> freed when {{close()}} method is called. Most of the time, {{close()}}
> method is called. However, when writing to intermediate Map output file or
> the spill files in {{MapTask}}, {{close()}} is never called since calling so
> would close the underlying stream which is not desirable. There is a single
> underlying physical stream that contains multiple logical streams one per
> partition of Map output.
> By default the amount of memory allocated per byte buffer is 128 KB and so
> the total memory allocated is 256 KB, This may not sound much. However, if
> the number of partitions (or number of reducers) is large (in the hundreds)
> and/or there are spill files created in {{MapTask}}, this can grow into a few
> hundred MB.
> I can think of two ways to address this issue:
> h2. Possible Fix - 1
> According to JDK documentation:
> {quote}
> The contents of direct buffers may reside outside of the normal
> garbage-collected heap, and so their impact upon the memory footprint of an
> application might not be obvious. It is therefore recommended that direct
> buffers be allocated primarily for large, long-lived buffers that are subject
> to the underlying system's native I/O operations. In general it is best to
> allocate direct buffers only when they yield a measureable gain in program
> performance.
> {quote}
> It is not clear to me whether there is any benefit of allocating direct byte
> buffers in {{CryptoOutputStream.java}}. In fact, there is a slight CPU
> overhead in moving data from {{outBuffer}} to a temporary byte array as per
> the following code in {{CryptoOutputStream.java}}.
> {code}
> /*
> * If underlying stream supports {@link ByteBuffer} write in future, needs
> * refine here.
> */
> final byte[] tmp = getTmpBuf();
> outBuffer.get(tmp, 0, len);
> out.write(tmp, 0, len);
> {code}
> Even if the underlying stream supports direct byte buffer IO (or direct IO in
> OS parlance), it is not clear whether it will yield any measurable
> performance gain.
> The fix would be to allocate a ByteBuffer on the heap for inBuffer and wrap a
> byte array in a {{ByteBuffer}} for {{outBuffer}}. By the way, the
> {{inBuffer}} and {{outBuffer}} have to be {{ByteBuffer}} as demanded by the
> {{encrypt()}} method in {{Encryptor}}.
> h2. Possible Fix - 2
> Assuming that we want to keep the buffers as direct byte buffers, we can
> create a new constructor to {{CryptoOutputStream}} and pass a boolean flag
> {{ownOutputStream}} to indicate whether the underlying stream will be owned
> by {{CryptoOutputStream}}. If it is true, then calling the {{close()}} method
> will close the underlying stream. Otherwise, when {{close()}} is called only
> the direct byte buffers will be freed and the underlying stream will not be
> closed.
> The scope of changes for this fix will be somewhat wider. We need to modify
> {{MapTask.java}}, {{CryptoUtils.java}}, and {{CryptoFSDataOutputStream.java}}
> as well to pass the ownership flag mentioned above.
> I can post a patch for either of the above. I welcome any other ideas from
> developers to fix this issue.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
