wenzhenghu commented on code in PR #65126:
URL: https://github.com/apache/doris/pull/65126#discussion_r3694387762
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java:
##########
@@ -733,20 +728,16 @@ public void registerExternalTableFromEvent(String dbName,
String tableName,
return;
}
- long tblId = Util.genIdByName(catalogName, dbName, tableName);
- // -1L means it will be dropped later, ignore
- if (tblId == ExternalMetaIdMgr.META_ID_FOR_NOT_EXISTS) {
- return;
- }
+ ExternalDatabase<?> externalDatabase = (ExternalDatabase<?>) db;
+ String localTableName =
externalDatabase.canonicalLocalTableNameFromRemote(tableName);
Review Comment:
Fixed in `85776101dc8`.
`HmsEventParser` now preserves the original table spelling from the HMS
payload for `CREATE_TABLE` and the target of `ALTER_TABLE`; database
normalization remains unchanged. Therefore, in mode 2 the event path registers
the same exact remote table identity and ID as normal metadata loading, instead
of introducing a lowercase alias. A later DROP using another Hive-equivalent
spelling is resolved through the retained case-insensitive name index and
removes that same identity.
Regression coverage was added in `HmsEventParserTest` for CREATE/ALTER
parsing and in `ExternalCatalogTest` for mode-2 CREATE followed by a
differently-cased DROP.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/MetastoreEventSyncDriver.java:
##########
@@ -289,14 +288,8 @@ private void applyRenameTable(PluginDrivenExternalCatalog
catalog, MetastoreChan
throws Exception {
String catalogName = catalog.getName();
CatalogMgr catalogMgr = Env.getCurrentEnv().getCatalogMgr();
- // legacy AlterTableEvent: a rename to a DIFFERENT key that already
exists locally is skipped
- // (processRename guard); a view recreate (after == before) always
proceeds (processRecreateTable).
- boolean sameKey =
descriptor.getDbName().equalsIgnoreCase(descriptor.getDbNameAfter())
- &&
descriptor.getTableName().equalsIgnoreCase(descriptor.getTableNameAfter());
- if (!sameKey &&
catalogMgr.externalTableExistInLocal(descriptor.getDbNameAfter(),
- descriptor.getTableNameAfter(), catalogName)) {
- return;
- }
+ // Always converge to "old removed, new registered". The target may
already be hot because a normal
+ // lookup raced with the event after the remote rename; it must not
prevent cleanup of the old identity.
catalogMgr.unregisterExternalTable(descriptor.getDbName(),
descriptor.getTableName(),
Review Comment:
Fixed in `85776101dc8`.
Before applying a structural descriptor, `MetastoreEventSyncDriver` now
invalidates the connector-owned cache using the descriptor remote names.
CREATE/DROP invalidate the affected key; rename invalidates both source and
target keys (deduplicated when they are identical); same-name DROP/CREATE
reincarnations invalidate once for each event. Existing refresh and partition
paths keep their current invalidation flow and are not duplicated.
`MetastoreEventSyncDriverRenameTest` now verifies
invalidation-before-FE-mutation ordering for table rename, same-name view
replacement, same-name table reincarnation, and database reincarnation.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java:
##########
@@ -432,14 +443,16 @@ public void validateCreateTableEngine(String engineName)
throws AnalysisExceptio
* REGISTER_DATABASE change (via {@code
CatalogMgr.registerExternalDatabaseFromEvent}). Pulled up from
* {@code HMSExternalCatalog} so a flipped (generic) catalog no longer
throws
* {@code NotImplementedException} on a create/rename-database event. The
body is fully generic
- * (buildDbForInit + metaCache, name-derived id) and mirrors the legacy
HMS implementation.
+ * (buildDbForInit + the shared metadata-cache update protocol) and
mirrors the legacy HMS implementation.
*/
@Override
- public void registerDatabase(long dbId, String dbName) {
- ExternalDatabase<? extends ExternalTable> db = buildDbForInit(dbName,
null, dbId, logType, false);
+ public void registerDatabase(String dbName) {
+ String localDbName = canonicalLocalDatabaseNameFromRemote(dbName);
Review Comment:
Fixed in `85776101dc8`.
The driver now pins `getEventSource()` to the connector classloader, keeps
`pollOnce()` under the event-source classloader, and runs descriptor
application plus master-side full refresh under the connector classloader. The
helper restores the caller TCCL in `finally`, including failure paths.
A recording-loader `MetastoreEventSyncDriverClassLoaderTest` covers
event-source acquisition, polling, descriptor invalidation/mutation, master
full refresh, failure propagation, and caller-loader restoration.
--
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]