dtarima commented on code in PR #45181:
URL: https://github.com/apache/spark/pull/45181#discussion_r1521457462
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -193,10 +193,40 @@ private[sql] object Dataset {
*/
@Stable
class Dataset[T] private[sql](
- @DeveloperApi @Unstable @transient val queryExecution: QueryExecution,
+ @DeveloperApi @Unstable @transient val queryUnpersisted: QueryExecution,
@DeveloperApi @Unstable @transient val encoder: Encoder[T])
extends Serializable {
+ private var queryPersisted: Option[(Array[Boolean], QueryExecution)] = None
+
+ def queryExecution: QueryExecution = {
+ val cacheStatesSign = queryUnpersisted.computeCacheStateSignature()
+ // If all children aren't cached, directly return the queryUnpersisted
+ if (cacheStatesSign.forall(b => !b)) {
Review Comment:
> I don't think making queryExecution wrapped by AtomicReference means it's
thread-safe. For example, we unpersist one of it's children in another thread,
and at meanwhile we call ds.count(), the cache consistency may be incorrect.
Yes, consistency of results cannot be guaranteed when persistence state
changes concurrently in different threads, but this is not what I was pointing
to. Thread safety is a basic concept, not related to business logic: when we
change a `var` in one thread, other threads might not see the updated
reference. In order to avoid it the reference needs to be marked `volatile`. In
the example above I used AtomicReference's `set` for simplicity, but it might
make sense to implement it using `compareAndSet` to get additional guarantees.
> Using 2 queryExecution variables may help reduce count of analysis.
I doubt that the additional complexity worth it. It's not a big deal...
Let's see what reviewers think.
--
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]
