You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2022/02/07 23:08:50 UTC

[impala] branch master updated: IMPALA-11050: Skip filemetadata reloading in processing AlterPartition event from event processor

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

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new 85c13b3  IMPALA-11050: Skip filemetadata reloading in processing AlterPartition event from event processor
85c13b3 is described below

commit 85c13b3e6fd9f3e6195de05b3a84c6c38bbb37b8
Author: Sourabh Goyal <so...@cloudera.com>
AuthorDate: Fri Dec 10 04:48:14 2021 -0800

    IMPALA-11050: Skip filemetadata reloading in processing AlterPartition
    event from event processor
    
    HdfsPartition in catalogD is a collection of files and each file is
    represented by a FileDescriptor. Currently when processing an
    AlterPartitionEvent, EventProcessor reloads partition metadata along
    with its file metadata. File metadata reloading is an expensive
    operation as it involves listing files in the underlying filesystem.
    
    This patch makes an improvement in alter partition event processing
    for external tables. The proposed solution is to compare storage
    descriptor of HMS partition object received from AlterPartition event
    with sd of already cached HdfsPartition. Reload file metadata if any
    of the following conditions hold true:
    
    * HdfsPartition's sd does not match sd of HMS partition object
    * Partition does not exist in catalogd cache but is present in HMS
    * Cache directive is present for HdfsPartition. File metadata reloading
      is required in this case to fetch latest file blocks info.
    
    Testing:
    * Added new unit tests
    
    Change-Id: I238b169f7f1122c62cbeb1434dbb675629a1e5f2
    Reviewed-on: http://gerrit.cloudera.org:8080/18083
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
---
 .../impala/catalog/FileMetadataLoadOpts.java       |  42 ++++++
 .../java/org/apache/impala/catalog/HdfsTable.java  |  67 +++++++++-
 .../impala/catalog/events/MetastoreEvents.java     |  69 +++++++---
 .../apache/impala/service/CatalogOpExecutor.java   |  16 ++-
 .../events/MetastoreEventsProcessorTest.java       | 147 +++++++++++++++++++++
 5 files changed, 313 insertions(+), 28 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/FileMetadataLoadOpts.java b/fe/src/main/java/org/apache/impala/catalog/FileMetadataLoadOpts.java
