TEZ-4088: Create in-memory ifile writer for transferring small payload #50
Conversation
…oads (follow up of TEZ-4075)
…oads (follow up of TEZ-4075)
tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/sort/impl/IFile.java
Outdated
Show resolved
Hide resolved
| this.taskOutput = taskOutput; | ||
| this.bufferFull = (cacheStream == null); | ||
| this.cacheSize = (cacheStream == null) ? 0 : | ||
| Math.max(getBaseCacheSize(), cacheStream.available()); |
There was a problem hiding this comment.
Nit: CacheSize could be replaced by availableDataSize which combines this calculation along with the one done in shouldWriteToDisk. Will make the code a little easier to read.
return totalSize >= (cacheSize - (checksumSize +
(2 * WritableUtils.getVIntSize(EOF_MARKER))));
There was a problem hiding this comment.
Yes, went through it one more time. Actually cacheSize itself is not needed. We can fix this with totalSize and cacheStream.getLimit(). I will fix this in next commit.
| if (outputPath == null) { | ||
| outputPath = taskOutput.getOutputFileForWrite(); | ||
| } | ||
| LOG.info("Creating file " + outputPath); |
There was a problem hiding this comment.
Nit: Add "Switching from mem stream to disk stream" to log message.
There was a problem hiding this comment.
Will fix this in next commit.
| outputPath = taskOutput.getOutputFileForWrite(); | ||
| } | ||
| LOG.info("Creating file " + outputPath); | ||
| FSDataOutputStream newRawOut = fs.create(outputPath); |
There was a problem hiding this comment.
Most of this is identical to what is done in the IFile constructor. Think it makes sense to break this out into a separate method, and re-use it, instead of repeating the code.
There was a problem hiding this comment.
Yes, makes sense to create setupOutputStream() in IFile itself.
| */ | ||
| public byte[] getData() { | ||
| if (!isDataFlushedToDisk()) { | ||
| byte[] buf = new byte[cacheStream.size()]; |
There was a problem hiding this comment.
Nit: Not sure if this really matters. Could avoid this copy by wrapping in a ByteBuffer (another copy is happening anyway when serializing to protobuf)
There was a problem hiding this comment.
Will fix this in next commit.
…oads (follow up of TEZ-4075)
|
Fixed review comments in recent commit. |
|
Precommit builds passed. Link: https://issues.apache.org/jira/browse/TEZ-4088?focusedCommentId=16949113&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16949113 Thanks @sidseth . I will merge it with master branch. |
No description provided.