bgaborg commented on a change in pull request #1691: HADOOP-16424. S3Guard 
fsck: Check internal consistency of the MetadataStore
URL: https://github.com/apache/hadoop/pull/1691#discussion_r350194520
 
 

 ##########
 File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardFsck.java
 ##########
 @@ -396,6 +413,234 @@ public Path getPath() {
     }
   }
 
+  /**
+   * Check the DynamoDB metadatastore internally for consistency.
+   * <pre>
+   * Tasks to do here:
+   *  - find orphan entries (entries without a parent).
+   *  - find if a file's parent is not a directory (so the parent is a file).
+   *  - find entries where the parent is a tombstone.
+   *  - warn: no lastUpdated field.
+   * </pre>
+   */
+  public List<ComparePair> checkDdbInternalConsistency (Path basePath)
+      throws IOException {
+    Preconditions.checkArgument(basePath.isAbsolute(), "path must be 
absolute");
+
+    List<ComparePair> comparePairs = new ArrayList<>();
+    String rootStr = basePath.toString();
+    LOG.info("Root for internal consistency check: {}", rootStr);
+    StopWatch stopwatch = new StopWatch();
+    stopwatch.start();
+
+    final Table table = metadataStore.getTable();
+    final String username = metadataStore.getUsername();
+    DDBTree ddbTree = new DDBTree();
+
+    /*
+     * I. Root node construction
+     * - If the root node is the real bucket root, a node is constructed 
instead of
+     *   doing a query to the ddb because the bucket root is not stored.
+     * - If the root node is not a real bucket root then the entry is queried 
from
+     *   the ddb and constructed from the result.
+     */
+
+    DDBPathMetadata baseMeta;
+
+    if (!basePath.isRoot()) {
+      PrimaryKey rootKey = pathToKey(basePath);
+      final GetItemSpec spec = new GetItemSpec()
+          .withPrimaryKey(rootKey)
+          .withConsistentRead(true);
+      final Item baseItem = table.getItem(spec);
+      baseMeta = itemToPathMetadata(baseItem, username);
+
+      if (baseMeta == null) {
+        throw new FileNotFoundException(
+            "Base element metadata is null. " +
+                "This means the base path element is missing, or wrong path is 
" +
+                "passed as base path to the internal ddb consistency 
checker.");
+      }
+    } else {
+      baseMeta = new DDBPathMetadata(
+          new S3AFileStatus(Tristate.UNKNOWN, basePath, username)
+      );
+    }
+
+    DDBTreeNode root = new DDBTreeNode(baseMeta);
+    ddbTree.addNode(root);
+    ddbTree.setRoot(root);
+
+    /*
+     * II. Build and check the descendant tree:
+     * 1. query all nodes where the prefix is the given root, and put it in 
the tree
+     * 2. Check connectivity: check if each parent is in the hashmap
+     *    - This is done in O(n): we only need to find the parent based on the
+     *      path with a hashmap lookup.
+     *    - Do a test if the graph is connected - if the parent is not in the
+     *      hashmap, we found an orphan entry.
+     *
+     * 3. Do test the elements for errors:
+     *    - File is a parent of a file.
+     *    - Entries where the parent is tombstoned but the entries are not.
+     *    - Warn on no lastUpdated field.
+     *
+     */
+    ExpressionSpecBuilder builder = new ExpressionSpecBuilder();
+    builder.withCondition(
+        ExpressionSpecBuilder.S("parent")
+            .beginsWith(pathToParentKey(basePath))
+    );
+    final IteratorSupport<Item, ScanOutcome> resultIterator = table.scan(
+        builder.buildForScan()).iterator();
+    resultIterator.forEachRemaining(item -> {
+      final DDBPathMetadata pmd = itemToPathMetadata(item, username);
+      DDBTreeNode ddbTreeNode = new DDBTreeNode(pmd);
+      ddbTree.addNode(ddbTreeNode);
+    });
+
+    LOG.debug("Root: {}", ddbTree.getRoot());
+
+    for (Map.Entry<Path, DDBTreeNode> entry : 
ddbTree.getContentMap().entrySet()) {
+      final DDBTreeNode node = entry.getValue();
+      final ComparePair pair = new ComparePair(null, node.val);
+      // let's skip the root node when checking.
+      if (node.getVal().getFileStatus().getPath().isRoot()) {
+        continue;
+      }
+
+      if(node.getVal().getLastUpdated() == 0) {
+        pair.violations.add(Violation.NO_LASTUPDATED_FIELD);
+      }
+
+      // skip further checking the basenode which is not the actual bucket 
root.
+      if (node.equals(ddbTree.getRoot())) {
+        continue;
+      }
+
+      final Path parent = node.getFileStatus().getPath().getParent();
+      final DDBTreeNode parentNode = ddbTree.getContentMap().get(parent);
+      if (parentNode == null) {
+        pair.violations.add(Violation.ORPHAN_DDB_ENTRY);
+      } else {
+        if (!node.isTombstoned() && !parentNode.isDirectory()) {
+          pair.violations.add(Violation.PARENT_IS_A_FILE);
+        }
+        if(!node.isTombstoned() && parentNode.isTombstoned()) {
+          pair.violations.add(Violation.PARENT_TOMBSTONED);
+        }
+      }
+
+      if (!pair.violations.isEmpty()) {
+        comparePairs.add(pair);
+      }
+
+      node.setParent(parentNode);
+    }
+
+    // Create a handler and handle each violated pairs
+    S3GuardFsckViolationHandler handler =
+        new S3GuardFsckViolationHandler(rawFS, metadataStore);
+    comparePairs.forEach(handler::handle);
+
+    stopwatch.stop();
+    LOG.info("Total scan time: {}s", stopwatch.now(TimeUnit.SECONDS));
+    LOG.info("Scanned entries: {}", ddbTree.contentMap.size());
+
+    return comparePairs;
+  }
+
+  /**
+   * DDBTree is the tree that represents the structure of items in the DynamoDB
+   */
+  public static class DDBTree {
+    final Map<Path, DDBTreeNode> contentMap = new HashMap<>();
+    DDBTreeNode root;
+
+    public DDBTree() {
+    }
+
+    public Map<Path, DDBTreeNode> getContentMap() {
+      return contentMap;
+    }
+
+    public DDBTreeNode getRoot() {
+      return root;
+    }
+
+    public void setRoot(DDBTreeNode root) {
+      this.root = root;
+    }
+
+    public void addNode(DDBTreeNode pm) {
+      contentMap.put(pm.getVal().getFileStatus().getPath(), pm);
+    }
+
+    @Override
+    public String toString() {
+      return "DDBTree{" +
+          "contentMap=" + contentMap +
+          ", root=" + root +
+          '}';
+    }
+  }
+
+  /**
+   * Tree node for DDBTree
+   */
+  public static class DDBTreeNode {
+    final DDBPathMetadata val;
+    DDBTreeNode parent;
 
 Review comment:
   parent is not final. it should be set after the construction of the instance.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to