squito commented on a change in pull request #25951: [SPARK-28917][CORE]
Synchronize access to RDD mutable state.
URL: https://github.com/apache/spark/pull/25951#discussion_r331246353
##########
File path: core/src/main/scala/org/apache/spark/rdd/RDD.scala
##########
@@ -103,6 +103,17 @@ abstract class RDD[T: ClassTag](
_sc
}
+ /**
+ * Lock for all mutable state of this RDD (persistence, partitions,
dependencies, etc.). We do
+ * not use `this` because RDDs are user-visible, so users might have added
their own locking on
+ * RDDs; sharing that could lead to a deadlock.
+ *
+ * One thread might hold the lock on many of these, for a chain of RDD
dependencies; but
+ * because DAGs are acyclic, and we only ever hold locks for one path in
that DAG, there is no
+ * chance of deadlock.
+ */
+ private val stateLock = new SerializableObject()
Review comment:
I agree with both of you, and I'm torn myself -- its a super weird thing to
do, but the fact is you could do it before. Should I break it now? I decided
not to, since it wasn't too hard to keep it working.
> (`partitions_` is `@transient` so it seems clear that it's not to be used
in executors).
I dunno if that is true -- sure that `private var` is transient but the
public method you actually call, `partitions`, it'll check for null and
regenerate the value.
... except, that it turns out that it doesn't work for some built in RDDs,
like ParallelCollectionRDD, because other data it needs are also `transient`.
But a user could have their own custom RDD which it works fine for. I started
to write up a test case like this but decided the test was getting ridiculously
involved for a "feature" we're not even sure we want to support.
> You could make the lock transient and check whether it is null and avoid
the locking on the executor side, but that seems like a marginal gain (few
bytes of serialized data).
doesn't sounds like its worth the added complexity in the code to me.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
