xiaoxuandev commented on code in PR #12868:
URL: https://github.com/apache/iceberg/pull/12868#discussion_r2057404607


##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/source/SerializableTableWithSize.java:
##########
@@ -65,8 +66,7 @@ public static Table copyOf(Table table) {
   @Override
   public void close() throws Exception {
     if (serializationMarker == null) {
-      LOG.info("Releasing resources");
-      io().close();
+      LOG.info("Executor-side cleanup: closing deserialized table resources");

Review Comment:
   Thanks for the review! Based on code inspection and some local debugging, 
Iceberg doesn’t explicitly call close() on most FileIO instances (i.e., the 
regular, non-deserialized ones). The exception is S3FileIO, which overrides 
finalize() to invoke close() during garbage collection—and this applies to 
deserialized copies as well. 
   And given that the underlying connection pool maybe shared, we might want to 
consider removing `finalize()` from `S3FileIO` to avoid unintended side effects 
during garbage collection. cc: @rdblue @aokolnychyi 
   
   Side note: finalize() was deprecated in Java 9 due to potential performance 
issues, deadlocks, and unpredictable behavior during GC.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to