Repository navigation
branch-4.1: [fix](meta cache) Invalidate external row count cache after metadata changes #64160 #68196
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,8 +158,13 @@ private RemovedCatalog removeCatalog(long catalogId) { | |
| } | ||
| String catalogName = catalog.getName(); | ||
| Env.getCurrentEnv().getRefreshManager().removeFromRefreshMap(catalogId); | ||
| // Remove both lock-free mappings first: a row-count load admitted after the fence below | ||
| // could otherwise still resolve this catalog through them and alias a same-name | ||
| // replacement, because RowCountKey equality uses only tableId. Fence last, while this | ||
| // write lock still excludes a same-name CREATE. | ||
| idToCatalog.remove(catalogId); | ||
| nameToCatalog.remove(catalogName); | ||
| Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(catalogId); | ||
| return new RemovedCatalog(catalog, catalogName); | ||
| } | ||
|
|
||
|
|
@@ -476,7 +481,11 @@ && hasLanceIdentityKeyChange(oldProperties, newProperties) | |
| } | ||
| log.setNewProps(loggedProperties); | ||
| replayAlterCatalogProps(log, oldProperties, false); | ||
| if (!(catalog instanceof ExternalCatalog)) { | ||
| // External catalogs journal the committed change from the fenced helper once publication | ||
| // has happened; non-external catalogs only reach this point on success. | ||
| Env.getCurrentEnv().getEditLog().logCatalogLog(OperationType.OP_ALTER_CATALOG_PROPS, log); | ||
| } | ||
| } finally { | ||
| writeUnlock(); | ||
| } | ||
|
|
@@ -931,15 +940,38 @@ private void alterExternalCatalogPropsFenced(ExternalCatalog externalCatalog, Ca | |
| Integer[] sec = {metadataRefreshIntervalSec, metadataRefreshIntervalSec}; | ||
| Env.getCurrentEnv().getRefreshManager().addToRefreshMap(catalogId, sec); | ||
| } | ||
| // 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. 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(); | ||
| 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); | ||
|
Contributor
There was a problem hiding this comment. [P1] Keep connector cleanup failures out of fatal property replay. A live ALTER now deliberately journals after properties are published even when reset/close throws, but an already-initialized follower replays that record through the same |
||
| } 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()); | ||
|
Contributor
There was a problem hiding this comment. [P1] Publish an opening row-count fence before this ALTER exposes the new metadata generation.
Contributor
Author
There was a problem hiding this comment. Fixed in 537ea2c. ALTER CATALOG now fences catalog row counts before |
||
| } | ||
| if (!isReplay && currentEnv != null) { | ||
|
Contributor
There was a problem hiding this comment. [P1] Validate JDBC name mapping before journaling this ALTER. A JDBC
Contributor
Author
There was a problem hiding this comment. Fixed in 8b8afb2. 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. |
||
| // Publication has happened once modifyCatalogProps is reached; journal it even if the | ||
| // reset's own cleanup throws. Pre-publication validation failures never reach here. | ||
| currentEnv.getEditLog().logCatalogLog(OperationType.OP_ALTER_CATALOG_PROPS, log); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1039,7 +1071,11 @@ public void unregisterExternalDatabase(String dbName, String catalogName) | |
| if (!(catalog instanceof ExternalCatalog)) { | ||
| throw new DdlException("Only support drop ExternalCatalog databases"); | ||
| } | ||
| HMSExternalCatalog hmsCatalog = (HMSExternalCatalog) catalog; | ||
| if (hmsCatalog.isDatabaseEventTargetExcluded(dbName)) { | ||
| return; | ||
| } | ||
| hmsCatalog.unregisterDatabase(dbName); | ||
| } | ||
|
|
||
| public void registerExternalDatabaseFromEvent(String dbName, String catalogName) | ||
|
|
@@ -1054,7 +1090,7 @@ public void registerExternalDatabaseFromEvent(String dbName, String catalogName) | |
|
|
||
| HMSExternalCatalog hmsCatalog = (HMSExternalCatalog) catalog; | ||
| long metadataLoadEpoch = hmsCatalog.acquireMetadataLoadEpoch(); | ||
| long dbId = Util.genIdByName(catalogName, hmsCatalog.localDatabaseNameFromRemote(dbName)); | ||
| // -1L means it will be dropped later, ignore | ||
| if (dbId == ExternalMetaIdMgr.META_ID_FOR_NOT_EXISTS) { | ||
| return; | ||
|
|
@@ -1075,8 +1111,29 @@ public void addExternalPartitions(String catalogName, String dbName, String tabl | |
| if (!(catalog instanceof ExternalCatalog)) { | ||
| throw new DdlException("Only support ExternalCatalog"); | ||
| } | ||
| if (catalog instanceof HMSExternalCatalog | ||
| && ((HMSExternalCatalog) catalog).isPartitionEventTargetExcluded(dbName, tableName)) { | ||
| return; | ||
| } | ||
| // Partition events are already committed remotely. Fence the row count by cached identity | ||
| // before any database/table reload can fail and make the ignored-not-found path return. | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateRowCountCache(catalog.getId(), dbName, tableName); | ||
| try { | ||
| addExternalPartitionsAfterFence(catalog, dbName, tableName, partitionNames, updateTime, | ||
| ignoreIfNotExists); | ||
| } finally { | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateRowCountCache(catalog.getId(), dbName, tableName); | ||
| } | ||
| } | ||
|
|
||
| private void addExternalPartitionsAfterFence(CatalogIf catalog, String dbName, String tableName, | ||
| List<String> partitionNames, long updateTime, boolean ignoreIfNotExists) throws DdlException { | ||
| DatabaseIf db = catalog.getDbNullable(dbName); | ||
|
Contributor
There was a problem hiding this comment. [P2] Skip invalidation for HMS events excluded by catalog filters. In a mode-2 HMS catalog that excludes
Contributor
Author
There was a problem hiding this comment. Fixed in 182ef43. HMS now checks the catalog database and include-table filters before any row-count or engine-cache invalidation. All three notification routes are covered: add/drop partition in CatalogMgr and alter-partition refresh in RefreshManager. CatalogMgrTest exercises excluded ARCHIVED.t and sales.cold alongside an included sales.hot target and verifies no cache-manager interaction for the excluded events. The 176 focused FE tests and full FE build pass. |
||
| if (db == null) { | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateTableByNameOrWider(catalog.getId(), dbName, tableName); | ||
| if (!ignoreIfNotExists) { | ||
| throw new DdlException("Database " + dbName + " does not exist in catalog " + catalog.getName()); | ||
| } | ||
|
|
@@ -1085,6 +1142,8 @@ public void addExternalPartitions(String catalogName, String dbName, String tabl | |
|
|
||
| TableIf table = db.getTableNullable(tableName); | ||
| if (table == null) { | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateTableByNameOrWider(catalog.getId(), dbName, tableName); | ||
| if (!ignoreIfNotExists) { | ||
| throw new DdlException("Table " + tableName + " does not exist in db " + dbName); | ||
| } | ||
|
|
@@ -1118,8 +1177,29 @@ public void dropExternalPartitions(String catalogName, String dbName, String tab | |
| if (!(catalog instanceof ExternalCatalog)) { | ||
| throw new DdlException("Only support ExternalCatalog"); | ||
| } | ||
| if (catalog instanceof HMSExternalCatalog | ||
| && ((HMSExternalCatalog) catalog).isPartitionEventTargetExcluded(dbName, tableName)) { | ||
| return; | ||
| } | ||
| // Partition events are already committed remotely. Fence the row count by cached identity | ||
| // before any database/table reload can fail and make the ignored-not-found path return. | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateRowCountCache(catalog.getId(), dbName, tableName); | ||
| try { | ||
| dropExternalPartitionsAfterFence(catalog, dbName, tableName, partitionNames, updateTime, | ||
| ignoreIfNotExists); | ||
| } finally { | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateRowCountCache(catalog.getId(), dbName, tableName); | ||
| } | ||
| } | ||
|
|
||
| private void dropExternalPartitionsAfterFence(CatalogIf catalog, String dbName, String tableName, | ||
| List<String> partitionNames, long updateTime, boolean ignoreIfNotExists) throws DdlException { | ||
| DatabaseIf db = catalog.getDbNullable(dbName); | ||
| if (db == null) { | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateTableByNameOrWider(catalog.getId(), dbName, tableName); | ||
| if (!ignoreIfNotExists) { | ||
| throw new DdlException("Database " + dbName + " does not exist in catalog " + catalog.getName()); | ||
| } | ||
|
|
@@ -1128,6 +1208,8 @@ public void dropExternalPartitions(String catalogName, String dbName, String tab | |
|
|
||
| TableIf table = db.getTableNullable(tableName); | ||
| if (table == null) { | ||
| Env.getCurrentEnv().getExtMetaCacheMgr() | ||
| .invalidateTableByNameOrWider(catalog.getId(), dbName, tableName); | ||
| if (!ignoreIfNotExists) { | ||
| throw new DdlException("Table " + tableName + " does not exist in db " + dbName); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Complete the property-specific transition when reset cleanup throws. The properties have already been published here, and the outer finally now journals them, but a JDBC
closeClient()exception exitsnotifyPropertiesUpdatedbeforeJdbcExternalCatalog.resetToUninitializedrebuildsidentifierMappingand before the generic hook removes cache groups for schema TTL, catalog weight, ormeta.cache.*changes. This fallback only empties entries, so the leader can keep old name-mapping semantics and old cache policy for a durably committed ALTER (while a follower that replays cleanly uses the new values). Preserve the cleanup exception if needed, but still run the connector-derived and property-specific post-publication transitions; add a throwing-close regression that verifies the new mapping and rebuilt policy group.