github-actions[bot] commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4056773402
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java:
##########
@@ -502,9 +544,36 @@ public void invalidateTableByEngine(long catalogId, String
engine, String dbName
public void invalidatePartitions(long catalogId,
String dbName, String tableName, List<String> partitions) {
- routeCatalogEngines(catalogId, cache -> safeInvalidate(
- cache, catalogId, "invalidatePartitions",
- () -> cache.invalidatePartitions(catalogId, dbName, tableName,
partitions)));
+ Optional<ExternalDatabase<? extends ExternalTable>> db =
getCachedDb(catalogId, dbName);
+ try {
+ routeCatalogEngines(catalogId, cache -> safeInvalidate(
+ cache, catalogId, "invalidatePartitions",
+ () -> cache.invalidatePartitions(catalogId, dbName,
tableName, partitions)));
Review Comment:
[P1] Fence row counts on the engine-only retry. When Hive catches
ERR_CACHE_INCONSISTENCY, getHivePartitionValues calls invalidateTableByEngine
and retries, but this helper only clears the selected metadata engine. The
independent rowCountCache remains publishable, while Hive's fallback row-count
estimate reads the same partition/file metadata, so a subsequent getRowCount
can return the pre-retry value until TTL. Please fence the table row-count
identity here (or in the retry) and add a regression test.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -168,6 +168,7 @@ public void replayRefreshTable(ExternalObjectLog log) {
}
// See comment in refreshDbInternal for why db and table may be null.
if (!db.isPresent()) {
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(catalog.getId());
LOG.warn("failed to find db when replaying refresh table: {}",
log.debugForRefreshTable());
Review Comment:
[P1] Fence cold REFRESH DATABASE replay too. This new table-replay miss
branch retires catalog/DB row counts, but replayRefreshDb still cache-lookups
the database and only logs/returns when it is absent. Because the DB metadata
cache can be evicted while row-count entries remain, a replayed REFRESH
DATABASE can leave a stale estimate to be served later. Please invalidate the
catalog (or carry/use a canonical DB ID) on the missing-DB branch and add a
cold database-cache replay test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1163,10 +1171,12 @@ public void unregisterDatabase(String dbName) {
if (LOG.isDebugEnabled()) {
LOG.debug("unregister database [{}]", dbName);
}
+ // Resolve the canonical database object before removing it from the
local metadata cache.
Review Comment:
[P2] Keep local DB removal independent of routed cache failures. This new
invalidateDb call runs before metaCache.invalidate so it can resolve the
numeric ID, but safeInvalidate does not catch a runtime failure from an engine
cache. If that happens, control never reaches local removal and the dropped DB
remains visible with stale metadata/replay state. Please capture the canonical
ID/object first and make metadata removal and row-count fencing finally-safe
around engine invalidation.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java:
##########
@@ -1050,6 +1050,7 @@ public void addExternalPartitions(String catalogName,
String dbName, String tabl
}
HiveExternalMetaCache cache =
Env.getCurrentEnv().getExtMetaCacheMgr().hive(catalog.getId());
cache.addPartitionsCache(hmsTable.getOrBuildNameMapping(),
partitionNames, partitionColumnTypes);
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(hmsTable);
hmsTable.setUpdateTime(updateTime);
Review Comment:
[P1] Also fence the unsupported ADD_PARTITION path. getPartitionColumnTypes
can throw NotSupportedException and this catch returns before the new
invalidateRowCountCache call. The metastore partition mutation is already
committed, and AddPartitionEvent treats this return as handled, so a
pre-existing row-count entry can survive indefinitely under the unchanged table
ID. Please fence the table before returning (or in a finally/conservative DB
scope) and add an unsupported-table event regression.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
