github-actions[bot] commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4128907357
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java:
##########
@@ -691,15 +792,70 @@ public void invalidateTableCache(ExternalTable
dorisTable) {
long catalogId = dorisTable.getCatalog().getId();
// Typed table invalidation bypasses the name-based invalidateTable()
entry point, so the
// Lance access-cache retirement that used to happen there has to be
repeated here.
- invalidateLanceTableAccess(catalogId);
- routeCatalogEngines(catalogId, cache -> safeInvalidate(
- cache, catalogId, "invalidateTable", () ->
cache.invalidateTable(dorisTable)));
+ try {
+ invalidateLanceTableAccess(catalogId);
+ routeCatalogEngines(catalogId, cache -> safeInvalidate(
+ cache, catalogId, "invalidateTable", () ->
cache.invalidateTable(dorisTable)));
+ } finally {
+ invalidateRowCountCache(dorisTable);
Review Comment:
[P1] Fence the held table before full REFRESH TABLE resets it.
`RefreshManager.refreshTableInternal` calls `unsetObjectCreated()` before
entering this typed invalidation, so a concurrent query can reload the new HMS
metadata while `routeCatalogEngines` is still running and read the old
completed row-count future; this `finally` fence cannot retract that plan.
Explicit REFRESH TABLE and full-table follower replay lack the opening fence
used by the event and post-commit paths. Publish a table row-count fence before
`unsetObjectCreated()`, keep this completion fence, and latch a query between
the reset and routed invalidation. This is separate from the previously
threaded selective partition refresh.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -619,15 +640,36 @@ public boolean unregisterTableForReplay(String tableName)
{
// Check whether the table still exists in the local replay cache.
ExternalTable dorisTable = getTableForReplay(tableName).orElse(null);
if (dorisTable == null) {
+ // A mode-2 table-name mapping can disappear while the old object
stays resident in
+ // MetaCache.metaObjCache; getTableForReplay then misses only
because the mapping is
+ // absent. Retire the hidden table-object generation so a
same-name recreation cannot
+ // reuse the prior incarnation, then widen the engine and
row-count fence.
+ if (!hasLocalTableName(tableName)) {
+ retireAllTableObjectsWithoutEngineInvalidation();
+ }
+ // The table object cache is much smaller than the row-count and
Hive engine caches. A drop
+ // or rename must still retire stale entries when the table object
was evicted, and the
+ // event carries the caller spelling, so widen to the canonical
scope.
+ Env.getCurrentEnv().getExtMetaCacheMgr()
+ .invalidateTableByNameOrWider(extCatalog.getId(),
getFullName(), tableName);
Review Comment:
[P1] Open a DB row-count fence before this known-name cold-table route.
Ordinary table-object eviction can make `getTableForReplay` miss while
`hasLocalTableName` remains true, so the pre-fenced retirement at line 648 is
skipped. On delayed follower DROP after a remote same-name recreation,
`invalidateTableByNameOrWider` routes DB engine invalidation before its final
row-count fence. A concurrent query can load the replacement under its
deterministic table ID during that work and use the old completed count. This
differs from the existing missing-name object-retirement thread at this line.
Fence the DB counts before entering the cold route, retain the completion
fence, and latch the known-name/evicted-object replay case.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1247,10 +1273,48 @@ public void unregisterDatabase(String dbName) {
if (LOG.isDebugEnabled()) {
LOG.debug("unregister database [{}]", dbName);
}
- if (isInitialized()) {
- metaCache.invalidate(dbName, Util.genIdByName(name, dbName));
+ if (!isInitialized()) {
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(getId(),
dbName);
+ return;
+ }
+ String localDbName = getLocalDatabaseName(dbName, true);
Review Comment:
[P1] Keep the historical DROP target when a mode-2 name map rebounds. If
remote `Foo` is dropped and case-only replacement `FOO` is created, a names
refresh can rebind `foo -> FOO` while the old `Foo` DB object remains cached. A
delayed `unregisterDatabase("Foo")` now resolves `FOO` here, so the following
object, engine, and row-count invalidations retire the replacement and leave
old `Foo` reusable if that spelling returns. The existing missing-map thread
does not cover this present-but-rebound mapping. The new cold replay identity
helper repeats the historical-name lookup. Preserve the pre-drop canonical
key/ID where available, or widen retirement when the mapping has changed; test
a delayed case-only drop/recreate.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java:
##########
@@ -691,15 +792,70 @@ public void invalidateTableCache(ExternalTable
dorisTable) {
long catalogId = dorisTable.getCatalog().getId();
// Typed table invalidation bypasses the name-based invalidateTable()
entry point, so the
// Lance access-cache retirement that used to happen there has to be
repeated here.
- invalidateLanceTableAccess(catalogId);
- routeCatalogEngines(catalogId, cache -> safeInvalidate(
- cache, catalogId, "invalidateTable", () ->
cache.invalidateTable(dorisTable)));
+ try {
+ invalidateLanceTableAccess(catalogId);
+ routeCatalogEngines(catalogId, cache -> safeInvalidate(
Review Comment:
[P1] Fence the typed table row count before evicting its engine entries.
Iceberg `SET CURRENT SNAPSHOT` commits a new snapshot and calls this method
without an opening fence. `routeCatalogEngines` removes Iceberg table/snapshot
entries here; a concurrent query can reload the new snapshot and still get the
old completed count until the `finally` at line 800. Iceberg snapshot/rewrite
actions share this path. Open the table row-count fence at manager entry,
retain the completion fence, and test a query paused after Iceberg entry
removal. This is distinct from full REFRESH TABLE, whose held-object reset
occurs before this method is entered.
--
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]
