github-actions[bot] commented on code in PR #66913:
URL: https://github.com/apache/doris/pull/66913#discussion_r4068188645
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -337,39 +332,40 @@ public void afterDropDb(String dbName) {
@Override
public boolean createTableImpl(CreateTableInfo createTableInfo) throws
UserException {
try {
- return executionAuthenticator.execute(() ->
performCreateTable(createTableInfo));
+ return executeCatalogOperation(() ->
performCreateTable(createTableInfo));
} catch (Exception e) {
throw new DdlException(
"Failed to create table: " + createTableInfo.getTableName() +
", error message is:" + e.getMessage(),
e);
}
}
- public boolean performCreateTable(CreateTableInfo createTableInfo) throws
UserException {
+ private boolean performCreateTable(CreateTableInfo createTableInfo) throws
UserException {
String dbName = createTableInfo.getDbName();
- ExternalDatabase<?> db = dorisCatalog.getDbNullable(dbName);
+ ExternalDatabase<?> db = getDatabaseWithinCatalogGeneration(dbName);
if (db == null) {
throw new UserException("Failed to get database: '" + dbName + "'
in catalog: " + dorisCatalog.getName());
}
String tableName = createTableInfo.getTableName();
// 1. first, check if table exist in remote
- if (tableExist(db.getRemoteName(), tableName)) {
+ if (tableExistsInternal(db.getRemoteName(), tableName)) {
if (createTableInfo.isIfNotExists()) {
LOG.info("create table[{}] which already exists", tableName);
return true;
} else {
ErrorReport.reportDdlException(ErrorCode.ERR_TABLE_EXISTS_ERROR, tableName);
}
}
- // 2. second, check fi table exist in local.
+ // 2. second, check if the table conflicts under Doris case-folding
rules.
// This is because case sensibility issue, eg:
// 1. lower_case_table_name = 1
// 2. create table tbl1;
// 3. create table TBL1; TBL1 does not exist in remote because the
remote system is case-sensitive.
// but because lower_case_table_name = 1, the table can not be
created in Doris because it is conflict with
// tbl1
- ExternalTable dorisTable = db.getTableNullable(tableName);
- if (dorisTable != null) {
+ // Keep this lookup on the retained Iceberg catalog generation.
ExternalDatabase is resettable,
+ // so calling getTableNullable() here could enumerate a newer catalog
generation.
+ if (hasCaseFoldTableCollision(db.getRemoteName(), tableName)) {
Review Comment:
[P1] Include views in the retained case-fold collision check
This fixes the G1/G2 race from the earlier thread, but
`listTableNamesInternal()` deliberately filters out every name returned by
`listViewNamesInternal()`. The old `db.getTableNullable()` lookup used
`IcebergExternalCatalog.listTableNamesFromRemote()`, which combines tables and
views because Doris exposes them in one local namespace. With
`lower_case_table_names` enabled, an existing view `tbl1` therefore no longer
blocks `CREATE TABLE TBL1`; the create can succeed and the next combined
enumeration sees two remote objects mapping to the same local name. Please
include retained-generation view names in this collision check and cover a
case-variant view/table pair.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -1800,18 +1922,40 @@ private Table useFrozenTableGeneration(Table
currentTable) {
"Unknown Iceberg system table type: %s",
systemTable.getSysTableType());
// Snapshot-selectable metadata tables must derive
their scans and schemas
// from the same frozen base generation as the
relation's snapshot fence.
+ frozenGenerationSource = cacheValue;
return
MetadataTableUtils.createMetadataTableInstance(frozenBaseTable, tableType);
}
+ // This scan falls back to the current generation, so its
asynchronous planner
Review Comment:
[P1] Keep each system table on one generation
This branch groups two different kinds of non-snapshot-selectable metadata
tables after already installing the injected snapshot's G1 runtime.
`ALL_DATA_FILES`/`ALL_DELETE_FILES`/`ALL_FILES`/`ALL_ENTRIES` are built by
`resolveBaseTable()` over frozen G1, but returning here leaves
`frozenGenerationSource` null, so async planning retains statement G2. Static
tables such as `SNAPSHOTS` are built over current G2, yet they keep G1's
authenticator, storage properties, and planning executor. After an injected G1
snapshot and catalog reset, the former can lose its real generation while
planning and the latter mixes a G2 table with G1 resources. Please classify by
whether the system table binds to the statement generation: retain G1 for
ALL_*, and clear/use G2 runtime state for truly static tables. Cover both
classes with an injected-G1/current-G2 reset and cancellation.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hudi/source/HudiScanNode.java:
##########
@@ -552,59 +644,372 @@ private void initPrunedPartitions() throws UserException
{
throw new UserException(ExceptionUtils.getRootCauseMessage(e), e);
}
partitionInit = true;
+ ensureHmsRuntimeGeneration();
}
@Override
public void startSplit(int numBackends) {
+ ensureHmsRuntimeGeneration();
if (prunedPartitions.isEmpty()) {
splitAssignment.finishSchedule();
+ releaseFsViewOnce();
return;
}
- AtomicInteger numFinishedPartitions = new AtomicInteger(0);
+ acquireFsView();
Review Comment:
[P1] Publish cancellation ownership before synchronizing the fs view
`acquireFsView()` does not return until `FsViewGeneration.getFsView()` has
run the potentially blocking remote `lease.get().sync()`, but the batch owner
is only constructed and registered after this call; the non-batch listing owner
is likewise created later in `getPartitionsSplits()`. The sync now runs under
the correct authenticator (the earlier thread), but cancellation during a
stalled call still has no statement or split-assignment closeable to signal, so
query work and this exact catalog generation can remain pinned without ever
reaching the accepted-task cancellation machinery. Please separate exact-lease
acquisition from sync, publish an owner before starting the I/O, make
cancellation signal that work, and release the lease only from its
actual-terminal path. Cover blocked-sync cancellation in both batch and
non-batch modes.
--
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]
