diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java index d2676f70e7..590b9c4902 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java @@ -638,22 +638,42 @@ public DirListingMetadata listChildren(final Path path) throws IOException { metas.add(meta); } + // Minor race condition here - if the path is deleted between + // getting the list of items and the directory metadata we might + // get a null in DDBPathMetadata. DDBPathMetadata dirPathMeta = get(path); - boolean isAuthoritative = false; - if(dirPathMeta != null) { - isAuthoritative = dirPathMeta.isAuthoritativeDir(); - } - LOG.trace("Listing table {} in region {} for {} returning {}", - tableName, region, path, metas); - - return (metas.isEmpty() && dirPathMeta == null) - ? null - : new DirListingMetadata(path, metas, isAuthoritative, - dirPathMeta.getLastUpdated()); + return getDirListingMetadataFromDirMetaAndList(path, metas, + dirPathMeta); }); } + DirListingMetadata getDirListingMetadataFromDirMetaAndList(Path path, + List metas, DDBPathMetadata dirPathMeta) { + boolean isAuthoritative = false; + if (dirPathMeta != null) { + isAuthoritative = dirPathMeta.isAuthoritativeDir(); + } + + LOG.trace("Listing table {} in region {} for {} returning {}", + tableName, region, path, metas); + + if (!metas.isEmpty() && dirPathMeta == null) { + // We handle this case as the directory is deleted. + LOG.warn("Directory marker is deleted, but the list of the directory " + + "elements is not empty: {}. This case is handled as if the " + + "directory was deleted.", metas); + return null; + } + + if(metas.isEmpty() && dirPathMeta == null) { + return null; + } + + return new DirListingMetadata(path, metas, isAuthoritative, + dirPathMeta.getLastUpdated()); + } + /** * build the list of all parent entries. * @param pathsToCreate paths to create diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMiscOperations.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMiscOperations.java index 03be39f7df..fc0f8940de 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMiscOperations.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestDynamoDBMiscOperations.java @@ -20,6 +20,7 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.util.List; import com.amazonaws.services.dynamodbv2.model.ResourceNotFoundException; import com.amazonaws.waiters.WaiterTimedOutException; @@ -27,6 +28,10 @@ import org.apache.hadoop.fs.s3a.AWSClientIOException; import org.apache.hadoop.test.HadoopTestBase; +import org.apache.hadoop.fs.Path; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.apache.hadoop.fs.s3a.s3guard.DynamoDBMetadataStore.translateTableWaitFailure; import static org.apache.hadoop.test.LambdaTestUtils.intercept; @@ -91,4 +96,17 @@ public void testTranslateWrappedOtherException() throws Throwable { assertEquals(e, ex.getCause()); } + @Test + public void testInnerListChildrenDirectoryNpe() throws Exception { + DynamoDBMetadataStore ddbms = new DynamoDBMetadataStore(); + Path p = mock(Path.class); + List metas = mock(List.class); + + when(metas.isEmpty()).thenReturn(false); + DDBPathMetadata dirPathMeta = null; + + assertNull("The return value should be null.", + ddbms.getDirListingMetadataFromDirMetaAndList(p, metas, dirPathMeta)); + } + }