github-actions[bot] commented on code in PR #69501:
URL: https://github.com/apache/doris/pull/69501#discussion_r4356914860
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -743,14 +743,34 @@ protected void invalidateTableCache(String
localTableName) {
}
}
+ /** Refreshes table membership and lookup identity without loading remote
metadata. */
+ public void invalidateTableLookup(String tableName) {
+ if (!isInitialized()) {
+ return;
+ }
+ // In case-insensitive lookup mode an older spelling may still be
loading while a refreshed
+ // names snapshot already contains the new spelling. Fence every
object/ID load in this database.
+ if (isTableNamesCaseInsensitive()) {
+ invalidateAllTableCache();
+ return;
+ }
+ String localTableName = resolveTableNameForInvalidation(tableName);
+ try {
+ invalidateTableCache(localTableName);
Review Comment:
[P2] Preserve ID publication for in-flight numeric lookups
With enabled caches and ordinary `lower_case_table_names=0`, an
`ExternalRowCountCache` worker can enter `getTableNullable(long)`, read the
indexed name at line 779, and pause before `tables.get(name)`. This new
per-table invalidation can then finish removing the object and its ID. When the
worker resumes, `tables.get` acquires a fresh post-refresh generation and
caches a replacement object, but that plain getter never restores the ID
mapping. Subsequent numeric lookups fail despite the object being cached, and
row-count/statistics callers return unknown until a lookup by name repairs it.
Make numeric loads revalidate or publish their ID within the guarded
object-publication action, and cover this capture/refresh/resume interleaving.
This is distinct from the existing mode-2 late bulk-clear thread: it uses the
single-key branch and publishes through the numeric path after ID removal has
completed.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -301,18 +316,41 @@ public void refreshExternalTableFromEvent(String
catalogName, String localDbName
}
public void refreshTableInternal(ExternalDatabase db, ExternalTable table,
long updateTime) {
+ refreshTableInternal(db, table, updateTime, false);
+ }
+
+ private void refreshTableInternal(ExternalDatabase db, ExternalTable
table, long updateTime,
+ boolean invalidateTableLookup) {
+ boolean invalidateDatabase = invalidateTableLookup &&
table.getCatalog().getLowerCaseTableNames() == 2;
table.unsetObjectCreated();
if (updateTime > 0) {
table.setUpdateTime(updateTime);
}
// Connector metadata is the row-count source, so invalidate it before
engine and row-count caches.
try {
if (table.getCatalog() instanceof PluginDrivenExternalCatalog) {
- ((PluginDrivenExternalCatalog)
table.getCatalog()).getConnector()
- .invalidateTable(db.getRemoteName(),
table.getRemoteName());
+ if (invalidateDatabase) {
+ // A refreshed local name may resolve to a different
remote spelling in this database.
+ ((PluginDrivenExternalCatalog)
table.getCatalog()).getConnector()
+ .invalidateDb(db.getRemoteName());
+ } else {
+ ((PluginDrivenExternalCatalog)
table.getCatalog()).getConnector()
+ .invalidateTable(db.getRemoteName(),
table.getRemoteName());
+ }
}
} finally {
- Env.getCurrentEnv().getExtMetaCacheMgr().invalidateTable(table);
+ try {
+ if (invalidateTableLookup) {
+ db.invalidateTableLookup(table.getName());
Review Comment:
[P2] Retain the refresh timestamp for the replacement object
`refreshTableInternal` writes `updateTime` onto the old table immediately
above, but this call evicts that object. Its replacement starts with
`ExternalTable.updateTime = 0`, so `REFRESH TABLE` followed by an
`information_schema.tables` query projecting only `UPDATE_TIME` now returns
NULL instead of the refresh time: `FrontendServiceImpl` reads the field
directly, and the schema scanner converts zero to NULL. It stays unset until a
later schema load calls `initSchemaAndUpdateTime`. Preserve the refresh time
across replacement and test it before loading schema; the modified test
currently checks the new timestamp only after `getFullSchema()`.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -301,18 +316,41 @@ public void refreshExternalTableFromEvent(String
catalogName, String localDbName
}
public void refreshTableInternal(ExternalDatabase db, ExternalTable table,
long updateTime) {
+ refreshTableInternal(db, table, updateTime, false);
+ }
+
+ private void refreshTableInternal(ExternalDatabase db, ExternalTable
table, long updateTime,
+ boolean invalidateTableLookup) {
+ boolean invalidateDatabase = invalidateTableLookup &&
table.getCatalog().getLowerCaseTableNames() == 2;
Review Comment:
[P2] Invalidate historical remote spellings in mode 1 as well
A local name can resolve to a different remote spelling with
`lower_case_table_names=1` too. For example, configure FE object/name expiry
shorter than the Iceberg connector table TTL, query remote `Foo` as local
`foo`, externally replace it with `FOO`, and query `foo` after FE-only expiry.
The FE now retains `FOO` while the independent connector entry for `Foo`
survives. Externally replace `FOO` with a new `Foo` containing different data,
then refresh before the FE `FOO` object expires: this condition is false, so
only connector key `FOO` is invalidated. The refreshed names then resolve back
to `Foo`, whose exact-case table/snapshot caches still describe the first
incarnation, allowing stale schema/data after successful refresh. Extend the
broad invalidation to naming modes that can change the remote spelling, and
cover this historical-key reuse case.
--
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]
