924060929 commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4130714671
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/HiveInsertExecutor.java:
##########
@@ -83,17 +83,40 @@ protected void doBeforeCommit() throws UserException {
protected void doAfterCommit() throws DdlException {
HMSExternalTable hmsTable = (HMSExternalTable) table;
+ // The transaction is already committed. Fence the row-count cache by
the held table
+ // identity before any fallible cache work (including
isPartitionedTable reinitialization),
+ // so an evicted or partially reloaded table cannot retain the
pre-insert count.
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(hmsTable);
+
// For partitioned tables, do selective partition refresh
// For non-partitioned tables, do full table cache invalidation
List<String> modifiedPartNames = Lists.newArrayList();
List<String> newPartNames = Lists.newArrayList();
- if (hmsTable.isPartitionedTable() && partitionUpdates != null &&
!partitionUpdates.isEmpty()) {
- HiveExternalMetaCache cache =
Env.getCurrentEnv().getExtMetaCacheMgr()
- .hive(hmsTable.getCatalog().getId());
- cache.refreshAffectedPartitions(hmsTable, partitionUpdates,
modifiedPartNames, newPartNames);
- } else {
- // Non-partitioned table or no partition updates, do full table
refresh
-
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateTableCache(hmsTable);
+ try {
+ if (hmsTable.isPartitionedTable() && partitionUpdates != null &&
!partitionUpdates.isEmpty()) {
+ HiveExternalMetaCache cache =
Env.getCurrentEnv().getExtMetaCacheMgr()
+ .hive(hmsTable.getCatalog().getId());
+ cache.refreshAffectedPartitions(hmsTable, partitionUpdates,
modifiedPartNames, newPartNames);
+ // Close the admission window opened by the fence above: a
load admitted after it can
+ // compute the pre-insert value from the still-resident file
list and publish it.
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(hmsTable);
Review Comment:
Fixed in 8b8afb28. Both full and selective committed Hive insert paths now
unset the held HMS table metadata and close the row-count fence after cache
work. The fallback path is covered too; focused HiveInsertExecutor tests pass.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -192,6 +213,7 @@ public void replayRefreshTable(ExternalObjectLog log) {
table = db.get().getTableForReplay(log.getTableId());
}
if (!table.isPresent()) {
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(catalog.getId(),
db.get().getId());
Review Comment:
Fixed in 8b8afb28. Warm-database/cold-table refresh replay now routes engine
invalidation using the log name; legacy ID-only records conservatively
invalidate the database. Added a cold-table replay regression test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java:
##########
@@ -931,15 +940,38 @@ private void
alterExternalCatalogPropsFenced(ExternalCatalog externalCatalog, Ca
Integer[] sec = {metadataRefreshIntervalSec,
metadataRefreshIntervalSec};
Env.getCurrentEnv().getRefreshManager().addToRefreshMap(catalogId,
sec);
}
- externalCatalog.modifyCatalogProps(newProps);
// The commit reset the catalog's execution context and closed its SDK
resources. Cached
// base generations and projections are bound to the replaced context;
retire them now so
// the next statement loads a generation the planning fences accept,
instead of retrying
- // against an unplannable cached generation until managed refresh.
+ // against an unplannable cached generation until managed refresh. The
properties are
+ // published before the reset's throwable cleanup, so retirement must
run either way.
Env currentEnv = Env.getCurrentEnv();
ExternalMetaCacheMgr cacheMgr = currentEnv == null ? null :
currentEnv.getExtMetaCacheMgr();
- if (cacheMgr != null) {
-
cacheMgr.onCatalogOperationalContextChanged(externalCatalog.getId());
+ try {
+ if (cacheMgr != null) {
+ // Close the old row-count generation before publishing the
new catalog context.
+ // The completion fence below also retires values loaded
during the reset.
+ cacheMgr.invalidateRowCountCache(externalCatalog.getId());
+ }
+ externalCatalog.modifyCatalogProps(newProps);
+ } catch (RuntimeException e) {
+ if (!isReplay) {
+ throw e;
+ }
+ // A follower must not terminate because a local connector cleanup
failed while applying
+ // an already-durable ALTER record. The property publication and
the derived-state
+ // transitions above are failure-safe, so the record is considered
applied.
+ LOG.warn("Failed to complete local cleanup while replaying ALTER
CATALOG for {}: {}",
+ externalCatalog.getName(), e.getMessage(), e);
+ } finally {
+ if (cacheMgr != null) {
+
cacheMgr.onCatalogOperationalContextChanged(externalCatalog.getId());
+ }
+ if (!isReplay && currentEnv != null) {
Review Comment:
Fixed in 8b8afb28. JDBC mapping candidates are parsed during pre-publication
ALTER validation. A malformed value now raises DdlException before property
mutation or edit-log publication. Added a CatalogMgr test proving both
properties and journal stay unchanged.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java:
##########
@@ -320,21 +320,40 @@ public boolean registerDatabaseFromEvent(long dbId,
String dbName, long metadata
return false;
}
+ /** HMS notifications are unfiltered; excluded targets must not change
this catalog's caches. */
+ public boolean isPartitionEventTargetExcluded(String dbName, String
tableName) {
+ if (!isDatabaseAllowedByFilterIgnoringCase(dbName)) {
Review Comment:
Fixed in 8b8afb28. Event handlers now pass the original HMS remote database
spelling to cache/filter operations, and database and include-table filters use
the same exact database-key semantics as listing. Added case-variant filter
coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -253,19 +289,30 @@ public void refreshExternalTableFromEvent(String
catalogName, String dbName, Str
if (!(catalog instanceof ExternalCatalog)) {
throw new DdlException("Only support refresh ExternalCatalog
Tables");
}
+ // Whole-table events are already committed remotely. Fence the row
count by cached identity
+ // before any database/table reload can fail and make the not-found
path return.
+ Env.getCurrentEnv().getExtMetaCacheMgr()
+ .invalidateRowCountCache(catalog.getId(), dbName, tableName);
DatabaseIf db = catalog.getDbNullable(dbName);
if (db == null) {
+ // Cold database: widen so independently resident engine entries
are retired too.
+ Env.getCurrentEnv().getExtMetaCacheMgr()
+ .invalidateTableByNameOrWider(catalog.getId(), dbName,
tableName);
Review Comment:
Fixed in 8b8afb28. Whole-table HMS refresh now applies the same target
visibility guard before row-count or engine invalidation. The filtered-event
test exercises excluded database and table targets beside hot entries.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1247,10 +1320,62 @@ 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;
+ }
+ Optional<Pair<String, Long>> identity = getDbIdentityForDrop(dbName);
+ if (!identity.isPresent()) {
+ // A mode-2 remote-to-local mapping can disappear (for example
after a names refresh)
+ // while the resident database object survives. The canonical key
is then unknown, so
+ // treat the scope as unknown: retire every cached database object
and flush the engine
+ // caches and row counts catalog-wide instead of evicting the
wrong local key.
+ retireAllDatabaseObjectsWithoutEngineInvalidation();
Review Comment:
Fixed in 8b8afb28. DROP and CREATE database event handlers preserve the
original remote spelling, allowing mode-2 DROP to resolve the correct canonical
identity; CatalogMgr skips excluded DROP events before invalidation. Rename
handles visible/excluded sides separately. Existing historical-identity tests
and new excluded-event coverage pass.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java:
##########
@@ -320,21 +320,40 @@ public boolean registerDatabaseFromEvent(long dbId,
String dbName, long metadata
return false;
}
+ /** HMS notifications are unfiltered; excluded targets must not change
this catalog's caches. */
+ public boolean isPartitionEventTargetExcluded(String dbName, String
tableName) {
+ if (!isDatabaseAllowedByFilterIgnoringCase(dbName)) {
+ return true;
+ }
+ return getIncludeTableMap().entrySet().stream()
Review Comment:
Fixed in 8b8afb28. HMS event filtering now reuses a transient parsed
include-table snapshot keyed by the configured property value and refreshes it
after property changes. Removed the full-map INFO log.
--
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]
