You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by to...@apache.org on 2018/08/09 21:48:15 UTC

[9/9] impala git commit: IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded

IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded

Prior to this patch, when a table is first loaded, the catalog iterated
over each of the partition directories and called getFileStatus() on
each, serially, to determine the overall access level of the table.

In some testing, each such call took 1-2ms, so this could add many
seconds to the overall table load time for a table with thousands of
partitions and also add to the NN load.

This patch adds some batch pre-fetching of file status information: for
any parent directory which contains more than one partition, we use the
listStatus() API to fetch the FileStatus objects in bulk.

A new unit test verifies the number of API calls made to the NameNode
during a table load.

Change-Id: I83e5ebc214d6620d165e13f8cc80f8fdda100734
Reviewed-on: http://gerrit.cloudera.org:8080/11027
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/7f9a74ff
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/7f9a74ff
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/7f9a74ff

Branch: refs/heads/master
Commit: 7f9a74ffcaf1818f1f3c9d427557acca21a627da
Parents: 8f9f91f
Author: Todd Lipcon <to...@cloudera.com>
Authored: Wed Jul 18 19:31:58 2018 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Thu Aug 9 21:39:30 2018 +0000

----------------------------------------------------------------------
 .../org/apache/impala/catalog/HdfsTable.java    | 149 ++++++++++++-------
 .../apache/impala/util/FsPermissionCache.java   |  62 ++++++++
 .../apache/impala/util/FsPermissionChecker.java |  14 +-
 .../org/apache/impala/catalog/CatalogTest.java  |  52 +++++++
 4 files changed, 218 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/7f9a74ff/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
index 38d1695..74f84e5 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -78,6 +78,7 @@ import org.apache.impala.thrift.TTableDescriptor;
 import org.apache.impala.thrift.TTableType;
 import org.apache.impala.util.AvroSchemaConverter;
 import org.apache.impala.util.AvroSchemaUtils;
+import org.apache.impala.util.FsPermissionCache;
 import org.apache.impala.util.FsPermissionChecker;
 import org.apache.impala.util.HdfsCachingUtil;
 import org.apache.impala.util.ListMap;
@@ -92,10 +93,12 @@ import com.codahale.metrics.Timer;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.HashMultiset;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Multiset;
 import com.google.common.collect.Sets;
 
 /**
@@ -765,10 +768,10 @@ public class HdfsTable extends Table implements FeFsTable {
     // using createPartition() calls. A single partition path can correspond to multiple
     // partitions.
     HashMap<Path, List<HdfsPartition>> partsByPath = Maps.newHashMap();
+    FsPermissionCache permCache = preloadPermissionsCache(msPartitions);
 
     Path tblLocation = FileSystemUtil.createFullyQualifiedPath(getHdfsBaseDirPath());
-    FileSystem fs = tblLocation.getFileSystem(CONF);
-    accessLevel_ = getAvailableAccessLevel(fs, tblLocation);
+    accessLevel_ = getAvailableAccessLevel(getFullName(), tblLocation, permCache);
 
     if (msTbl.getPartitionKeysSize() == 0) {
       Preconditions.checkArgument(msPartitions == null || msPartitions.isEmpty());
@@ -776,13 +779,14 @@ public class HdfsTable extends Table implements FeFsTable {
       // We model partitions slightly differently to Hive - every file must exist in a
       // partition, so add a single partition with no keys which will get all the
       // files in the table's root directory.
-      HdfsPartition part = createPartition(msTbl.getSd(), null);
+      HdfsPartition part = createPartition(msTbl.getSd(), null, permCache);
       partsByPath.put(tblLocation, Lists.newArrayList(part));
       if (isMarkedCached_) part.markCached();
       addPartition(part);
     } else {
       for (org.apache.hadoop.hive.metastore.api.Partition msPartition: msPartitions) {
-        HdfsPartition partition = createPartition(msPartition.getSd(), msPartition);
+        HdfsPartition partition = createPartition(msPartition.getSd(), msPartition,
+            permCache);
         addPartition(partition);
         // If the partition is null, its HDFS path does not exist, and it was not added
         // to this table's partition list. Skip the partition.
@@ -802,26 +806,6 @@ public class HdfsTable extends Table implements FeFsTable {
   }
 
   /**
-   * Helper method to load the partition file metadata from scratch. This method is
-   * optimized for loading newly added partitions. For refreshing existing partitions
-   * use refreshPartitionFileMetadata(HdfsPartition).
-   */
-  private void resetAndLoadPartitionFileMetadata(HdfsPartition partition) {
-    Path partDir = partition.getLocationPath();
-    try {
-      FileMetadataLoadStats stats =
-          resetAndLoadFileMetadata(partDir, Lists.newArrayList(partition));
-      if (LOG.isDebugEnabled()) {
-        LOG.debug(String.format("Loaded file metadata for %s %s", getFullName(),
-            stats.debugString()));
-      }
-    } catch (Exception e) {
-        LOG.error("Error loading file metadata for path: " + partDir.toString() +
-            ". Partitions file metadata could be partially loaded.", e);
-    }
-  }
-
-  /**
    * Returns the thread pool size to load the file metadata of this table.
    * 'numPaths' is the number of paths for which the file metadata should be loaded.
    *
@@ -911,9 +895,10 @@ public class HdfsTable extends Table implements FeFsTable {
    * from that.
    * Always returns READ_WRITE for S3 and ADLS files.
    */
