[ 
https://issues.apache.org/jira/browse/HDFS-10383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15283246#comment-15283246
 ] 

Mingliang Liu commented on HDFS-10383:
--------------------------------------

Thanks [~xyao] for the review.

I reverted the change of {{createStripedFile()}} because of its intension:
{code:java}
  /**
   * Creates the metadata of a file in striped layout. This method only
   * manipulates the NameNode state without injecting data to DataNode.
   * You should disable periodical heartbeat before use this.
   * ...
   */
{code}

You made a good point about the double {{completeFile()}}. Yes the 
{{DFSStripedStream#close()}} will try to complete the file implicitly when 
closing, leading to a double complete file problem. However, there is no data 
is injected to DN, plus the client is not aware of the added block by 
{{addBlockToFile()}} method which also only manipulates NameNode states. 
Suppose we delete the 1st explicit {{completeFile()}} in 
{{createStripedFile()}}, the {{DFSStripedOutputStream}} in try-with-resource 
will complete a null last block when closing, which will fail for sure.

I think this maybe the reason why {{createStripedFile()}} completes file 
explicitly with a valid last block value. The side effect is that, the 2nd 
implicit {{completeFile()}} will fail with a {{LeaseExpiredException: File is 
not open for writing}}. To handle this, the {{IOUtils#cleanup}} swallows it in 
the finally block.

> Safely close resources in DFSTestUtil
> -------------------------------------
>
>                 Key: HDFS-10383
>                 URL: https://issues.apache.org/jira/browse/HDFS-10383
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: test
>            Reporter: Mingliang Liu
>            Assignee: Mingliang Liu
>         Attachments: HDFS-10383.000.patch, HDFS-10383.001.patch, 
> HDFS-10383.002.patch
>
>
> There are a few of methods in {{DFSTestUtil}} that do not close the resource 
> safely, or elegantly. We can use the try-with-resource statement to address 
> this problem.
> Specially, as {{DFSTestUtil}} is popularly used in test, we need to preserve 
> any exceptions thrown during the processing of the resource while still 
> guaranteeing it's closed finally. Take for example,the current implementation 
> of {{DFSTestUtil#createFile()}} closes the FSDataOutputStream in the 
> {{finally}} block, and when closing if the internal 
> {{DFSOutputStream#close()}} throws any exception, which it often does, the 
> exception thrown during the processing will be lost. See this [test 
> failure|https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/testReport/org.apache.hadoop.hdfs/TestAsyncDFSRename/testAggressiveConcurrentAsyncRenameWithOverwrite/],
>  and we have to guess what was the root cause.
> Using try-with-resource, we can close the resources safely, and the 
> exceptions thrown both in processing and closing will be available (closing 
> exception will be suppressed).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to