[Hadoop-common-dev] [jira] Created: (HADOOP-2926) Ignoring IOExceptions on close

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

I am not sure why this would not work now..
Then shouldn't we use it, instead of using IOUtils.closeStream() at all?
Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

This bug has the potential that errors-writing-to-block file go undetected. This can (theoretically) result in a higher probability of data corruption.
But every close() added in the patch is for a socket or socket stream. Strictly socket streams don't even need to be closed. Am I missing something?
Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Fix For: 0.16.1

Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.

Ignoring IOExceptions on close
------------------------------

Key: HADOOP-2926
URL: https://issues.apache.org/jira/browse/HADOOP-2926
Project: Hadoop Core
Issue Type: Bug
Components: dfs
Affects Versions: 0.16.0
Reporter: Owen O'Malley
Assignee: dhruba borthakur
Priority: Critical
Attachments: closeStream.patch


Currently in HDFS there are a lot of calls to IOUtils.closeStream that are from finally blocks. I'm worried that this can lead to data corruption in the file system. Take the first instance in DataNode.copyBlock: it writes the block and then calls closeStream on the output stream. If there is an error at the end of the file that is detected in the close, it will be *completely* ignored. Note that logging the error is not enough, the error should be thrown so that the client knows the failure happened.
{code}
try {
file1.write(...);
file2.write(...);
} finally {
IOUtils.closeStream(file);
}
{code}
is *bad*. It must be rewritten as:
{code}
try {
file1.write(...);
file2.write(...);
file1.close(...);
file2.close(...);
} catch (IOException ie) {
IOUtils.closeStream(file1);
IOUtils.closeStream(file2);
throw ie;
}
{code}
I also think that IOUtils.closeStream should be renamed IOUtils.cleanupFailedStream or something to make it clear it can only be used after the write operation has failed and is being cleaned up.