uros-b commented on code in PR #56584:
URL: https://github.com/apache/spark/pull/56584#discussion_r3444119046
##########
mllib/src/main/scala/org/apache/spark/ml/Model.scala:
##########
@@ -57,5 +57,19 @@ abstract class Model[M <: Model[M]] extends Transformer {
self =>
* 4, For 3-rd extension, if external languages are used, it is recommended
to override
* this method and return a proper size.
*/
- private[spark] def estimatedSize: Long = SizeEstimator.estimate(self)
+ private[spark] def estimatedSize: Long = {
+ // SPARK-57521: Temporarily clear the parent reference during size
estimation.
+ // After fit(), the parent estimator may retain indirect references to the
SparkSession
+ // (via closures or query plan state from DataFrame operations executed
during fit).
+ // SizeEstimator traverses the entire reachable object graph, causing it
to count
+ // shared SparkSession state as part of every model's size.
+ // The parent is @transient (not persisted) and is not needed for
transform() or save().
+ val savedParent = parent
+ parent = null
Review Comment:
Please investigate and address a possible thread-safety regression here.
There is a side-effecting mutation of shared state in a base-class method. Two
concurrent estimatedSize calls on the same model can interleave so both save
then both restore, with the second finally clobbering parent to null
permanently; a concurrent reader (hasParent, transform, save) can also observe
parent == null during the window.
In the current path this is masked because estimatedSize is invoked inside
MLCache.register (which is synchronized) on a freshly-fit, not-yet-shared
model, so it is not an active production bug today, but estimatedSize is
private[spark] and the previous implementation was side-effect free, so the new
contract is strictly weaker.
Please consider a non-mutating approach rather than mutating shared instance
state.
--
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]
