[ https://issues.apache.org/jira/browse/HDFS-15679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17251824#comment-17251824 ]
Jim Brennan commented on HDFS-15679: ------------------------------------ [~eddyxu], [~andrew.wang], we would appreciate your review. We are trying to understand why the unit tests for -HDFS-12612- expect closeImpl() to be called twice, when it seems like the intent was to ensure it is closed only once. > DFSOutputStream close should be a no-op when called multiple times > ------------------------------------------------------------------ > > Key: HDFS-15679 > URL: https://issues.apache.org/jira/browse/HDFS-15679 > Project: Hadoop HDFS > Issue Type: Bug > Reporter: Ahmed Hussein > Assignee: Ahmed Hussein > Priority: Major > Labels: pull-request-available > Time Spent: 1h 20m > Remaining Estimate: 0h > > While I was looking into the incorrect implementation of HDFS-15678, I found > that once I implement the correct logic, the Junit test fails. > It turns out that there is inconsistency in {{DFSOutputStream.closeImpl()}} > introduced by HDFS-13164. > The change in [that > line|https://github.com/apache/hadoop/commit/51088d323359587dca7831f74c9d065c2fccc60d#diff-3a80b95578dc5079cebf0441e1dab63d5844c02fa2d04071c165ec4f7029f918R860] > makes the close() throws exception multiple times which contradicts with > HDFS-5335. > That change breaks the semantic of {{close()}}. For convenience, this is a > test code that fails because of the change in HDFS-13164. > {code:java} > public void testCloseTwice() throws IOException { > DistributedFileSystem fs = cluster.getFileSystem(); > FSDataOutputStream os = fs.create(new Path("/test")); > DFSOutputStream dos = (DFSOutputStream) Whitebox.getInternalState(os, > "wrappedStream"); > DataStreamer streamer = (DataStreamer) Whitebox > .getInternalState(dos, "streamer"); > @SuppressWarnings("unchecked") > LastExceptionInStreamer ex = (LastExceptionInStreamer) Whitebox > .getInternalState(streamer, "lastException"); > Throwable thrown = (Throwable) Whitebox.getInternalState(ex, "thrown"); > Assert.assertNull(thrown); > // force stream to break. output stream needs to encounter a real > // error to properly mark it closed with an exception > cluster.shutdown(true, false); > try { > dos.close(); > Assert.fail("should have thrown"); > } catch (IOException e) { > Assert.assertEquals(e.toString(), EOFException.class, e.getClass()); > } > thrown = (Throwable) Whitebox.getInternalState(ex, "thrown"); > Assert.assertNull(thrown); > dos.close(); > // even if the exception is set again, close should not throw it > ex.set(new IOException("dummy")); > dos.close(); > } > {code} -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org