924060929 commented on code in PR #66913:
URL: https://github.com/apache/doris/pull/66913#discussion_r3903291621
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergTransaction.java:
##########
@@ -148,7 +150,8 @@ public void beginInsert(ExternalTable dorisTable, Table
targetTable,
this.writeSchemaContext = insertCtx == null
? Optional.empty() : insertCtx.getWriteSchemaContext();
try {
- ops.getExecutionAuthenticator().execute(() -> {
+ acquireWritableTableLease(dorisTable);
Review Comment:
Fixed in bf22c673628. `beforeExec()` is now inside the existing
catch/finally envelope for both normal and empty inserts, so every
post-registration setup failure reaches `onFail()` and removes both the
manager-local and global transaction entries. Added deterministic regressions
for both paths; the captured live catalog authenticator is intentionally
unavailable in the test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergTransaction.java:
##########
@@ -770,17 +827,21 @@ public boolean requireStrictCleanup() {
@Override
public void rollback() {
- if (isRewriteMode) {
- // Clear the collected files for rewrite mode
- synchronized (filesToDelete) {
- filesToDelete.clear();
- }
- synchronized (filesToAdd) {
- filesToAdd.clear();
+ try {
+ if (isRewriteMode) {
+ // Clear the collected files for rewrite mode
+ synchronized (filesToDelete) {
+ filesToDelete.clear();
+ }
+ synchronized (filesToAdd) {
+ filesToAdd.clear();
+ }
+ LOG.info("Rewrite transaction rolled back");
}
- LOG.info("Rewrite transaction rolled back");
+ // For insert mode, do nothing as original implementation
+ } finally {
+ releaseWritableTableLease();
Review Comment:
Fixed in bf22c673628. Iceberg commit now enters
`IcebergTransaction.commit()` directly, where remote work still runs under the
authenticator retained by the writable generation. Iceberg rollback now invokes
the captured manager directly because it only clears transaction-local state
and releases that retained generation; it no longer depends on the mutable live
catalog authenticator. The new lifecycle tests verify cleanup after reset.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -544,11 +1047,14 @@ private IcebergSnapshotCacheValue loadSnapshotProjection(
MTMVRelatedTableIf table = (MTMVRelatedTableIf) dorisTable;
IcebergSnapshot latestIcebergSnapshot =
IcebergUtils.getLatestIcebergSnapshot(projectionTable);
IcebergPartitionInfo icebergPartitionInfo;
- if (!table.isValidRelatedTable()) {
+ boolean validRelatedTable = validRelatedTableOverride == null
+ ? table.isValidRelatedTable() : validRelatedTableOverride;
Review Comment:
Fixed in bf22c673628. Snapshot projection now classifies the exact captured
`projectionTable` through a pure `IcebergUtils.isValidRelatedTable(Table)`
function for both native and HMS Iceberg paths. It neither performs a second
borrow nor reads/writes the ExternalTable/DLA boolean cache. Added
native-generation and HMS stale-cache regressions; the latter verifies an old
cached `true` cannot classify a captured two-field generation.
--
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]
