[ https://issues.apache.org/jira/browse/HDFS-7889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14382364#comment-14382364 ]
Zhe Zhang commented on HDFS-7889: --------------------------------- Thanks Bo for the patch! The main striped writing logic in {{DFSStripedOutputStream#writeChunkImpl}} look good to me. Please find an early review below. More details to follow soon. # Isn't {{currentPacket}} always null after {{super.writeChunk}}? Do we still need to following? {code} // If current packet has not been enqueued for transmission, // we need to check if the cell buffer is full if (currentPacket != null && sizeOfCellInBuffer[curIdx] == cellSize) { if (DFSClient.LOG.isDebugEnabled()) { DFSClient.LOG.debug("DFSClient writeChunk cell buffer full seqno=" + currentPacket.getSeqno() + ", curIdx=" + curIdx + ", src=" + src + ", bytesCurBlock=" + streamer().getBytesCurBlock() + ", blockSize=" + blockSize + ", appendChunk=" + streamer().getAppendChunk()); } streamer().waitAndQueuePacket(currentPacket); currentPacket = null; adjustChunkBoundary(); encounterBlockBoundary(); } {code} # Shouldn't we call super.writeChunkImpl instead? We definitely don't want to handle the tracing scope twice. {code} super.writeChunk(... {code} # Maybe {{streamer()}} should be {{getStreamer()}}, and {{leadingStreamer()}} should be {{getLeadingStreamer()}}? # Can we get rid of {{sizeOfCellInBuffer}}, by using {{cellBuffers\[i\].array().length}}? # {{hookAfterRetrievingBlock}} looks complex and I'm not sure if it's the best way to allocate new blocks. Can we move it to {{DataStreamer}}? > Subclass DFSOutputStream to support writing striping layout files > ----------------------------------------------------------------- > > Key: HDFS-7889 > URL: https://issues.apache.org/jira/browse/HDFS-7889 > Project: Hadoop HDFS > Issue Type: Sub-task > Reporter: Li Bo > Assignee: Li Bo > Attachments: HDFS-7889-001.patch > > > After HDFS-7888, we can subclass {{DFSOutputStream}} to support writing > striping layout files. -- This message was sent by Atlassian JIRA (v6.3.4#6332)