singhpk234 commented on code in PR #16031:
URL: https://github.com/apache/iceberg/pull/16031#discussion_r3118376469
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/CommitMetadata.java:
##########
@@ -56,7 +56,7 @@ public static <R, E extends Exception> R withCommitProperties(
ExceptionUtil.castAndThrow(e, exClass);
return null;
} finally {
- COMMIT_PROPERTIES.set(ImmutableMap.of());
+ COMMIT_PROPERTIES.remove();
Review Comment:
The COMMIT_PROPS is already declared as static final, Immutable empty map
https://github.com/apache/iceberg/blob/d4db77be070a780cb54b84a6024d6e4ba095bc1e/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/CommitMetadata.java#L33
it get allocated this memory as soon as the class is loaded
I am unsure how much are we actually saving ? it may just happen we would
not this class `withCommitProperties` api at all and this stays ? and specially
its a driver thread that we are talking of if you are dong multi threaded
commits, i wanna know who how much these thread are ?
I agree with *best practice* PO, it makes sense, but i think as you all
already pointed out i want to know the impact of this change, since this
mention it solves leak.
--
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]