new file mode 100644
index 0000000..6448dba
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/FileMetadataLoadOpts.java
@@ -0,0 +1,42 @@
+// 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.catalog;
+
+/**
+ * This class describes various options for file metadata reloading. It is
+ * used when a partition metadata is being reloaded.
+ * FORCE_LOAD option triggers file metadata loading in every case which can have
+ * performance implications but will always ensure correctness
+ * Other options i.e NO_LOAD, LOAD_IF_SD_CHANGED should only be used if
+ * the user is certain that skipping of file metadata reloading
+ * (for some or all partitions) won't cause any correctness issues
+ */
+public enum FileMetadataLoadOpts {
+  NO_LOAD("do not load"),
+  FORCE_LOAD("force load"),
+  LOAD_IF_SD_CHANGED("load only if storage descriptor changed");
+
+  private String description;
+  FileMetadataLoadOpts(String d) {
+    this.description = d;
+  }
+
+  public String getDescription() {
+    return this.description;
+  }
+}
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 e595e25..2ee1c3e 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -2702,10 +2702,23 @@ public class HdfsTable extends Table implements FeFsTable {
 
   /**
    * Reloads the HdfsPartitions which correspond to the given partNames. Returns the
-   * number of partitions which were reloaded.
+   * number of partitions which were reloaded. This method also reloads file metadata
+   * of all the partitions for the given partNames
    */
   public int reloadPartitionsFromNames(IMetaStoreClient client,
       List<String> partNames, String reason) throws CatalogException {
+    return reloadPartitionsFromNames(client, partNames, reason,
+        FileMetadataLoadOpts.FORCE_LOAD);
+  }
+
+  /**
+   * Reloads the HdfsPartitions which correspond to the given partNames. Returns the
+   * number of partitions which were reloaded.
+   * fileMetadataLoadOpts: decides how to reload file metadata for the partitions
+   */
+  public int reloadPartitionsFromNames(IMetaStoreClient client,
+      List<String> partNames, String reason, FileMetadataLoadOpts fileMetadataLoadOpts)
+      throws CatalogException {
     Preconditions.checkState(partNames != null && !partNames.isEmpty());
     LOG.info(String.format("Reloading partition metadata: %s %s (%s)",
         getFullName(), generateDebugStr(partNames, 3), reason));
@@ -2721,7 +2734,7 @@ public class HdfsTable extends Table implements FeFsTable {
           hmsPartToHdfsPart.put(partition, hdfsPartition);
         }
       }
-      reloadPartitions(client, hmsPartToHdfsPart, true);
+      reloadPartitions(client, hmsPartToHdfsPart, fileMetadataLoadOpts);
       return hmsPartToHdfsPart.size();
     } catch (NoSuchObjectException e) {
       // HMS throws a NoSuchObjectException if the table does not exist
@@ -2781,15 +2794,38 @@ public class HdfsTable extends Table implements FeFsTable {
    *                            reconstructed from the HMS partition key. If the
    *                            value for a given partition key is null then nothing is
    *                            removed and a new HdfsPartition is simply added.
-   * @param loadFileMetadata If true, file metadata will be reloaded.
+   * @param loadFileMetadata If true, file metadata for all hmsPartsToHdfsParts will be
+   *                        reloaded.
    */
   public void reloadPartitions(IMetaStoreClient client,
       Map<Partition, HdfsPartition> hmsPartsToHdfsParts, boolean loadFileMetadata)
       throws CatalogException {
+    if (loadFileMetadata) {
+      reloadPartitions(client, hmsPartsToHdfsParts, FileMetadataLoadOpts.FORCE_LOAD);
+    } else {
+      reloadPartitions(client, hmsPartsToHdfsParts, FileMetadataLoadOpts.NO_LOAD);
+    }
+  }
+
+  /**
+   * Reloads the metadata of partitions given by a map of HMS Partitions to existing (old)
+   * HdfsPartitions.
+   * @param hmsPartsToHdfsParts The map of HMS partition object to the old HdfsPartition.
+   *                            Every key-value in this map represents the HdfsPartition
+   *                            which needs to be removed from the table and
+   *                            reconstructed from the HMS partition key. If the
+   *                            value for a given partition key is null then nothing is
+   *                            removed and a new HdfsPartition is simply added.
+   * @param fileMetadataLoadOpts describes how to load file metadata
+   */
+  public void reloadPartitions(IMetaStoreClient client,
+      Map<Partition, HdfsPartition> hmsPartsToHdfsParts,
+      FileMetadataLoadOpts fileMetadataLoadOpts) throws CatalogException {
     Preconditions.checkState(isWriteLockedByCurrentThread(), "Write Lock should be "
         + "held before reloadPartitions");
     FsPermissionCache permissionCache = new FsPermissionCache();
     Map<HdfsPartition.Builder, HdfsPartition> partBuilderToPartitions = new HashMap<>();
+    Set<HdfsPartition.Builder> partBuildersFileMetadataRefresh = new HashSet<>();
     for (Map.Entry<Partition, HdfsPartition> entry : hmsPartsToHdfsParts.entrySet()) {
       Partition hmsPartition = entry.getKey();
       HdfsPartition oldPartition = entry.getValue();
@@ -2801,11 +2837,32 @@ public class HdfsTable extends Table implements FeFsTable {
       if (oldPartition != null) {
         partBuilder.setFileDescriptors(oldPartition);
       }
+      switch (fileMetadataLoadOpts) {
+        case FORCE_LOAD:
+          partBuildersFileMetadataRefresh.add(partBuilder);
+          break;
+        case LOAD_IF_SD_CHANGED:
+          if (oldPartition == null || !oldPartition.compareSd(hmsPartition.getSd()) ||
+              partBuilder.isMarkedCached()) {
+            partBuildersFileMetadataRefresh.add(partBuilder);
+          }
+          break;
+        case NO_LOAD:
+          // do nothing
+          break;
+        default:
+          throw new CatalogException("Invalid filemetadataOpts: " +
+              fileMetadataLoadOpts.name() + " in reloadPartitions()");
+      }
       partBuilderToPartitions.put(partBuilder, oldPartition);
     }
-    if (loadFileMetadata) {
+    if (!partBuildersFileMetadataRefresh.isEmpty()) {
+      LOG.info("for table {}, file metadataOps: {}, refreshing file metadata for {}"
+              + " out of {} partitions to reload in reloadPartitions()", getFullName(),
+          fileMetadataLoadOpts.name(), partBuildersFileMetadataRefresh.size(),
+          partBuilderToPartitions.size());
       // load file metadata in parallel
-      loadFileMetadataForPartitions(client, partBuilderToPartitions.keySet(),
+      loadFileMetadataForPartitions(client, partBuildersFileMetadataRefresh,
           /*isRefresh=*/true);
     }
     for (Map.Entry<HdfsPartition.Builder, HdfsPartition> entry :
diff --git a/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java b/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
index 1d56c81..1353809 100644
--- a/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
+++ b/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
@@ -59,6 +59,7 @@ import org.apache.impala.catalog.CatalogException;
 import org.apache.impala.catalog.CatalogServiceCatalog;
 import org.apache.impala.catalog.DatabaseNotFoundException;
 import org.apache.impala.catalog.Db;
+import org.apache.impala.catalog.FileMetadataLoadOpts;
 import org.apache.impala.catalog.HdfsTable;
 import org.apache.impala.catalog.MetaStoreClientPool;
 import org.apache.impala.catalog.IncompleteTable;
@@ -858,21 +859,41 @@ public class MetastoreEvents {
      * Reloads the partitions provided, only if the table is loaded and if the partitions
      * exist in catalogd.
      * @param partitions the list of Partition objects which need to be reloaded.
-     * @param loadFileMetadata
+     * @param fileMetadataLoadOpts: describes how to reload file metadata for partitions
      * @param reason The reason for reload operation which is used for logging by
      *               catalogd.
      */
-    protected void reloadPartitions(List<Partition> partitions, boolean loadFromEvent,
-        boolean loadFileMetadata, String reason) throws CatalogException {
+    protected void reloadPartitions(List<Partition> partitions,
+        FileMetadataLoadOpts fileMetadataLoadOpts, String reason)
+        throws CatalogException {
       try {
-        int numPartsRefreshed;
-        if (loadFromEvent) {
-          numPartsRefreshed = catalogOpExecutor_.reloadPartitionsFromEvent(getEventId(),
-              dbName_, tblName_, partitions, loadFileMetadata, reason);
-        } else {
-          numPartsRefreshed = catalogOpExecutor_.reloadPartitionsIfExist(getEventId(),
-              dbName_, tblName_, partitions, reason);
+        int numPartsRefreshed = catalogOpExecutor_.reloadPartitionsIfExist(getEventId(),
+            dbName_, tblName_, partitions, reason, fileMetadataLoadOpts);
+        if (numPartsRefreshed > 0) {
+          metrics_.getCounter(MetastoreEventsProcessor.NUMBER_OF_PARTITION_REFRESHES)
+              .inc(numPartsRefreshed);
         }
+      } catch (TableNotLoadedException e) {
+        debugLog("Ignoring the event since table {} is not loaded",
+            getFullyQualifiedTblName());
+      } catch (DatabaseNotFoundException | TableNotFoundException e) {
+        debugLog("Ignoring the event since table {} is not found",
+            getFullyQualifiedTblName());
+      }
+    }
+
+    /**
+     * Reloads partitions from the event if they exist. Does not fetch those partitions
+     * from HMS. Does NOT reload file metadata
+     * @param partitions: Partitions to be reloaded
+     * @param reason: Reason for reloading. Mainly used for logging in catalogD
+     * @throws CatalogException
+     */
+    protected void reloadPartitionsFromEvent(List<Partition> partitions, String reason)
+        throws CatalogException {
+      try {
+        int numPartsRefreshed = catalogOpExecutor_.reloadPartitionsFromEvent(getEventId(),
+            dbName_, tblName_, partitions, reason);
         if (numPartsRefreshed > 0) {
           metrics_.getCounter(MetastoreEventsProcessor.NUMBER_OF_PARTITION_REFRESHES)
               .inc(numPartsRefreshed);
@@ -886,6 +907,7 @@ public class MetastoreEvents {
       }
     }
 
+
     /**
      * To decide whether to skip processing this event, fetch table from cache
      * and compare the last synced event id of cache table with this event id.
@@ -1259,7 +1281,10 @@ public class MetastoreEvents {
         // Ignore event if table or database is not in catalog. Throw exception if
         // refresh fails. If the partition does not exist in metastore the reload
         // method below removes it from the catalog
-        reloadPartitions(Arrays.asList(insertPartition_), false, true, "INSERT event");
+        // forcing file metadata reload so that new files (due to insert) are reflected
+        // HdfsPartition
+        reloadPartitions(Arrays.asList(insertPartition_),
+            FileMetadataLoadOpts.FORCE_LOAD, "INSERT event");
       } catch (CatalogException e) {
         throw new MetastoreNotificationNeedsInvalidateException(debugString("Refresh "
                 + "partition on table {} partition {} failed. Event processing cannot "
@@ -2021,8 +2046,10 @@ public class MetastoreEvents {
         List<TPartitionKeyValue> tPartSpec = getTPartitionSpecFromHmsPartition(msTbl_,
             partitionAfter_);
         try {
-          reloadPartitions(Arrays.asList(partitionAfter_), false, true,
-              "ALTER_PARTITION event");
+          // load file metadata only if storage descriptor of partitionAfter_ differs
+          // from sd of HdfsPartition
+          reloadPartitions(Arrays.asList(partitionAfter_),
+              FileMetadataLoadOpts.LOAD_IF_SD_CHANGED, "ALTER_PARTITION event");
         } catch (CatalogException e) {
           throw new MetastoreNotificationNeedsInvalidateException(debugString("Refresh "
                   + "partition on table {} partition {} failed. Event processing cannot "
@@ -2059,8 +2086,8 @@ public class MetastoreEvents {
       boolean incrementalRefresh =
           BackendConfig.INSTANCE.getHMSEventIncrementalRefreshTransactionalTable();
       if (incrementalRefresh) {
-        reloadPartitions(Collections.singletonList(partitionAfter_), true, false,
-            "ALTER_PARTITION");
+        reloadPartitionsFromEvent(Collections.singletonList(partitionAfter_),
+            "ALTER_PARTITION EVENT FOR TRANSACTIONAL TABLE");
       } else {
         reloadTableFromCatalog("ALTER_PARTITION", true);
       }
@@ -2157,7 +2184,17 @@ public class MetastoreEvents {
           partitions.add(event.getPartitionForBatching());
         }
         try {
-          reloadPartitions(partitions, false, true, getEventType().toString() + " event");
+          if (baseEvent_ instanceof InsertEvent) {
+            // for insert event, always reload file metadata so that new files
+            // are reflected in HdfsPartition
+            reloadPartitions(partitions, FileMetadataLoadOpts.FORCE_LOAD,
+                getEventType().toString() + " event");
+          } else {
+            // alter partition event. Reload file metadata of only those partitions
+            // for which sd has changed
+            reloadPartitions(partitions, FileMetadataLoadOpts.LOAD_IF_SD_CHANGED,
+                getEventType().toString() + " event");
+          }
         } catch (CatalogException e) {
           throw new MetastoreNotificationNeedsInvalidateException(String.format(
               "Refresh partitions on table %s failed when processing event ids %s-%s. "
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 20d743a..709bc47 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -29,6 +29,7 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import java.io.ByteArrayOutputStream;
+import java.io.File;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.util.ArrayList;
@@ -99,6 +100,7 @@ import org.apache.impala.catalog.DataSource;
 import org.apache.impala.catalog.DatabaseNotFoundException;
 import org.apache.impala.catalog.Db;
 import org.apache.impala.catalog.FeCatalogUtils;
+import org.apache.impala.catalog.FileMetadataLoadOpts;
 import org.apache.impala.catalog.IcebergTable;
 import org.apache.impala.catalog.FeFsPartition;
 import org.apache.impala.catalog.FeFsTable;
@@ -141,7 +143,6 @@ import org.apache.impala.common.ImpalaException;
 import org.apache.impala.common.ImpalaRuntimeException;
 import org.apache.impala.common.InternalException;
 import org.apache.impala.common.JniUtil;
-import org.apache.impala.common.Metrics;
 import org.apache.impala.common.Pair;
 import org.apache.impala.common.Reference;
 import org.apache.impala.common.TransactionException;
@@ -4240,11 +4241,13 @@ public class CatalogOpExecutor {
    * @param partsFromEvent List of {@link Partition} objects from the events to be
    *                       reloaded.
    * @param reason Reason for reloading the partitions for logging purposes.
+   * @param fileMetadataLoadOpts describes how to reload file metadata for partsFromEvent
    * @return the number of partitions which were reloaded. If the table does not exist,
    * returns 0. Some partitions could be skipped if they don't exist anymore.
    */
   public int reloadPartitionsIfExist(long eventId, String dbName, String tblName,
-      List<Partition> partsFromEvent, String reason) throws CatalogException {
+      List<Partition> partsFromEvent, String reason,
+      FileMetadataLoadOpts fileMetadataLoadOpts) throws CatalogException {
     Table table = catalog_.getTable(dbName, tblName);
     if (table == null) {
       DeleteEventLog deleteEventLog = catalog_.getMetastoreEventProcessor()
@@ -4294,7 +4297,7 @@ public class CatalogOpExecutor {
       int numOfPartsReloaded;
       try (MetaStoreClient metaStoreClient = catalog_.getMetaStoreClient()) {
         numOfPartsReloaded = hdfsTable.reloadPartitionsFromNames(
-            metaStoreClient.getHiveClient(), partNames, reason);
+            metaStoreClient.getHiveClient(), partNames, reason, fileMetadataLoadOpts);
       }
       hdfsTable.setCatalogVersion(newCatalogVersion);
       return numOfPartsReloaded;
@@ -4321,6 +4324,7 @@ public class CatalogOpExecutor {
   /**
    * Reloads the given partitions if they exist and have not been removed since the event
    * was generated. We don't retrieve partitions from HMS but use partitions from event.
+   * This api does NOT reload file metadata when reloading partitions
    *
    * @param eventId EventId being processed.
    * @param dbName Database name for the partition
@@ -4328,13 +4332,11 @@ public class CatalogOpExecutor {
    * @param partsFromEvent List of {@link Partition} objects from the events to be
    *                       reloaded.
    * @param reason Reason for reloading the partitions for logging purposes.
-   * @param loadFileMetadata If true, reload file metadata. Otherwise, just reload
-   *                         partitions metadata.
    * @return the number of partitions which were reloaded. If the table does not exist,
    * returns 0. Some partitions could be skipped if they don't exist anymore.
    */
   public int reloadPartitionsFromEvent(long eventId, String dbName, String tblName,
-      List<Partition> partsFromEvent, boolean loadFileMetadata, String reason)
+      List<Partition> partsFromEvent, String reason)
       throws CatalogException {
     Table table = catalog_.getTable(dbName, tblName);
     if (table == null) {
@@ -4367,7 +4369,7 @@ public class CatalogOpExecutor {
       int numOfPartsReloaded;
       try (MetaStoreClient metaStoreClient = catalog_.getMetaStoreClient()) {
         numOfPartsReloaded = hdfsTable.reloadPartitionsFromEvent(
-            metaStoreClient.getHiveClient(), partsFromEvent, loadFileMetadata, reason);
+            metaStoreClient.getHiveClient(), partsFromEvent, false, reason);
       }
       hdfsTable.setCatalogVersion(newCatalogVersion);
       return numOfPartsReloaded;
diff --git a/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java b/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
index 42f3f4c..b3148d1 100644
--- a/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
@@ -97,6 +97,7 @@ import org.apache.impala.catalog.events.MetastoreEvents.MetastoreEventFactory;
 import org.apache.impala.catalog.events.MetastoreEvents.MetastoreEventPropertyKey;
 import org.apache.impala.catalog.events.MetastoreEvents.MetastoreEventType;
 import org.apache.impala.catalog.events.MetastoreEventsProcessor.EventProcessorStatus;
+import org.apache.impala.catalog.FileMetadataLoadOpts;
 import org.apache.impala.common.FileSystemUtil;
 import org.apache.impala.common.ImpalaException;
 import org.apache.impala.common.Pair;
@@ -2714,6 +2715,12 @@ public class MetastoreEventsProcessorTest {
     }
   }
 
+  /**
+   * The test asserts that file metadata is not reloaded when processing
+   * alter partition event for a transactional table. Because for such tables,
+   * file metadata reloading is done during commit txn event processing
+   * @throws Exception
+   */
   @Test
   public void testAlterPartitionNotReloadFMD() throws Exception {
     // Turn on incremental refresh for transactional table
@@ -2773,6 +2780,146 @@ public class MetastoreEventsProcessorTest {
     }
   }
 
+  /**
+   * For an external table, the test asserts file metadata is not reloaded during
+   * AlterPartitionEvent processing if only partition parameters are changed
+   * @throws Exception
+   */
+  @Test
+  public void testAlterPartitionNoFileMetadataReload() throws Exception {
+    createDatabase(TEST_DB_NAME, null);
+    final String testTblName = "testAlterPartitionNoFileMetadataReload";
+    createTable(testTblName, true);
+    eventsProcessor_.processEvents();
+    // simulate the table being loaded by explicitly calling load table
+    loadTable(testTblName);
+    List<List<String>> partVals = new ArrayList<>();
+
+    // create 1 partition
+    partVals.add(Arrays.asList("1"));
+    addPartitions(TEST_DB_NAME, testTblName, partVals);
+
+    eventsProcessor_.processEvents();
+    assertEquals("Unexpected number of partitions fetched for the loaded table", 1,
+        ((HdfsTable) catalog_.getTable(TEST_DB_NAME, testTblName))
+            .getPartitions()
+            .size());
+    HdfsTable tbl = (HdfsTable) catalog_.getTable(TEST_DB_NAME, testTblName);
+    // get file metadata load counter before altering the partition
+    long fileMetadataLoadBefore =
+        tbl.getMetrics().getCounter(HdfsTable.NUM_LOAD_FILEMETADATA_METRIC).getCount();
+    // issue alter partition ops
+    partVals.clear();
+    partVals.add(Arrays.asList("1"));
+    String testKey = "randomDummyKey1";
+    String testVal = "randomDummyVal1";
+    alterPartitionsParams(TEST_DB_NAME, testTblName, testKey, testVal,
+        partVals);
+    eventsProcessor_.processEvents();
+
+    tbl = (HdfsTable) catalog_.getTable(TEST_DB_NAME, testTblName);
+    long fileMetadataLoadAfter =
+        tbl.getMetrics().getCounter(HdfsTable.NUM_LOAD_FILEMETADATA_METRIC).getCount();
+    assertEquals(fileMetadataLoadAfter, fileMetadataLoadBefore);
+
+    Collection<? extends FeFsPartition> parts =
+        FeCatalogUtils.loadAllPartitions((HdfsTable)
+            catalog_.getTable(TEST_DB_NAME, testTblName));
+    FeFsPartition singlePartition =
+        Iterables.getOnlyElement(parts);
+    String val = singlePartition.getParameters().getOrDefault(testKey, null);
+    assertNotNull("Expected " + testKey + " to be present in partition parameters",
+        val);
+    assertEquals(testVal, val);
+  }
+
+  /*
+  For an external table, the test asserts that file metadata is reloaded for an alter
+   partition event if the storage descriptor of partition to be altered has changed or
+  partition does not exist in cache but is present in HMS
+   */
+  @Test
+  public void testAlterPartitionFileMetadataReload() throws Exception {
+    createDatabase(TEST_DB_NAME, null);
+    final String testTblName = "testAlterPartitionFileMetadataReload";
+    createTable(testTblName, true);
+    // sync to latest event id
+    eventsProcessor_.processEvents();
+
+    // simulate the table being loaded by explicitly calling load table
+    loadTable(testTblName);
+    List<List<String>> partVals = new ArrayList<>();
+
+    // create 1 partition
+    partVals.add(Arrays.asList("1"));
+    addPartitions(TEST_DB_NAME, testTblName, partVals);
+
+    eventsProcessor_.processEvents();
+    assertEquals("Unexpected number of partitions fetched for the loaded table", 1,
+        ((HdfsTable) catalog_.getTable(TEST_DB_NAME, testTblName))
+            .getPartitions()
+            .size());
+    // HdfsPartitionSdCompareTest already covers all the ways in which storage
+    // descriptor can differ. Here we will only alter sd location to test that
+    // file metadata is reloaded.
+    HdfsTable tbl = (HdfsTable) catalog_.getTable(TEST_DB_NAME, testTblName);
+    long fileMetadataLoadBefore =
+        tbl.getMetrics().getCounter(HdfsTable.NUM_LOAD_FILEMETADATA_METRIC).getCount();
+
+    partVals.clear();
+    partVals.add(Arrays.asList("1"));
+    String newLocation = "/path/to/new_location/";
+    alterPartitions(testTblName, partVals, newLocation);
+    eventsProcessor_.processEvents();
+    // not asserting new location check since it is already done in previous tests
+    long fileMetadataLoadAfter =
+        tbl.getMetrics().getCounter(HdfsTable.NUM_LOAD_FILEMETADATA_METRIC).getCount();
+    assertEquals(fileMetadataLoadBefore + 1, fileMetadataLoadAfter);
+    /*
+     Drop a partition only from cache not from HMS. In such case,
+     the expected behavior is that tbl.reloadPartitions() for such
+     partition should *not* skip file metadata reload even if
+     FileMetadataLoadOpts is LOAD_IF_SD_CHANGED. The following code asserts
+     this behaviour
+     */
+    String partName = FeCatalogUtils.getPartitionName(tbl, partVals.get(0));
+    List<HdfsPartition> partitionsToDrop =
+        tbl.getPartitionsForNames(Arrays.asList(partName));
+    tbl.dropPartitions(partitionsToDrop, false);
+    assertEquals(0, tbl.getPartitions().size());
+
+    fileMetadataLoadBefore =
+        tbl.getMetrics().getCounter(HdfsTable.NUM_LOAD_FILEMETADATA_METRIC).getCount();
+    Partition msPartition = null;
+    try (MetaStoreClient metaStoreClient = catalog_.getMetaStoreClient()) {
+      msPartition = metaStoreClient.getHiveClient().getPartition(TEST_DB_NAME,
+          testTblName, Arrays.asList("1"));
+      Map<Partition, HdfsPartition> mp = new HashMap();
+      mp.put(msPartition, null);
+      // reload the just dropped partition again with LOAD_IF_SD_CHANGED file
+      // metadataOpts. Assert that partition is loaded back in tbl
+      if (!catalog_.tryWriteLock(tbl)) {
+        throw new CatalogException("Couldn't acquire write lock on table: " +
+            tbl.getFullName());
+      }
+      catalog_.getLock().writeLock().unlock();
+      try {
+        tbl.reloadPartitions(metaStoreClient.getHiveClient(), mp,
+            FileMetadataLoadOpts.LOAD_IF_SD_CHANGED);
+      } finally {
+        if (tbl.isWriteLockedByCurrentThread()) {
+          tbl.releaseWriteLock();
+        }
+      }
+    }
+    assertEquals(1, tbl.getPartitionsForNames(Arrays.asList(partName)).size());
+
+    fileMetadataLoadAfter =
+        tbl.getMetrics().getCounter(HdfsTable.NUM_LOAD_FILEMETADATA_METRIC).getCount();
+
+    assertEquals(fileMetadataLoadBefore+1, fileMetadataLoadAfter);
+  }
+
   private abstract class AlterTableExecutor {
     protected abstract void execute() throws Exception;