[ 
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)

Reply via email to