924060929 commented on code in PR #66914:
URL: https://github.com/apache/doris/pull/66914#discussion_r3821207021
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergStatementScope.java:
##########
@@ -83,6 +83,39 @@ static Table sharedTable(ConnectorSession session, String
dbName, String tableNa
() -> snapshotReadTable(loader.get()));
}
+ /**
+ * Statement-scoped variant for a table borrowed from {@link
IcebergTableCache}. The memoized holder is
+ * {@link AutoCloseable}, so the engine's statement-scope teardown
releases the borrower only after scan
+ * pumps have quiesced. Cache eviction and statement completion may happen
in either order; the underlying
+ * FileIO is closed only after both owners release it.
+ */
+ static Table sharedBorrowedTable(ConnectorSession session, String dbName,
String tableName,
+ Supplier<IcebergTableCache.TableLease> loader, Supplier<Table>
unscopedLoader) {
+ if (session == null || session.getStatementScope() ==
ConnectorStatementScope.NONE) {
Review Comment:
Fixed in 5c2bdd1643d. Cross-statement metadata operations now create an
isolated ConnectorStatementScopeImpl and close it in finally at every
production call site. Iceberg therefore uses the enabled table cache and
retains a TableLease for the complete schema/statistics/row-count operation
instead of taking the NONE/raw-loader path. Post-rebase FE-core call-path tests
pass 136/136, and Iceberg scope/cache tests pass 49/49 within the focused
52-test Iceberg suite.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -1525,6 +1581,10 @@ public java.util.logging.Logger getParentLogger() throws
java.sql.SQLFeatureNotS
@Override
public void close() throws IOException {
+ // Release connector-owned cache references while the catalog objects
are still available to classify
+ // and close per-table FileIO. Active statement borrowers retain their
own references and defer the actual
+ // cleanup until their leases close.
+ invalidateAll();
Review Comment:
Fixed in 5c2bdd1643d. Cached table owners now retain an Iceberg
catalog-generation lease. Connector close and REST 401 replacement retire the
generation and release the cache owner, but defer catalog/delegate cleanup
until in-flight loads and the final active table borrower release. The tracker
covers loads spanning one or more replacements and failed loads. Focused tests
include connector-close-with-active-borrower and generation rotation/abort
cases; all 52 Iceberg lifecycle/cache/scope tests pass post-rebase.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiScanPlanProvider.java:
##########
@@ -294,30 +294,33 @@ basePath, inputFormat, serdeLib, columnNames,
columnTypes, partitionFieldNames(m
HoodieLocalEngineContext engineCtx = new
HoodieLocalEngineContext(metaClient.getStorageConf());
HoodieTableFileSystemView fsView =
FileSystemViewManager.createInMemoryFileSystemView(
engineCtx, metaClient, metadataConfig);
+ try {
+ // Resolve partitions
+ List<String> partitionPaths = resolvePartitions(hudiHandle,
metaClient);
- // Resolve partitions
- List<String> partitionPaths = resolvePartitions(hudiHandle,
metaClient);
-
- List<ConnectorScanRange> ranges = new ArrayList<>();
- for (String partitionPath : partitionPaths) {
- Map<String, String> partValues = parsePartitionValues(
- partitionPath, hudiHandle.getPartitionKeyNames());
+ List<ConnectorScanRange> ranges = new ArrayList<>();
+ for (String partitionPath : partitionPaths) {
+ Map<String, String> partValues = parsePartitionValues(
+ partitionPath, hudiHandle.getPartitionKeyNames());
- if (useNativeCowPath) {
- collectCowSplits(fsView, partitionPath, queryInstant,
- basePath, partValues, ranges, schemaIdResolver);
- } else {
- collectMorSplits(fsView, partitionPath, queryInstant,
- basePath, inputFormat, serdeLib,
- columnNames, columnTypes, partValues, forceJni,
ranges, schemaIdResolver);
+ if (useNativeCowPath) {
+ collectCowSplits(fsView, partitionPath, queryInstant,
+ basePath, partValues, ranges, schemaIdResolver);
+ } else {
+ collectMorSplits(fsView, partitionPath, queryInstant,
+ basePath, inputFormat, serdeLib,
+ columnNames, columnTypes, partValues, forceJni,
ranges, schemaIdResolver);
+ }
}
- }
- LOG.info("Hudi scan planning: {}.{} type={} partitions={} splits={}",
- hudiHandle.getDbName(), hudiHandle.getTableName(),
- hudiHandle.getHudiTableType(), partitionPaths.size(),
ranges.size());
+ LOG.info("Hudi scan planning: {}.{} type={} partitions={}
splits={}",
+ hudiHandle.getDbName(), hudiHandle.getTableName(),
+ hudiHandle.getHudiTableType(), partitionPaths.size(),
ranges.size());
- return ranges;
+ return ranges;
+ } finally {
+ fsView.close();
Review Comment:
Fixed in 5c2bdd1643d. Both the normal Hudi scan view and
MORIncrementalRelation view now use try-with-resources, and the separately
created HoodieTableMetadata is also closed with try-with-resources semantics.
The recording-fake lifecycle test verifies normal close and that a close
failure is suppressed behind the original planning/listing failure. Hudi
focused tests pass 2/2 post-rebase.
##########
fe/fe-connector/fe-connector-hudi/pom.xml:
##########
@@ -258,6 +258,16 @@ under the License.
<artifactId>junit-jupiter</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
Review Comment:
Fixed in 5c2bdd1643d. Removed both mockito-core and kryo-shaded from the
Hudi connector POM. HudiResourceLifecycleTest now uses a small recording
AutoCloseable fake and a narrow Callable/AutoCloseable lifecycle seam.
Connector focused tests and Checkstyle pass with no mocking dependency added.
--
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]
