github-actions[bot] commented on code in PR #68238:
URL: https://github.com/apache/doris/pull/68238#discussion_r4079659067
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -168,60 +175,70 @@ public void handleRefreshTable(String catalogName, String
dbName, String tableNa
}
public void replayRefreshTable(ExternalObjectLog log) {
- ExternalCatalog catalog = (ExternalCatalog)
Env.getCurrentEnv().getCatalogMgr().getCatalog(log.getCatalogId());
- if (catalog == null) {
- LOG.warn("failed to find catalog when replaying refresh table:
{}", log.debugForRefreshTable());
- return;
- }
- invalidateLanceTableAccess(catalog);
- Optional<ExternalDatabase<? extends ExternalTable>> db;
- if (!Strings.isNullOrEmpty(log.getDbName())) {
- db = catalog.getDbForReplay(log.getDbName());
- } else {
- db = catalog.getDbForReplay(log.getDbId());
- }
- // See comment in refreshDbInternal for why db and table may be null.
- if (!db.isPresent()) {
- LOG.warn("failed to find db when replaying refresh table: {}",
log.debugForRefreshTable());
- return;
- }
- Optional<? extends ExternalTable> table;
- if (!Strings.isNullOrEmpty(log.getTableName())) {
- table = db.get().getTableForReplay(log.getTableName());
- } else {
- table = db.get().getTableForReplay(log.getTableId());
- }
- if (!table.isPresent()) {
- LOG.warn("failed to find table when replaying refresh table: {}",
log.debugForRefreshTable());
- return;
- }
- if (!Strings.isNullOrEmpty(log.getNewTableName())) {
- // this is a rename table op
- db.get().unregisterTable(log.getTableName());
- db.get().resetMetaCacheNames();
- } else {
- List<String> modifiedPartNames = log.getPartitionNames();
- List<String> newPartNames = log.getNewPartitionNames();
- if (catalog instanceof HMSExternalCatalog
- && ((modifiedPartNames != null &&
!modifiedPartNames.isEmpty())
- || (newPartNames != null && !newPartNames.isEmpty()))) {
- // Partition-level cache invalidation, only for hive catalog
- HiveExternalMetaCache cache =
Env.getCurrentEnv().getExtMetaCacheMgr()
- .hive(catalog.getId());
- cache.refreshAffectedPartitionsCache((HMSExternalTable)
table.get(), modifiedPartNames, newPartNames);
- if (table.get() instanceof HMSExternalTable &&
log.getLastUpdateTime() > 0) {
- ((HMSExternalTable)
table.get()).setUpdateTime(log.getLastUpdateTime());
- }
- LOG.info("replay refresh partitions for table {}, "
- + "modified partitions count: {}, "
- + "new partitions count: {}",
- table.get().getName(), modifiedPartNames == null ? 0 :
modifiedPartNames.size(),
- newPartNames == null ? 0 : newPartNames.size());
+ replayRefreshSafely("refresh table " + log.getCatalogId(), () -> {
+ ExternalCatalog catalog = (ExternalCatalog)
Env.getCurrentEnv().getCatalogMgr()
+ .getCatalog(log.getCatalogId());
+ if (catalog == null) {
+ LOG.warn("failed to find catalog when replaying refresh table:
{}", log.debugForRefreshTable());
+ return;
+ }
+ invalidateLanceTableAccess(catalog);
+ Optional<ExternalDatabase<? extends ExternalTable>> db;
+ if (!Strings.isNullOrEmpty(log.getDbName())) {
+ db = catalog.getDbForReplay(log.getDbName());
} else {
- // Full table cache invalidation
- refreshTableInternal(db.get(), table.get(),
log.getLastUpdateTime());
+ db = catalog.getDbForReplay(log.getDbId());
}
- }
+ // See comment in refreshDbInternal for why db and table may be
null.
+ if (!db.isPresent()) {
+ LOG.warn("failed to find db when replaying refresh table: {}",
log.debugForRefreshTable());
+ invalidatePaimonCatalogForUnresolvedReplay(catalog);
+ return;
+ }
+ Optional<? extends ExternalTable> table;
+ if (!Strings.isNullOrEmpty(log.getTableName())) {
+ table = db.get().getTableForReplay(log.getTableName());
+ } else {
+ table = db.get().getTableForReplay(log.getTableId());
+ }
+ if (!table.isPresent()) {
+ LOG.warn("failed to find table when replaying refresh table:
{}", log.debugForRefreshTable());
+ // A case-insensitive table-name mapping may have disappeared
while the canonical
+ // table object is still cached; retire it so a same-name
recreation cannot reuse
+ // the previous incarnation.
+ db.get().retireAllTableObjectsWithoutEngineInvalidation();
Review Comment:
**[P2] Avoid retiring siblings for a normal replay cache miss**
`getTableForReplay(name)` returns empty both when the name cannot be
resolved and when its mapping is intact but that table object is simply cold.
In the latter ordinary follower state, this call swaps the entire database
table-object cache, so a refresh record for one cold table evicts every
unrelated hot `ExternalTable` and forces later queries to rebuild them.
Name-bearing full-table refresh records are emitted by explicit refreshes,
non-partitioned Hive inserts, and external-table metadata mutations, so
repeated replay can keep churning the follower cache. Please reserve
database-wide retirement for a genuinely unresolved name or legacy identity,
and handle a known-but-uncached target without discarding its siblings; a
replay test with a cold target and cached sibling would cover the distinction.
--
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]
