diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ServletUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ServletUtil.java
index 993394d59bf..a401f3f65a4 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ServletUtil.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ServletUtil.java
@@ -60,6 +60,20 @@ public class ServletUtil {
s = s.trim();
return s.length() == 0? null: s;
}
+
+ /**
+ * @return a long value as passed in the given parameter, throwing
+ * an exception if it is not present or if it is not a valid number.
+ */
+ public static long parseLongParam(ServletRequest request, String param)
+ throws IOException {
+ String paramStr = request.getParameter(param);
+ if (paramStr == null) {
+ throw new IOException("Invalid request has no " + param + " parameter");
+ }
+
+ return Long.valueOf(paramStr);
+ }
public static final String HTML_TAIL = "
\n"
+ "Hadoop, "
diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
index cdfdec2328a..c26bda149f4 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -249,6 +249,8 @@ Release 2.0.1-alpha - UNRELEASED
HDFS-3575. HttpFS does not log Exception Stacktraces (brocknoland via tucu)
+ HDFS-3574. Fix small race and do some cleanup in GetImageServlet (todd)
+
BREAKDOWN OF HDFS-3042 SUBTASKS
HDFS-2185. HDFS portion of ZK-based FailoverController (todd)
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java
index 958cb14b9db..074fd237e54 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/GetImageServlet.java
@@ -45,8 +45,10 @@ import org.apache.hadoop.hdfs.server.protocol.RemoteEditLog;
import org.apache.hadoop.hdfs.util.DataTransferThrottler;
import org.apache.hadoop.hdfs.util.MD5FileUtils;
import org.apache.hadoop.http.HttpServer;
+import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.io.MD5Hash;
import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.ServletUtil;
import org.apache.hadoop.util.StringUtils;
import com.google.common.annotations.VisibleForTesting;
@@ -125,23 +127,14 @@ public class GetImageServlet extends HttpServlet {
throw new IOException(errorMessage);
}
CheckpointFaultInjector.getInstance().beforeGetImageSetsHeaders();
- setFileNameHeaders(response, imageFile);
- setVerificationHeaders(response, imageFile);
- // send fsImage
- TransferFsImage.getFileServer(response.getOutputStream(), imageFile,
- getThrottler(conf));
+ serveFile(imageFile);
} else if (parsedParams.isGetEdit()) {
long startTxId = parsedParams.getStartTxId();
long endTxId = parsedParams.getEndTxId();
File editFile = nnImage.getStorage()
.findFinalizedEditsFile(startTxId, endTxId);
- setVerificationHeaders(response, editFile);
-
- setFileNameHeaders(response, editFile);
- // send edits
- TransferFsImage.getFileServer(response.getOutputStream(), editFile,
- getThrottler(conf));
+ serveFile(editFile);
} else if (parsedParams.isPutImage()) {
final long txid = parsedParams.getTxId();
@@ -181,6 +174,28 @@ public class GetImageServlet extends HttpServlet {
}
return null;
}
+
+ private void serveFile(File file) throws IOException {
+ FileInputStream fis = new FileInputStream(file);
+ try {
+ setVerificationHeaders(response, file);
+ setFileNameHeaders(response, file);
+ if (!file.exists()) {
+ // Potential race where the file was deleted while we were in the
+ // process of setting headers!
+ throw new FileNotFoundException(file.toString());
+ // It's possible the file could be deleted after this point, but
+ // we've already opened the 'fis' stream.
+ // It's also possible length could change, but this would be
+ // detected by the client side as an inaccurate length header.
+ }
+ // send file
+ TransferFsImage.getFileServer(response, file, fis,
+ getThrottler(conf));
+ } finally {
+ IOUtils.closeStream(fis);
+ }
+ }
});
} catch (Throwable t) {
@@ -192,7 +207,7 @@ public class GetImageServlet extends HttpServlet {
}
}
- private static void setFileNameHeaders(HttpServletResponse response,
+ public static void setFileNameHeaders(HttpServletResponse response,
File file) {
response.setHeader(CONTENT_DISPOSITION, "attachment; filename=" +
file.getName());
@@ -204,7 +219,7 @@ public class GetImageServlet extends HttpServlet {
* @param conf configuration
* @return a data transfer throttler
*/
- private final DataTransferThrottler getThrottler(Configuration conf) {
+ public final static DataTransferThrottler getThrottler(Configuration conf) {
long transferBandwidth =
conf.getLong(DFSConfigKeys.DFS_IMAGE_TRANSFER_RATE_KEY,
DFSConfigKeys.DFS_IMAGE_TRANSFER_RATE_DEFAULT);
@@ -262,7 +277,7 @@ public class GetImageServlet extends HttpServlet {
* Set headers for content length, and, if available, md5.
* @throws IOException
*/
- private void setVerificationHeaders(HttpServletResponse response, File file)
+ public static void setVerificationHeaders(HttpServletResponse response, File file)
throws IOException {
response.setHeader(TransferFsImage.CONTENT_LENGTH,
String.valueOf(file.length()));
@@ -335,7 +350,7 @@ public class GetImageServlet extends HttpServlet {
if (key.equals("getimage")) {
isGetImage = true;
try {
- txId = parseLongParam(request, TXID_PARAM);
+ txId = ServletUtil.parseLongParam(request, TXID_PARAM);
} catch (NumberFormatException nfe) {
if (request.getParameter(TXID_PARAM).equals(LATEST_FSIMAGE_VALUE)) {
fetchLatest = true;
@@ -345,11 +360,11 @@ public class GetImageServlet extends HttpServlet {
}
} else if (key.equals("getedit")) {
isGetEdit = true;
- startTxId = parseLongParam(request, START_TXID_PARAM);
- endTxId = parseLongParam(request, END_TXID_PARAM);
+ startTxId = ServletUtil.parseLongParam(request, START_TXID_PARAM);
+ endTxId = ServletUtil.parseLongParam(request, END_TXID_PARAM);
} else if (key.equals("putimage")) {
isPutImage = true;
- txId = parseLongParam(request, TXID_PARAM);
+ txId = ServletUtil.parseLongParam(request, TXID_PARAM);
} else if (key.equals("port")) {
remoteport = new Integer(val[0]).intValue();
} else if (key.equals(STORAGEINFO_PARAM)) {
@@ -405,16 +420,5 @@ public class GetImageServlet extends HttpServlet {
return fetchLatest;
}
- private static long parseLongParam(HttpServletRequest request, String param)
- throws IOException {
- // Parse the 'txid' parameter which indicates which image is to be
- // fetched.
- String paramStr = request.getParameter(param);
- if (paramStr == null) {
- throw new IOException("Invalid request has no " + param + " parameter");
- }
-
- return Long.valueOf(paramStr);
- }
}
}
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/TransferFsImage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/TransferFsImage.java
index 492cf28fbe7..cc477084387 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/TransferFsImage.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/TransferFsImage.java
@@ -25,6 +25,8 @@ import java.util.ArrayList;
import java.util.List;
import java.lang.Math;
+import javax.servlet.ServletOutputStream;
+import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.logging.Log;
@@ -145,16 +147,16 @@ public class TransferFsImage {
* A server-side method to respond to a getfile http request
* Copies the contents of the local file into the output stream.
*/
- static void getFileServer(OutputStream outstream, File localfile,
+ public static void getFileServer(ServletResponse response, File localfile,
+ FileInputStream infile,
DataTransferThrottler throttler)
throws IOException {
byte buf[] = new byte[HdfsConstants.IO_FILE_BUFFER_SIZE];
- FileInputStream infile = null;
+ ServletOutputStream out = null;
try {
- infile = new FileInputStream(localfile);
CheckpointFaultInjector.getInstance()
.aboutToSendFile(localfile);
-
+ out = response.getOutputStream();
if (CheckpointFaultInjector.getInstance().
shouldSendShortFile(localfile)) {
@@ -178,14 +180,14 @@ public class TransferFsImage {
buf[0]++;
}
- outstream.write(buf, 0, num);
+ out.write(buf, 0, num);
if (throttler != null) {
throttler.throttle(num);
}
}
} finally {
- if (infile != null) {
- infile.close();
+ if (out != null) {
+ out.close();
}
}
}