[
https://issues.apache.org/jira/browse/HDFS-3405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13898365#comment-13898365
]
Andrew Wang commented on HDFS-3405:
-----------------------------------
Hi Vinayakumar, nice work on this JIRA! It's something I've always though we
should fix. Here are some review comments:
Overall:
* It seems unnecessary to have two servlets here. Why not have a single
combined ImageServlet? It's okay to rename the URL to something like
{{/imagetransfer}} too, it's an internal API.
* If you do decide to have multiple servlets, we need interface annotations and
class javadoc for the new classes
* There's a conf option right now for image transfer timeout and image transfer
throttling. It looks like the PUT no longer obeys either of these config keys.
* We also should update the hdfs-default.xml description of
dfs.image.transfer.timeout, since it is in fact a socket timeout now that the
GETs aren't nested. I think we should also lower it back down to 60s, a more
normal value for a socket timeout.
TransferFsImage:
* Comment "Uploades the imagefile using HTTP Post method" should read "Uploads
the fsimage using HTTP PUT".
* Instead of building the URL with query parameters up manually, we can use
HttpClient's URIBuilder. This would be a nice refactor to do elsewhere too.
* In writeFileToPutRequest, we can use IOUtils.copy instead of doing our own
buffering. The existing for loop syntax is also messy, it'd be better as a
while loop.
PutImageServlet
* Seems unnecessary to handle putting an image on both POST and PUT, let's just
choose one.
Tests:
* Need to updated javadoc for testSecondaryFailsWithErrorBeforeSettingHeaders,
since we removed the "above test".
* Have you tried this with a large fsimage, e.g. 2GB+? Last time I looked into
this, we could sometimes run into issues with HttpURLConnection.
* I'm surprised there aren't any new tests. Testing that the timeout and
transfer rate config keys apply to the POST would be good, as well as unit
tests for secure HA/non-HA environments if we don't already have that, and the
SPNEGO stuff.
> Checkpointing should use HTTP POST or PUT instead of GET-GET to send merged
> fsimages
> ------------------------------------------------------------------------------------
>
> Key: HDFS-3405
> URL: https://issues.apache.org/jira/browse/HDFS-3405
> Project: Hadoop HDFS
> Issue Type: Improvement
> Affects Versions: 1.0.0, 3.0.0, 2.0.5-alpha
> Reporter: Aaron T. Myers
> Assignee: Vinayakumar B
> Attachments: HDFS-3405.patch, HDFS-3405.patch, HDFS-3405.patch,
> HDFS-3405.patch, HDFS-3405.patch, HDFS-3405.patch, HDFS-3405.patch,
> HDFS-3405.patch, HDFS-3405.patch, HDFS-3405.patch, HDFS-3405.patch,
> HDFS-3405.patch, HDFS-3405.patch
>
>
> As Todd points out in [this
> comment|https://issues.apache.org/jira/browse/HDFS-3404?focusedCommentId=13272986&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13272986],
> the current scheme for a checkpointing daemon to upload a merged fsimage
> file to an NN is to issue an HTTP get request to tell the target NN to issue
> another GET request back to the checkpointing daemon to retrieve the merged
> fsimage file. There's no fundamental reason the checkpointing daemon can't
> just use an HTTP POST or PUT to send back the merged fsimage file, rather
> than the double-GET scheme.
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
