[hadoop] branch trunk updated: HADOOP-16186. S3Guard: NPE in DynamoDBMetadataStore.lambda$listChildren.

[st...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

stevel pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/trunk by this push:
     new cfb0186  HADOOP-16186. S3Guard: NPE in DynamoDBMetadataStore.lambda$listChildren.
cfb0186 is described below

commit cfb01869038065defe50ab53d4d1eda4e6cdee33
Author: Gabor Bota <ga...@cloudera.com>
AuthorDate: Thu Mar 28 15:49:56 2019 +0000

    HADOOP-16186. S3Guard: NPE in DynamoDBMetadataStore.lambda$listChildren.
    
    Author:    Gabor Bota
---
 .../fs/s3a/s3guard/DynamoDBMetadataStore.java      | 42 ++++++++++++++++------
 .../fs/s3a/s3guard/TestDynamoDBMiscOperations.java | 18 ++++++++++
 2 files changed, 49 insertions(+), 11 deletions(-)

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 d2676f7..590b9c4 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 class DynamoDBMetadataStore implements MetadataStore,
             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<PathMetadata> 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 03be39f..fc0f894 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 @@ package org.apache.hadoop.fs.s3a.s3guard;
 
 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.junit.Test;
 
 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 class TestDynamoDBMiscOperations extends HadoopTestBase {
     assertEquals(e, ex.getCause());
   }
 
+  @Test
+  public void testInnerListChildrenDirectoryNpe() throws Exception {
+    DynamoDBMetadataStore ddbms = new DynamoDBMetadataStore();
+    Path p = mock(Path.class);
+    List<PathMetadata> metas = mock(List.class);
+
+    when(metas.isEmpty()).thenReturn(false);
+    DDBPathMetadata dirPathMeta = null;
+
+    assertNull("The return value should be null.",
+        ddbms.getDirListingMetadataFromDirMetaAndList(p, metas, dirPathMeta));
+  }
+
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org