steveloughran commented on a change in pull request #1892:
URL: https://github.com/apache/hadoop/pull/1892#discussion_r412220807
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java
##########
@@ -441,9 +443,16 @@ public Path getLocalPathForWrite(String pathStr, long size,
int dirNum = ctx.getAndIncrDirNumLastAccessed(randomInc);
while (numDirsSearched < numDirs) {
long capacity = ctx.dirDF[dirNum].getAvailable();
+ if (capacity > maxCapacity) {
+ maxCapacity = capacity;
+ }
if (capacity > size) {
- returnPath =
- createPath(ctx.localDirs[dirNum], pathStr, checkWrite);
+ try {
+ returnPath = createPath(ctx.localDirs[dirNum], pathStr,
+ checkWrite);
+ } catch (Exception e) {
+ errorText = e.getMessage();
Review comment:
1. Log the exception @ debug
2. store the caught exception in a variable alongside errorText
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java
##########
@@ -459,8 +468,13 @@ public Path getLocalPathForWrite(String pathStr, long size,
}
//no path found
- throw new DiskErrorException("Could not find any valid local " +
- "directory for " + pathStr);
+ String newErrorText = "Could not find any valid local directory for " +
+ pathStr + " with requested size " + size +
+ " as the max capacity in any directory is " + maxCapacity;
+ if (errorText != null) {
+ newErrorText = newErrorText + " due to " + errorText;
+ }
+ throw new DiskErrorException(newErrorText);
Review comment:
if any exception had been caught and stored in L484, use it in the
constructor/setCause here. Stack traces are too important to lose.
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java
##########
@@ -532,4 +533,19 @@ public void testGetLocalPathForWriteForInvalidPaths()
throws Exception {
}
}
+ /**
+ * Test to check the LocalDirAllocation for the less space HADOOP-16769.
+ *
+ * @throws Exception
+ */
+ @Test(timeout = 30000)
+ public void testGetLocalPathForWriteForLessSpace() throws Exception {
+ String dir0 = buildBufferDir(ROOT, 0);
+ String dir1 = buildBufferDir(ROOT, 1);
+ conf.set(CONTEXT, dir0 + "," + dir1);
+ LambdaTestUtils.intercept(DiskErrorException.class, "as the max capacity" +
Review comment:
minor nit, move the "as the max" capacity string onto a line on its own'
merge it back to a single string,
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
