[jira] [Commented] (HDFS-3388) GetJournalEditServlet should catch more exceptions, not just IOException
[
https://issues.apache.org/jira/browse/HDFS-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13272896#comment-13272896
]
Tsz Wo (Nicholas), SZE commented on HDFS-3388:
--
- Since GetJournalEditServletFaultInjector is an inner class, let's simply call
it FaultInjector.
- GetJournalEditServletFaultInjector.getInstance() is not used (and it should
be static if you want to use it.)
- change
{code}
new String(path1.toString() + "/current")
{code}
to
{code}
"path1 + "/current"
{code}
> GetJournalEditServlet should catch more exceptions, not just IOException
>
>
> Key: HDFS-3388
> URL: https://issues.apache.org/jira/browse/HDFS-3388
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha, name-node
>Reporter: Brandon Li
>Assignee: Brandon Li
> Attachments: HDFS-3388.HDFS-3092.patch, HDFS-3388.HDFS-3092.patch
>
>
> GetJournalEditServlet has the same problem as that of GetImageServlet
> (HDFS-3330). It should be fixed in the same way. Also need to make
> CheckpointFaultInjector visible for journal service tests.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HDFS-3388) GetJournalEditServlet should catch more exceptions, not just IOException
[ https://issues.apache.org/jira/browse/HDFS-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13272521#comment-13272521 ] Brandon Li commented on HDFS-3388: -- Todd, it seems to be doable if we make the fault injector class as nested static class. In term of the manageability of fault injector classes, people could argue that it might be better to keep all the fault injector classes in a different package. Let me upload the new path and see what other folks think. > GetJournalEditServlet should catch more exceptions, not just IOException > > > Key: HDFS-3388 > URL: https://issues.apache.org/jira/browse/HDFS-3388 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: ha, name-node >Reporter: Brandon Li >Assignee: Brandon Li > Attachments: HDFS-3388.HDFS-3092.patch > > > GetJournalEditServlet has the same problem as that of GetImageServlet > (HDFS-3330). It should be fixed in the same way. Also need to make > CheckpointFaultInjector visible for journal service tests. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HDFS-3388) GetJournalEditServlet should catch more exceptions, not just IOException
[ https://issues.apache.org/jira/browse/HDFS-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270995#comment-13270995 ] Todd Lipcon commented on HDFS-3388: --- Hey Brandon. I personally prefer to keep the fault injector classes local to each individual component being fault-tested. Otherwise it might grow unmanageable over time. They could even become package-private inner classes of each individual class that uses this fault injection technique. I think that's cleanest if doable? > GetJournalEditServlet should catch more exceptions, not just IOException > > > Key: HDFS-3388 > URL: https://issues.apache.org/jira/browse/HDFS-3388 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: ha, name-node >Reporter: Brandon Li >Assignee: Brandon Li > Attachments: HDFS-3388.HDFS-3092.patch > > > GetJournalEditServlet has the same problem as that of GetImageServlet > (HDFS-3330). It should be fixed in the same way. Also need to make > CheckpointFaultInjector visible for journal service tests. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HDFS-3388) GetJournalEditServlet should catch more exceptions, not just IOException
[ https://issues.apache.org/jira/browse/HDFS-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270976#comment-13270976 ] Brandon Li commented on HDFS-3388: -- So the root cause of the false-OK-response is due to the output stream close operation (response.getOutputStream().close()), which sends false-OK-response to client before HTTP server processes the uncaught exception. I did a test with the above mentioned change and also make GetJournalEditServlet.doGet throw an Error. Even though this error can't be caught by GetJournalEditServlet.doGet, it is handled by HTTP server and the client can get the error message. Thanks, Nicholas. > GetJournalEditServlet should catch more exceptions, not just IOException > > > Key: HDFS-3388 > URL: https://issues.apache.org/jira/browse/HDFS-3388 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: ha, name-node >Reporter: Brandon Li >Assignee: Brandon Li > Attachments: HDFS-3388.HDFS-3092.patch > > > GetJournalEditServlet has the same problem as that of GetImageServlet > (HDFS-3330). It should be fixed in the same way. Also need to make > CheckpointFaultInjector visible for journal service tests. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HDFS-3388) GetJournalEditServlet should catch more exceptions, not just IOException
[
https://issues.apache.org/jira/browse/HDFS-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270955#comment-13270955
]
Tsz Wo (Nicholas), SZE commented on HDFS-3388:
--
The correct way to fix the bug is not to close output stream when an error is
sent. Below is a suggestion.
{code}
@@ -140,8 +141,12 @@
DataTransferThrottler throttler =
GetImageServlet.getThrottler(conf);
// send edits
-TransferFsImage.getFileServer(response.getOutputStream(),
-editFile, throttler);
+OutputStream out = response.getOutputStream();
+try {
+ TransferFsImage.getFileServer(out, editFile, throttler);
+} finally {
+ out.close();
+}
} else {
response
.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED,
@@ -155,8 +160,6 @@
String errMsg = "getedit failed. " + StringUtils.stringifyException(ie);
response.sendError(HttpServletResponse.SC_GONE, errMsg);
throw new IOException(errMsg);
-} finally {
- response.getOutputStream().close();
}
}
{code}
> GetJournalEditServlet should catch more exceptions, not just IOException
>
>
> Key: HDFS-3388
> URL: https://issues.apache.org/jira/browse/HDFS-3388
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha, name-node
>Reporter: Brandon Li
>Assignee: Brandon Li
> Attachments: HDFS-3388.HDFS-3092.patch
>
>
> GetJournalEditServlet has the same problem as that of GetImageServlet
> (HDFS-3330). It should be fixed in the same way. Also need to make
> CheckpointFaultInjector visible for journal service tests.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HDFS-3388) GetJournalEditServlet should catch more exceptions, not just IOException
[ https://issues.apache.org/jira/browse/HDFS-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270853#comment-13270853 ] Brandon Li commented on HDFS-3388: -- Renaming it sounds better to me. How about calling it FaultInjector so it can also be used for error simulation in other ears inside HDFS? > GetJournalEditServlet should catch more exceptions, not just IOException > > > Key: HDFS-3388 > URL: https://issues.apache.org/jira/browse/HDFS-3388 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: ha, name-node >Reporter: Brandon Li >Assignee: Brandon Li > Attachments: HDFS-3388.HDFS-3092.patch > > > GetJournalEditServlet has the same problem as that of GetImageServlet > (HDFS-3330). It should be fixed in the same way. Also need to make > CheckpointFaultInjector visible for journal service tests. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HDFS-3388) GetJournalEditServlet should catch more exceptions, not just IOException
[ https://issues.apache.org/jira/browse/HDFS-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270832#comment-13270832 ] Todd Lipcon commented on HDFS-3388: --- Good idea. Maybe we should also rename CheckpointFaultInjector to FileTransferFaultInjector or something? Or split it in two, since if I recall correctly some of the fault points are checkpoint-specific whereas others are just for the GetImageServlet. > GetJournalEditServlet should catch more exceptions, not just IOException > > > Key: HDFS-3388 > URL: https://issues.apache.org/jira/browse/HDFS-3388 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: ha, name-node >Reporter: Brandon Li >Assignee: Brandon Li > Attachments: HDFS-3388.HDFS-3092.patch > > > GetJournalEditServlet has the same problem as that of GetImageServlet > (HDFS-3330). It should be fixed in the same way. Also need to make > CheckpointFaultInjector visible for journal service tests. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
