IMPALA-9211: Fix adding new table to stale db due to concurrent reset
When adding a new table to the catalog, we first get the db object from
dbCache and then add the table into it. When doing reset(), i.e. global
INVALIDATE METADATA, we replace the whole dbCache with a new one and
load db and table names from HMS. These two operations have race
conflicts so should both be protected by a exclusive lock, i.e. write
lock of version lock.
Currently, CatalogServiceCatalog.addTable() does not get the db object
within the write lock, which may get a stale db object and add new table
into it. This patch moves the operations into the protection of write
lock.
Tests:
- Ran test_concurrent_ddls.py without errors for CreateTable like
IMPALA-9135 found.
- Ran Core tests
Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592
Reviewed-on: http://gerrit.cloudera.org:8080/14820
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
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 74a39e4..2b603f1 100644
--- a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
@@ -1721,21 +1721,22 @@
}
/**
- * Adds a table with the given name to the catalog and returns the new table,
- * loading the metadata if needed.
+ * Adds a table with the given name to the catalog and returns the new table.
*/
- public Table addTable(String dbName, String tblName) {
- Db db = getDb(dbName);
- if (db == null) return null;
- Table incompleteTable = IncompleteTable.createUninitializedTable(db, tblName);
+ public Table addIncompleteTable(String dbName, String tblName) {
versionLock_.writeLock().lock();
try {
+ // IMPALA-9211: get db object after holding the writeLock in case of getting stale
+ // db object due to concurrent INVALIDATE METADATA
+ Db db = getDb(dbName);
+ if (db == null) return null;
+ Table incompleteTable = IncompleteTable.createUninitializedTable(db, tblName);
incompleteTable.setCatalogVersion(incrementAndGetCatalogVersion());
db.addTable(incompleteTable);
+ return db.getTable(tblName);
} finally {
versionLock_.writeLock().unlock();
}
- return db.getTable(tblName);
}
/**
@@ -1973,7 +1974,7 @@
removeTable(oldTableName.getDb_name(), oldTableName.getTable_name());
if (oldTable == null) return Pair.create(null, null);
return Pair.create(oldTable,
- addTable(newTableName.getDb_name(), newTableName.getTable_name()));
+ addIncompleteTable(newTableName.getDb_name(), newTableName.getTable_name()));
} finally {
versionLock_.writeLock().unlock();
}
@@ -2170,7 +2171,7 @@
// Add a new uninitialized table to the table cache, effectively invalidating
// any existing entry. The metadata for the table will be loaded lazily, on the
// on the next access to the table.
- Table newTable = addTable(dbName, tblName);
+ Table newTable = addIncompleteTable(dbName, tblName);
Preconditions.checkNotNull(newTable);
if (loadInBackground_) {
tableLoadingMgr_.backgroundLoad(new TTableName(dbName.toLowerCase(),
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 3fba93b..5689a0d 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -2245,7 +2245,8 @@
}
}
// Add the table to the catalog cache
- Table newTbl = catalog_.addTable(newTable.getDbName(), newTable.getTableName());
+ Table newTbl = catalog_.addIncompleteTable(newTable.getDbName(),
+ newTable.getTableName());
addTableToCatalogUpdate(newTbl, response.result);
}
} catch (Exception e) {
@@ -2339,7 +2340,8 @@
"CREATE TABLE CACHED");
applyAlterTable(newTable);
}
- Table newTbl = catalog_.addTable(newTable.getDbName(), newTable.getTableName());
+ Table newTbl = catalog_.addIncompleteTable(newTable.getDbName(),
+ newTable.getTableName());
addTableToCatalogUpdate(newTbl, response.result);
if (authzConfig_.isEnabled()) {
authzManager_.updateTableOwnerPrivilege(serverName, newTable.getDbName(),
