This is an automated email from the ASF dual-hosted git repository. tmarshall pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 4f8a1b9b088e44821c59710c353a13c94f85d305 Author: Anurag Mantripragada <[email protected]> AuthorDate: Mon Oct 7 20:58:49 2019 -0700 IMPALA-9017: Alter table events on dropped table/db should be ignored. Alter table events received on a table/db that does not exist in catalog can be safely ignored. This is because a table not present in the catalog means either: 1. The table was created in HMS but not processed by the events processor beacuase it is already in an error state or, 2. It was dropped/renamed by the same catalog. Prior to this change, alter table events generated by renames would try to add the new table names to the cache irrespective of weather old table was in catalog cache. After this change a drop + create sequence happens only if table before rename exists in catalog. Testing: Added tests for rename + drop table/db sequences to verify events processor does not error out. Change-Id: I534accd704f76772ebe47865eff0fcdc7a033a48 Reviewed-on: http://gerrit.cloudera.org:8080/14385 Reviewed-by: Vihang Karajgaonkar <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- .../impala/catalog/CatalogServiceCatalog.java | 34 ++++++++++------------ .../impala/catalog/events/MetastoreEvents.java | 18 ++++-------- .../events/MetastoreEventsProcessorTest.java | 25 ++++++++++++++++ 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java index 6e40dd0..7798c3e 100644 --- a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java +++ b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java @@ -1957,30 +1957,28 @@ public class CatalogServiceCatalog extends Catalog { } /** - * Renames the table by atomically removing oldTable and adding the newTable. If the - * oldTable is not found this operation becomes a add new table if not exists - * operation. - * - * @return a pair of booleans. The first of the pair is set if the oldTableName was - * found and removed. The second boolean is set if the new table didn't exist before - * and hence was added. + * Renames the table by atomically removing oldTable and adding the newTable. + * @return true if oldTable was removed and newTable was added, false if oldTable or + * it's db are not in catalog. */ - public Pair<Boolean, Boolean> renameOrAddTableIfNotExists(TTableName oldTableName, - TTableName newTableName) - throws CatalogException { - boolean oldTableRemoved = false; - boolean newTableAdded; + public boolean renameTableIfExists(TTableName oldTableName, + TTableName newTableName) { + boolean tableRenamed = false; versionLock_.writeLock().lock(); try { Db db = getDb(oldTableName.db_name); if (db != null) { - // remove the oldTable if it exists - oldTableRemoved = - removeTable(oldTableName.db_name, oldTableName.table_name) != null; + Table existingTable = removeTable(oldTableName.db_name, oldTableName.table_name); + // Add the newTable only if oldTable existed. + if (existingTable != null) { + Table incompleteTable = IncompleteTable.createUninitializedTable(db, + newTableName.getTable_name()); + incompleteTable.setCatalogVersion(incrementAndGetCatalogVersion()); + db.addTable(incompleteTable); + tableRenamed = true; + } } - // add the new tbl if it doesn't exist - newTableAdded = addTableIfNotExists(newTableName.db_name, newTableName.table_name); - return new Pair<>(oldTableRemoved, newTableAdded); + return tableRenamed; } finally { versionLock_.writeLock().unlock(); } 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 bd892a5..9909e9d 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 @@ -983,21 +983,15 @@ public class MetastoreEvents { TTableName newTTableName = new TTableName(tableAfter_.getDbName(), tableAfter_.getTableName()); - // atomically rename the old table to new table - Pair<Boolean, Boolean> result = null; - result = catalog_.renameOrAddTableIfNotExists(oldTTableName, newTTableName); - - // old table was not found. This could be because catalogD is stale and didn't - // have any entry for the oldTable - if (!result.first) { + // Table is renamed only if old table existed in the catalog. Otherwise the event + // is skipped. + if (!catalog_.renameTableIfExists(oldTTableName, newTTableName)) { debugLog("Did not remove old table to rename table {} to {} since " + "it does not exist anymore", qualify(oldTTableName), qualify(newTTableName)); - } - // the new table from the event was not added since it was already present - if (!result.second) { - debugLog("Did not add new table name while renaming table {} to {}", - qualify(oldTTableName), qualify(newTTableName)); + } else { + infoLog("Renamed old table {} to new table {}.", qualify(oldTTableName), + qualify(newTTableName)); } } 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 7056b7d..e25475d 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 @@ -914,6 +914,31 @@ public class MetastoreEventsProcessorTest { .getCounter(MetastoreEventsProcessor.NUMBER_OF_TABLE_INVALIDATES).getCount(); assertEquals("Unexpected number of table invalidates after trivial alter", numberOfInvalidatesBefore + 4, numberOfInvalidatesAfterTrivialAlter); + + // Simulate rename and drop sequence for table/db. + String tblName = "alter_drop_test"; + createTable(tblName, false); + eventsProcessor_.processEvents(); + alterTableRename(tblName, "new_table_1"); + dropTableFromImpala(TEST_DB_NAME, tblName); + eventsProcessor_.processEvents(); + // Alter event generated by a rename on a dropped table should be skipped. + assertEquals(EventProcessorStatus.ACTIVE, eventsProcessor_.getStatus()); + + // Rename from Impala and drop table. + createTable(tblName, false); + eventsProcessor_.processEvents(); + alterTableRenameFromImpala(TEST_DB_NAME, tblName, "new_table_2"); + dropTableFromImpala(TEST_DB_NAME, tblName); + eventsProcessor_.processEvents(); + + createTable(tblName, false); + eventsProcessor_.processEvents(); + alterTableRename(tblName, "new_tbl_3"); + dropDatabaseCascadeFromImpala(TEST_DB_NAME); + eventsProcessor_.processEvents(); + // Alter event generated by a rename on a dropped database should be skipped. + assertEquals(EventProcessorStatus.ACTIVE, eventsProcessor_.getStatus()); } /**