-  private TAccessLevel getAvailableAccessLevel(FileSystem fs, Path location)
-      throws IOException {
-
+  private static TAccessLevel getAvailableAccessLevel(String tableName,
+      Path location, FsPermissionCache permCache) throws IOException {
+    Preconditions.checkNotNull(location);
+    FileSystem fs = location.getFileSystem(CONF);
     // Avoid calling getPermissions() on file path for S3 files, as that makes a round
     // trip to S3. Also, the S3A connector is currently unable to manage S3 permissions,
     // so for now it is safe to assume that all files(objects) have READ_WRITE
@@ -928,11 +913,9 @@ public class HdfsTable extends Table implements FeFsTable {
     // permissions to hadoop users/groups (HADOOP-14437).
     if (FileSystemUtil.isADLFileSystem(fs)) return TAccessLevel.READ_WRITE;
 
-    FsPermissionChecker permissionChecker = FsPermissionChecker.getInstance();
     while (location != null) {
       try {
-        FsPermissionChecker.Permissions perms =
-            permissionChecker.getPermissions(fs, location);
+        FsPermissionChecker.Permissions perms = permCache.getPermissions(location);
         if (perms.canReadAndWrite()) {
           return TAccessLevel.READ_WRITE;
         } else if (perms.canRead()) {
@@ -946,39 +929,29 @@ public class HdfsTable extends Table implements FeFsTable {
       }
     }
     // Should never get here.
-    Preconditions.checkNotNull(location, "Error: no path ancestor exists");
-    return TAccessLevel.NONE;
+    throw new NullPointerException("Error determining access level for table " +
+        tableName + ": no path ancestor exists for path: " + location);
   }
 
   /**
-   * Creates a new HdfsPartition object to be added to HdfsTable's partition list.
+   * Creates new HdfsPartition objects to be added to HdfsTable's partition list.
    * Partitions may be empty, or may not even exist in the filesystem (a partition's
    * location may have been changed to a new path that is about to be created by an
    * INSERT). Also loads the file metadata for this partition. Returns new partition
    * if successful or null if none was created.
    *
-   * Throws CatalogException if the supplied storage descriptor contains metadata that
-   * Impala can't understand.
-   */
-  public HdfsPartition createAndLoadPartition(
-      org.apache.hadoop.hive.metastore.api.Partition msPartition)
-      throws CatalogException {
-    HdfsPartition hdfsPartition = createPartition(msPartition.getSd(), msPartition);
-    resetAndLoadPartitionFileMetadata(hdfsPartition);
-    return hdfsPartition;
-  }
-
-  /**
-   * Same as createAndLoadPartition() but is optimized for loading file metadata of
-   * newly created HdfsPartitions in parallel.
+   * Throws CatalogException if one of the supplied storage descriptors contains metadata
+   * that Impala can't understand.
    */
   public List<HdfsPartition> createAndLoadPartitions(
       List<org.apache.hadoop.hive.metastore.api.Partition> msPartitions)
       throws CatalogException {
     HashMap<Path, List<HdfsPartition>> partsByPath = Maps.newHashMap();
     List<HdfsPartition> addedParts = Lists.newArrayList();
+    FsPermissionCache permCache = preloadPermissionsCache(msPartitions);
     for (org.apache.hadoop.hive.metastore.api.Partition partition: msPartitions) {
-      HdfsPartition hdfsPartition = createPartition(partition.getSd(), partition);
+      HdfsPartition hdfsPartition = createPartition(partition.getSd(), partition,
+          permCache);
       Preconditions.checkNotNull(hdfsPartition);
       addedParts.add(hdfsPartition);
       Path partitionPath = hdfsPartition.getLocationPath();
@@ -998,8 +971,8 @@ public class HdfsTable extends Table implements FeFsTable {
    * object.
    */
   private HdfsPartition createPartition(StorageDescriptor storageDescriptor,
-      org.apache.hadoop.hive.metastore.api.Partition msPartition)
-      throws CatalogException {
+      org.apache.hadoop.hive.metastore.api.Partition msPartition,
+      FsPermissionCache permCache) throws CatalogException {
     HdfsStorageDescriptor fileFormatDescriptor =
         HdfsStorageDescriptor.fromStorageDescriptor(this.name_, storageDescriptor);
     List<LiteralExpr> keyValues;
@@ -1010,13 +983,14 @@ public class HdfsTable extends Table implements FeFsTable {
     }
     Path partDirPath = new Path(storageDescriptor.getLocation());
     try {
-      FileSystem fs = partDirPath.getFileSystem(CONF);
       if (msPartition != null) {
         HdfsCachingUtil.validateCacheParams(msPartition.getParameters());
       }
+      TAccessLevel accessLevel = getAvailableAccessLevel(getFullName(), partDirPath,
+          permCache);
       HdfsPartition partition =
           new HdfsPartition(this, msPartition, keyValues, fileFormatDescriptor,
-          new ArrayList<FileDescriptor>(), getAvailableAccessLevel(fs, partDirPath));
+          new ArrayList<FileDescriptor>(), accessLevel);
       partition.checkWellFormed();
       // Set the partition's #rows.
       if (msPartition != null && msPartition.getParameters() != null) {
@@ -1273,8 +1247,8 @@ public class HdfsTable extends Table implements FeFsTable {
     hdfsBaseDir_ = msTbl.getSd().getLocation();
     isMarkedCached_ = HdfsCachingUtil.validateCacheParams(msTbl.getParameters());
     Path location = new Path(hdfsBaseDir_);
-    FileSystem fs = location.getFileSystem(CONF);
-    accessLevel_ = getAvailableAccessLevel(fs, location);
+    accessLevel_ = getAvailableAccessLevel(getFullName(), location,
+        new FsPermissionCache());
     setMetaStoreTable(msTbl);
   }
 
@@ -1290,7 +1264,7 @@ public class HdfsTable extends Table implements FeFsTable {
     org.apache.hadoop.hive.metastore.api.Table msTbl = getMetaStoreTable();
     Preconditions.checkNotNull(msTbl);
     setPrototypePartition(msTbl.getSd());
-    HdfsPartition part = createPartition(msTbl.getSd(), null);
+    HdfsPartition part = createPartition(msTbl.getSd(), null, new FsPermissionCache());
     addPartition(part);
     if (isMarkedCached_) part.markCached();
     refreshPartitionFileMetadata(part);
@@ -1577,8 +1551,11 @@ public class HdfsTable extends Table implements FeFsTable {
     msPartitions.addAll(MetaStoreUtil.fetchPartitionsByName(client,
         Lists.newArrayList(partitionNames), db_.getName(), name_));
 
+    FsPermissionCache permCache = preloadPermissionsCache(msPartitions);
+
     for (org.apache.hadoop.hive.metastore.api.Partition msPartition: msPartitions) {
-      HdfsPartition partition = createPartition(msPartition.getSd(), msPartition);
+      HdfsPartition partition = createPartition(msPartition.getSd(), msPartition,
+          permCache);
       addPartition(partition);
       // If the partition is null, its HDFS path does not exist, and it was not added to
       // this table's partition list. Skip the partition.
@@ -1587,6 +1564,64 @@ public class HdfsTable extends Table implements FeFsTable {
     }
   }
 
+  /**
+   * For each of the partitions in 'msPartitions' with a location inside the table's
+   * base directory, attempt to pre-cache the associated file permissions into the
+   * returned cache. This takes advantage of the fact that many partition directories will
+   * be in the same parent directories, and we can bulk fetch the permissions with a
+   * single round trip to the filesystem instead of individually looking up each.
+   */
+  private FsPermissionCache preloadPermissionsCache(List<Partition> msPartitions) {
+    FsPermissionCache permCache = new FsPermissionCache();
+    // Only preload permissions if the number of partitions to be added is
+    // large (3x) relative to the number of existing partitions. This covers
+    // two common cases:
+    //
+    // 1) initial load of a table (no existing partition metadata)
+    // 2) ALTER TABLE RECOVER PARTITIONS after creating a table pointing to
+    // an already-existing partition directory tree
+    //
+    // Without this heuristic, we would end up using a "listStatus" call to
+    // potentially fetch a bunch of irrelevant information about existing
+    // partitions when we only want to know about a small number of newly-added
+    // partitions.
+    if (msPartitions.size() < partitionMap_.size() * 3) return permCache;
+
+    // TODO(todd): when HDFS-13616 (batch listing of multiple directories)
+    // is implemented, we could likely implement this with a single round
+    // trip.
+    Multiset<Path> parentPaths = HashMultiset.create();
+    for (Partition p : msPartitions) {
+      // We only do this optimization for partitions which are within the table's base
+      // directory. Otherwise we risk a case where a user has specified an external
+      // partition location that is in a directory containing a high number of irrelevant
+      // files, and we'll potentially regress performance compared to just looking up
+      // the partition file directly.
+      String loc = p.getSd().getLocation();
+      if (!loc.startsWith(hdfsBaseDir_)) continue;
+      Path parent = new Path(loc).getParent();
+      if (parent == null) continue;
+      parentPaths.add(parent);
+    }
+
+    // For any paths that contain more than one partition, issue a listStatus
+    // and pre-cache the resulting permissions.
+    for (Multiset.Entry<Path> entry : parentPaths.entrySet()) {
+      if (entry.getCount() == 1) continue;
+      Path p = entry.getElement();
+      try {
+        FileSystem fs = p.getFileSystem(CONF);
+        permCache.precacheChildrenOf(fs, p);
+      } catch (IOException ioe) {
+        // If we fail to pre-warm the cache we'll just wait for later when we
+        // try to actually load the individual permissions, at which point
+        // we can handle the issue accordingly.
+        LOG.debug("Unable to bulk-load permissions for parent path: " + p, ioe);
+      }
+    }
+    return permCache;
+  }
+
   @Override
   protected List<String> getColumnNamesWithHmsStats() {
     List<String> ret = Lists.newArrayList();
@@ -2024,7 +2059,7 @@ public class HdfsTable extends Table implements FeFsTable {
   public void reloadPartition(HdfsPartition oldPartition, Partition hmsPartition)
       throws CatalogException {
     HdfsPartition refreshedPartition = createPartition(
-        hmsPartition.getSd(), hmsPartition);
+        hmsPartition.getSd(), hmsPartition, new FsPermissionCache());
     refreshPartitionFileMetadata(refreshedPartition);
     Preconditions.checkArgument(oldPartition == null
         || HdfsPartition.KV_COMPARATOR.compare(oldPartition, refreshedPartition) == 0);

http://git-wip-us.apache.org/repos/asf/impala/blob/7f9a74ff/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java b/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java
new file mode 100644
index 0000000..48e8ecf
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java
@@ -0,0 +1,62 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.impala.util;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.impala.util.FsPermissionChecker.Permissions;
+
+/**
+ * Simple non-thread-safe cache for resolved file permissions. This allows
+ * pre-caching permissions by listing the status of all files within a directory,
+ * and then using that cache to avoid round trips to the FileSystem for later
+ * queries of those paths.
+ */
+public class FsPermissionCache {
+  private static Configuration CONF = new Configuration();
+  private Map<Path, Permissions> cache_ = new HashMap<>();
+
+  public Permissions getPermissions(Path location) throws IOException {
+    Permissions perms = cache_.get(location);
+    if (perms != null) return perms;
+    FsPermissionChecker checker = FsPermissionChecker.getInstance();
+    FileSystem fs = location.getFileSystem(CONF);
+    perms = checker.getPermissions(fs, location);
+    cache_.put(location, perms);
+    return perms;
+  }
+
+  public void precacheChildrenOf(FileSystem fs, Path p)
+      throws FileNotFoundException, IOException {
+    FsPermissionChecker checker = FsPermissionChecker.getInstance();
+    RemoteIterator<FileStatus> iter = fs.listStatusIterator(p);
+    while (iter.hasNext()) {
+      FileStatus status = iter.next();
+      Permissions perms = checker.getPermissions(fs, status);
+      cache_.put(status.getPath(), perms);
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/impala/blob/7f9a74ff/fe/src/main/java/org/apache/impala/util/FsPermissionChecker.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/util/FsPermissionChecker.java b/fe/src/main/java/org/apache/impala/util/FsPermissionChecker.java
index 136c525..db5c555 100644
--- a/fe/src/main/java/org/apache/impala/util/FsPermissionChecker.java
+++ b/fe/src/main/java/org/apache/impala/util/FsPermissionChecker.java
@@ -283,11 +283,21 @@ public class FsPermissionChecker {
   public Permissions getPermissions(FileSystem fs, Path path) throws IOException {
     Preconditions.checkNotNull(fs);
     Preconditions.checkNotNull(path);
+    return getPermissions(fs, fs.getFileStatus(path));
+  }
+
+  /**
+   * Returns a Permissions object for the given FileStatus object. In the common
+   * case that ACLs are not in use, this does not require any additional round-trip
+   * to the FileSystem. This allows batch construction using APIs like
+   * FileSystem.listStatus(...).
+   */
+  public Permissions getPermissions(FileSystem fs, FileStatus fileStatus)
+      throws IOException {
     AclStatus aclStatus = null;
-    FileStatus fileStatus = fs.getFileStatus(path);
     if (fileStatus.getPermission().getAclBit()) {
       try {
-        aclStatus = fs.getAclStatus(path);
+        aclStatus = fs.getAclStatus(fileStatus.getPath());
       } catch (AclException ex) {
         if (LOG.isTraceEnabled()) {
           LOG.trace(

http://git-wip-us.apache.org/repos/asf/impala/blob/7f9a74ff/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
index 856fc95..7ff5054 100644
--- a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -30,14 +31,20 @@ import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.codec.binary.Base64;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.GlobalStorageStatistics;
+import org.apache.hadoop.fs.StorageStatistics;
+import org.apache.hadoop.hdfs.DFSOpsCountStatistics;
 import org.apache.hadoop.hive.metastore.TableType;
 import org.apache.hadoop.hive.metastore.api.ColumnStatisticsData;
 import org.apache.impala.analysis.FunctionName;
 import org.apache.impala.analysis.LiteralExpr;
 import org.apache.impala.analysis.NumericLiteral;
 import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient;
+import org.apache.impala.common.Reference;
 import org.apache.impala.testutil.CatalogServiceTestCatalog;
 import org.apache.impala.thrift.TFunctionBinaryType;
+import org.apache.impala.thrift.TTableName;
 import org.junit.Test;
 
 import com.google.common.base.Strings;
@@ -283,6 +290,51 @@ public class CatalogTest {
         catalog_.getOrLoadTable("functional", "AllTypes"));
   }
 
+  /**
+   * Regression test for IMPALA-7320: we should use batch APIs to fetch
+   * file permissions for partitions.
+   */
+  @Test
+  public void testNumberOfGetFileStatusCalls() throws CatalogException, IOException {
+    // Reset the filesystem statistics and load the table, ensuring that it's
+    // loaded fresh by invalidating it first.
+    GlobalStorageStatistics stats = FileSystem.getGlobalStorageStatistics();
+    stats.reset();
+    catalog_.invalidateTable(new TTableName("functional", "alltypes"),
+        /*tblWasRemoved=*/new Reference<Boolean>(),
+        /*dbWasAdded=*/new Reference<Boolean>());
+
+    HdfsTable table = (HdfsTable)catalog_.getOrLoadTable("functional", "AllTypes");
+    StorageStatistics opsCounts = stats.get(DFSOpsCountStatistics.NAME);
+
+    // We expect:
+    // - one listLocatedStatus() per partition, to get the file info
+    // - one listStatus() for the month=2010/ dir
+    // - one listStatus() for the month=2009/ dir
+    long expectedCalls = table.getPartitionIds().size() + 2;
+    // Due to HDFS-13747, the listStatus calls are incorrectly accounted as
+    // op_list_located_status. So, we'll just add up the two to make our
+    // assertion resilient to this bug.
+    long seenCalls = opsCounts.getLong("op_list_located_status") +
+        opsCounts.getLong("op_list_status");
+    assertEquals(expectedCalls, seenCalls);
+
+    // We expect only one getFileStatus call, for the top-level directory.
+    assertEquals(1L, (long)opsCounts.getLong("op_get_file_status"));
+
+    // Now test REFRESH on the table...
+    stats.reset();
+    catalog_.reloadTable(table);
+
+    // Again, we expect only one getFileStatus call, for the top-level directory.
+    assertEquals(1L, (long)opsCounts.getLong("op_get_file_status"));
+    // REFRESH calls listStatus on each of the partitions, but doesn't re-check
+    // the permissions of the partition directories themselves.
+    assertEquals(table.getPartitionIds().size(),
+        (long)opsCounts.getLong("op_list_status"));
+  }
+
+
   @Test
   public void TestPartitions() throws CatalogException {
     HdfsTable table =