nastra commented on code in PR #8315:
URL: https://github.com/apache/iceberg/pull/8315#discussion_r1293236872
##########
aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java:
##########
@@ -238,7 +238,8 @@ protected TableOperations newTableOps(TableIdentifier
tableIdentifier) {
awsProperties,
tableSpecificCatalogPropertiesBuilder.buildOrThrow(),
hadoopConf,
- tableIdentifier);
+ tableIdentifier,
+ closeableGroup);
Review Comment:
no need to pass the `closeableGroup`. Why not just
```
--- a/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
+++ b/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
@@ -217,24 +217,30 @@ public class GlueCatalog extends BaseMetastoreCatalog
// FileIO initialization depends on tableSpecificCatalogProperties,
so a new FileIO is
// initialized each time
- return new GlueTableOperations(
- glue,
- lockManager,
- catalogName,
- awsProperties,
- tableSpecificCatalogPropertiesBuilder.buildOrThrow(),
- hadoopConf,
- tableIdentifier);
+ GlueTableOperations tableOperations =
+ new GlueTableOperations(
+ glue,
+ lockManager,
+ catalogName,
+ awsProperties,
+ tableSpecificCatalogPropertiesBuilder.buildOrThrow(),
+ hadoopConf,
+ tableIdentifier);
+ closeableGroup.addCloseable(tableOperations.io());
+ return tableOperations;
}
- return new GlueTableOperations(
- glue,
- lockManager,
- catalogName,
- awsProperties,
- catalogProperties,
- hadoopConf,
- tableIdentifier);
+ GlueTableOperations tableOperations =
+ new GlueTableOperations(
+ glue,
+ lockManager,
+ catalogName,
+ awsProperties,
+ catalogProperties,
+ hadoopConf,
+ tableIdentifier);
+ closeableGroup.addCloseable(tableOperations.io());
+ return tableOperations;
```
But even then, this only solves it for the case where the `GlueCatalog`
itself is being actively closed by calling `close()`.
If you're running Spark and using `GlueCatalog` and a `FileIO` instance is
being created on an executor node, then there's no backlink anymore and we
don't know when/how to call `close()`.
That's why I suggested to fix this issue similarly to how it was done in
https://github.com/apache/iceberg/pull/7487
--
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]
