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* POV, it makes sense, and i am not opposed to
this change, though i would like to understand this better as even when we call
remove(), this will call thread local's intial value which is
'ImmutableMap.of()` which is something we are already doing
```
Removes the current thread's value for this thread-local variable. If this
thread-local variable is subsequently read by the current thread, its value
will be reinitialized by invoking its initialValue() method, unless its value
is set by the current thread in the interim. This may result in multiple
invocations of the initialValue method in the current thread.
```
--
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]
