github-actions[bot] commented on code in PR #65126:
URL: https://github.com/apache/doris/pull/65126#discussion_r3689973534
##########
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:
**[P1] Invalidate connector caches for event-driven renames**
This now converges the FE keys by unregistering and re-registering the
table, but neither `CatalogMgr` mutator invokes the connector's invalidation
hooks. `HmsEventSource` only polls/parses through the same cache-backed HMS
client; it does not flush it, while `HiveConnector.invalidateTable()` is what
clears cached HMS metadata, file listings, derived partition views, and built
siblings. After an external `t -> t_arch; t_new -> t` swap, FE can therefore
point at the new `t` while the connector still serves the old `t` incarnation
until TTL. Please invalidate the source and target remote keys during event
application (including same-name DROP/CREATE reincarnations and the database
analog), and cover this with a recording-connector cache test.
##########
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:
**[P1] Preserve case on CREATE and ALTER event targets**
This helper cannot recover case that the HMS descriptor has already
discarded: `HmsEventParser` still lowercases `CREATE_TABLE` and the
`ALTER_TABLE` target, while the recent DROP fix preserves the original
spelling. In mode 2 the supplied spelling is the canonical object/ID key. If
`MixedTbl` is warmed before the delayed CREATE event, registering `mixedtbl`
adds a second alias to the hot names/ID state; the lowercase lookup index then
routes a later DROP of `MixedTbl` to the wrong key, leaving the hot mixed-case
object and engine caches. Even with cold names, the lowercase retained ID
survives normal lookup plus DROP. Please preserve the original CREATE/ALTER
target spelling too and add a parser-to-event mode-2 race/drop regression.
##########
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:
**[P1] Pin event canonicalization to the connector loader**
This event path now canonicalizes through `fromRemoteDatabaseName()` (and
the table path does the same), which dispatches via `PluginDrivenMetadata` into
connector identifier hooks. However, `MetastoreEventSyncDriver` pins TCCL only
around `pollOnce()` and restores the FE daemon loader before
`applyDescriptors()`. A directory connector whose identifier hook uses
plugin-local services or by-name classes can therefore fail only while applying
events and repeatedly reset the event cursor. Please run descriptor application
under the event source/connector classloader, or centralize these callbacks
behind a TCCL-pinned `PluginDrivenExternalCatalog` boundary, with a
recording-loader test.
--
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]
