HDFS-3574. Fix small race and do some cleanup in GetImageServlet. Contributed by Todd Lipcon.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1356939 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Todd Lipcon 2012-07-03 20:55:29 +00:00
parent b8389e4c73
commit 3728d16160
4 changed files with 58 additions and 36 deletions

View File

@ -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 = "<hr />\n"
+ "<a href='http://hadoop.apache.org/core'>Hadoop</a>, "

View File

@ -397,6 +397,8 @@ Branch-2 ( Unreleased changes )
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)

View File

@ -46,8 +46,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;
@ -126,23 +128,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();
@ -182,6 +175,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) {
@ -193,7 +208,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());
@ -205,7 +220,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);
@ -263,7 +278,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()));
@ -336,7 +351,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;
@ -346,11 +361,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)) {
@ -406,16 +421,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);
}
}
}

View File

@ -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;
@ -147,16 +149,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)) {
@ -180,14 +182,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();
}
}
}