zhangstar333 commented on code in PR #69384:
URL: https://github.com/apache/doris/pull/69384#discussion_r4335234631
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -263,7 +263,11 @@ public void replayRefreshTable(ExternalObjectLog log) {
}
if (!Strings.isNullOrEmpty(log.getNewTableName())) {
// this is a rename table op
- db.get().unregisterTable(log.getTableName());
+ db.get().unregisterTableForReplay(log.getTableName());
+ if
(db.get().getTableForReplay(log.getNewTableName()).isPresent()
Review Comment:
### F-04 ยท Replay renames even when the source table object is cold
- **Severity**: Major
- **Where**:
`fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:266-270`
```java
db.get().unregisterTableForReplay(log.getTableName());
if
(db.get().getTableForReplay(log.getNewTableName()).isPresent()
||
db.get().hasLocalTableName(log.getNewTableName())) {
db.get().unregisterTableForReplay(log.getNewTableName());
}
```
**What is wrong.** A follower with cached table names but no cached source
table object skips the rename-specific names invalidation.
**Why it happens.**
`fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:235-264`
looks up the old object and returns on a miss before reaching this branch and
resetMetaCacheNames. ExternalDatabase.getTableForReplay only probes the object
cache; getTableNamesWithLock separately serves cached names. The fallback
invalidateTableByNameOrWider clears engine caches and row counts, not
ExternalDatabase metadata names.
**When it bites.** A follower runs SHOW TABLES and caches events, then the
events table object is evicted while the names snapshot remains valid. SHOW
TABLES itself constructs table objects through getTables, so the concrete
trigger requires subsequent object eviction rather than assuming SHOW TABLES
never loads the object. The leader successfully renames events to
renamed_events in a REST namespace. The follower replays the journal but
continues listing events and omitting renamed_events until a separate names
refresh. This needs no concurrent external mutation. Broad engine invalidation
means this scenario alone does not prove stale engine schema/data.
**Suggested fix.** Move the whole existing rename branch immediately after
successful database resolution, before looking up the old table object, and
return after handling it. Keep both-name retirement and resetMetaCacheNames on
this path. Remove the later duplicate branch; leave the generic cold-source
fallback for non-rename refreshes. The essential early dispatch is:
```diff
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java
@@ before resolving the old table object
+ if (!Strings.isNullOrEmpty(log.getNewTableName())) {
+ db.get().unregisterTableForReplay(log.getTableName());
+ if
(db.get().getTableForReplay(log.getNewTableName()).isPresent()
+ ||
db.get().hasLocalTableName(log.getNewTableName())) {
+
db.get().unregisterTableForReplay(log.getNewTableName());
+ }
+ db.get().resetMetaCacheNames();
+ return;
+ }
Optional<? extends ExternalTable> table;
```
--
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]
