You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "peter-toth (via GitHub)" <gi...@apache.org> on 2025/04/29 16:23:12 UTC
[PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
peter-toth opened a new pull request, #50757:
URL: https://github.com/apache/spark/pull/50757
### What changes were proposed in this pull request?
This PR is an alternative to https://github.com/apache/spark/pull/50029 to identify map stages that contain non-deterministic expressions and set their output deterministic level to `DeterministicLevel.INDETERMINATE`.
The problem with stages whose tasks produce different result during reruns (task retries) is very similar to what has been fixed in https://github.com/apache/spark/pull/22112.
In https://github.com/apache/spark/pull/22112 a map stage without local sort followed by a `RoundRobin` partitioning made the stage "order sensitive", so the output deterministic level of the map stage could become `INDETERMINATE` depending on the previous stage's `UNORDERED` level.
The issue that this PR fixes is when a map stage itself is non-deterministic and so rerunning it might result in different output. Such a stage should be marked non-deterministic and its output deterministic level shoule be `INDETERMINATE`.
### Why are the changes needed?
The PR fixes a correctness issue with task reruns due to fetch failures when the query contains an exhange with a partitioning containing a non-deterministic expression or a deterministic expression whose lineage contains non-deterministic ancestors.
E.g.:
```
SELECT count(DISTINCT r)
FROM (
SELECT rand() AS r FROM range(1000)
)
```
has the following plan:
```
*(3) HashAggregate(keys=[], functions=[count(distinct r#280)], output=[count(DISTINCT r)#279L])
+- Exchange SinglePartition, ENSURE_REQUIREMENTS, [plan_id=180]
+- *(2) HashAggregate(keys=[], functions=[partial_count(distinct r#280)], output=[count#283L])
+- *(2) HashAggregate(keys=[r#280], functions=[], output=[r#280])
+- Exchange hashpartitioning(r#280, 5), ENSURE_REQUIREMENTS, [plan_id=175]
+- *(1) HashAggregate(keys=[knownfloatingpointnormalized(normalizenanandzero(r#276)) AS r#280], functions=[], output=[r#280])
+- *(1) Project [rand(5640666616196273984) AS r#276]
+- *(1) Range (0, 1000, step=1, splits=2)
```
If there is a fetch failure in the second map stage then the a task of the first map stage needs to rerun and it might return a different result to its first run. As the `Exchange` between the first and second map stage uses an expression (`r`) that has a non-deterministic expression (`rand()`) in its linage, the output of the task rerun might end up in a different partition in the second stage, which could lead to correctness issue in the case only the failed task of the second stage was reexecuted.
By setting the output deterministic level of the first stage to `INDETERMINATE`, all the tasks of the second stage (and all tasks of succeeding stages) will be reexecuted, which fixes the correctness issue.
Please note that:
- `rand()` is not a good example here because it got seeded during planning, so it is more like an order sensitive function. But currently it is hard split non-deterministic SQL expressions to order sensitive or non-deterministic in terms of stage output levels. Mapping all non-deterministic SQL expressions to non-deterministic stage output levels keeps us on the safe side.
- If `rand()` didn't contribute to any `Exchange` partitioning then most likely we wouldn't face any correctness issue even witout marking the first stage non-deterministic. But this PR priorizes simplicity and more sophisticated options can be implemented in follow-up tickets.
### Does this PR introduce _any_ user-facing change?
Yes, it fixes a correctness issue.
### How was this patch tested?
This correctness issue is very hard to unit test as it can come up only when there is a fetch failure. The unit test I added only tests whether the right stages are marked non-deterministic.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2848429985
Thank you for clarifying that Mridul.
On Fri, May 2, 2025, 8:57 PM Asif Shahid ***@***.***> wrote:
> I see... Some how I remembered date related SQL functions to be
> inDerterministic, and not allowing projects to collapse etc..
> Anyways , I suppose intent is conveyed..
>
> On Fri, May 2, 2025, 8:14 PM Mridul M ***@***.***> wrote:
>
>> ***@***.**** commented on this pull request.
>> ------------------------------
>>
>> In core/src/main/scala/org/apache/spark/rdd/MapPartitionsRDD.scala
>> <https://github.com/apache/spark/pull/50757#discussion_r2072300212>:
>>
>> > */
>> private[spark] class MapPartitionsRDD[U: ClassTag, T: ClassTag](
>> var prev: RDD[T],
>> f: (TaskContext, Int, Iterator[T]) => Iterator[U], // (TaskContext, partition index, iterator)
>> preservesPartitioning: Boolean = false,
>> isFromBarrier: Boolean = false,
>> - isOrderSensitive: Boolean = false)
>> + isOrderSensitive: Boolean = false,
>> + isNonDeterministic: Boolean = false)
>>
>> Extend and override getOutputDeterministicLevel instead
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <https://github.com/apache/spark/pull/50757#pullrequestreview-2813131166>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AC6XG2EEDUV7RKGNQSPETWD24QX7TAVCNFSM6AAAAAB4DOZR32VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQMJTGEZTCMJWGY>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>>
>
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2848539891
I see many new comments on this PR (some even seem to be unrelated), but my understanding after https://github.com/apache/spark/pull/50757#issuecomment-2844972082 is that we wil do runtime shuffle checksum validation to determine if a stage behaves non-deterministically, so this PR (and https://github.com/apache/spark/pull/50029) which aims to mark a stage non-deterministic based on related DF query plans/expressions is not needed.
Actually what we could probably do is to mark a stage deterministic based on the query plan to avoid shuffle checksum computation on surely deterministic stages to save same costs (https://github.com/apache/spark/pull/50757#issuecomment-2845349959). This is because we know deterministic behaviour of stages assembled from a DF query, but we don't know it when stages are created via RDD API.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2847985465
My knowledge is limited , so pls bear with me, I thought there would be a result stage after every map stage implying that there can be no two consequent Map stages. And a ResultStage is always committed ( i.e cannot be lost or rolled back once completed successfully, IIUC). So should not a ResultStage sort of break the chain of inDeterminancy , even with the current code where RDDS get their determinancy flag from parent RDDs across stages...? An RDD emanating out of a ResultStage should not ever be inDeterminate..if I understand correctly.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2069316385
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
@ahshahid Regarding point 3 I was open for your change and asked you to extend your integration test with some extra logs to prove it is testing a specific case where I have seen problems. But that got stuck there: https://github.com/attilapiros/spark/pull/8#issuecomment-2797246777
Regarding point 5. I see value a lot of value in the `inDeterministic` flag. As currently we cannot distinguish whether a shuffle map stage indeterministic because of its parent or on its own. Let's say `ShuffleMapStageX` is indeterministic because of its operation and along the way to a result stage there is another `ShuffleMapStageY` which is only indeterministic as it is descendent of `ShuffleMapStageX` but the result stage is fetching from `ShuffleMapStageY` when the fetch failure happens we are still have a deterministic output so even if the result stage is half ready we can continue our work without reverting its output. (In addition latter it would make sense to extend the RDD API to let a user set it when they are using indeterminate operation in the map/flatMap body.)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2070581649
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -103,13 +103,21 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
AttributeSet(expressions) -- producedAttributes)
/**
- * Returns true when the all the expressions in the current node as well as all of its children
+ * Returns true when the current node (all the expressions in it) is deterministic.
+ */
+ def deterministicNode: Boolean = _deterministicNode()
+
+ private val _deterministicNode =
+ new BestEffortLazyVal[JBoolean](() => expressions.forall(_.deterministic))
+
+ /**
+ * Returns true when all the expressions in the current node as well as all of its children
* are deterministic
*/
def deterministic: Boolean = _deterministic()
- private val _deterministic = new BestEffortLazyVal[JBoolean](() =>
- expressions.forall(_.deterministic) && children.forall(_.deterministic))
+ private val _deterministic =
+ new BestEffortLazyVal[JBoolean](() => deterministicNode && children.forall(_.deterministic))
Review Comment:
I did not want to broach Union thing , ie. fraught with inconsistency not only in this aspect, but even canonicalization aspect etc...
But the Union issue you are mentioning can be easily handled, regrading the hasInDeterminism by considering all legs... so I dont think that is a blocker.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2839515234
@cloud-fan , could you please take a look at this PR? It is a very similar to your https://github.com/apache/spark/pull/22112
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2844972082
My worry is that the `Expression#deterministic` is a bit abused in Spark, e.g. `SparkPartitionID`, `InputFileName` are marked as nondeterministic, but they produce the same result when retry. Full stage retry is very expensive, and triggering it unnecessarily is also a regression for Spark users. This is why I'm pushing for runtime shuffle checksum: https://github.com/apache/spark/pull/50230
A workaround to this issue is to disable Spark stage retry, and add job retry at the application side. We can document it as a known issue if needed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2848374385
one more thing to note is that it would be quite common to current date, current time sql functions which are inDeterministic , as filters, and if they exist at base level, it could potentially make whole query plan inDeterministic with expensive retries...
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2067064628
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -103,13 +103,21 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
AttributeSet(expressions) -- producedAttributes)
/**
- * Returns true when the all the expressions in the current node as well as all of its children
+ * Returns true when the current node (all the expressions in it) is deterministic.
+ */
+ def deterministicNode: Boolean = _deterministicNode()
+
+ private val _deterministicNode =
+ new BestEffortLazyVal[JBoolean](() => expressions.forall(_.deterministic))
+
+ /**
+ * Returns true when all the expressions in the current node as well as all of its children
* are deterministic
*/
def deterministic: Boolean = _deterministic()
- private val _deterministic = new BestEffortLazyVal[JBoolean](() =>
- expressions.forall(_.deterministic) && children.forall(_.deterministic))
+ private val _deterministic =
+ new BestEffortLazyVal[JBoolean](() => deterministicNode && children.forall(_.deterministic))
Review Comment:
From what I understand from your simplified PR ( & assuming I understood it correctly), you are moving the nonDeterministic computation indicator from expression to Plan Level..
I would limit the notion of non deterministic computation indicator to expression itself..and keep the flag at expression level itself , rather than Plan Level..
1) Primarily, because a Plan is inDeterministic due to its expression. Consider
Would not that mean that a Plan node is inDeterministic even if itself does not utilize any non-deterministic computation , from a node below?
2) Most of the times , in tree manipulation, the plan nodes are cloned, the expressions retain same reference. so there is more likleyhood of unncessary tree traversal if the non-determinictic computation flag is kept in the Plan level.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2845342978
IMHO the issue of inDeterministic value of an expression should be looked only from the basis of whether ShuffleStage can loose/add row because of inDeterministic nature of the expression.. Beyond that if we are doing anything , then that is sort of trying to put an order/ or expected outcome to an inDeterministic expression's value, which is a contradiction to the meaning of inDeterminancy.
If Roundrobin partitioning is causing trouble, then that is not an issue of inDeterminancy, as if it was, then we are expecting a certain expected behaviour from an inDeterminancy, which is contradicting the definition.
Also, to know if an stage is inDeterminate, it need not look into the entire QueryPlan tree within the stage, that should be based on the output expressions of the stage ( whether any expression is using an inDtereminant component) and only restricted to whether stage is making use of that inDeterminant component in partitioning.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2845350793
I also doubt that user would be able to specify the inDeterminancy flag in the Map operations as I think it is going to make it complicated for user to understand its impact, and if inadvertently to feel safe if he /she marks it as inDeterminate, the cost would be very high in terms of unnnecessary retry. When after going inside the code, we are struggling with the interpretation / behaviour of inDeterminancy, expecting user to do that is a big ask.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2845812865
I am out right now.. will check back.. but what I m trying to say is that a
map stage should be marked inDeterminate iff the partitioner is using
indeterministic Val. And afaik only SQL has metadata to convey that info...
I don't know in what situation a non SQL rdd can get that info and that too
correctly.
On Thu, May 1, 2025, 2:12 PM Attila Zsolt Piros ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> On core/src/main/scala/org/apache/spark/rdd/RDD.scala
> <https://github.com/apache/spark/pull/50757#discussion_r2070814828>:
>
> Please check this lines in RDD.scala:
>
> https://github.com/apache/spark/blob/085bfc21ce0e162eb15257649ec1a3dc6f25c5be/core/src/main/scala/org/apache/spark/rdd/RDD.scala#L2111-L2114
>
> This is where INDETERMINATE level is inherited from one stage to another.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/spark/pull/50757#discussion_r2070814828>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AC6XG2HINSI2JX6BTVEQSMD24KE4TAVCNFSM6AAAAAB4DOZR32VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQMJQG42DANRYGM>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2847859035
@attilapiros : I get what you are pointing out in RDD code...
Its committers call.
My view is :
1) A inDeterministic expression is something which is not predictable and no order of any form should be expected from it, whether read once or multiple times.
2) The only requirement from spark side should be that if a Partitioner is using that inDeterministic component, then it should not loose/add extra row during retry.
If we stick to above 2 requirements,
1) then the Shuffle Stage / or any stage, should just consult the RDDs which it has to see if its deterministic or not.
2) An RDD's determinism shoiuld be just based on nature of the Partitioner it has. If the Partitioner used by RDD is using an inDeterministic expression , then it should be marked as inDeterminate ( and as of now AFAIK, its possible only in SQL Layer's RDD where the Partitioner has that info...)
3) In case of core , from what I have understood the problem is related to Round Robin Partitioning logic... Now that ideally is a separate issue and should not have been tied to the inDeterminism... AFAIk if round robin issue is not mixed with inDeterminancy , there is no way an RDD can have inDeterminancy as true ( nor is it needed).
And I might be off the mark, but if Round Robin Partitioning issue needs to be resolved by piggy backing on inDeterminancyy logic, then may be the check could be as small as marking any RDD with RoundRobin Partitioner as inDeterminate should solve the issue , as what is needed is retry of all partitions in that case.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2070564298
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
> @ahshahid Regarding point 3 I was open for your change and asked you to extend your integration test with some extra logs to prove it is testing a specific case where I have seen problems. But that got stuck there: [attilapiros#8 (comment)](https://github.com/attilapiros/spark/pull/8#issuecomment-2797246777)
> Let me re-read your comments to understand what you are hinting, but I dont see how that PR which is specifically exposing the race condition , relevant to this PR.. The question of race condition which the other PR is mentioning, will not even arise if stage always shows determinancy as true..
> Regarding point 5. I see value a lot of value in the `inDeterministic` flag. As currently we cannot distinguish whether a shuffle map stage indeterministic because of its parent or on its own. Let's say `ShuffleMapStageX` is indeterministic because of its operation and along the way to a result stage there is another `ShuffleMapStageY` which is only indeterministic as it is descendent of `ShuffleMapStageX` but the result stage is fetching from `ShuffleMapStageY` when the fetch failure happens we are still have a deterministic output so even if the result stage is half ready we can continue our work without reverting its output. (In addition latter it would make sense to extend the RDD API to let a user set it when they are using indeterminate operation in the map/flatMap body.)
In my PR, A Shuffle Map stage is considered inDeterministic IFF it is using the inDeterministic value as partitioner. In the above example, if ShuffleMapStage Y is using the inDeterminate output originally created in ( ShuffleMapStage X), then only the stage Y is considered inDeterministic. and that is the right behaviour.
If ShuffleStage Y is not using inDeterminate expression as partitioner, then no outputs need to be discraded of result stage.. So the problem you mention does not exist.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2845349959
Thanks @cloud-fan , somehow I missed that PR. Runtime shuffle checksum seems like a good idea, but it must come with some costs as well.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2070588096
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -103,13 +103,21 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
AttributeSet(expressions) -- producedAttributes)
/**
- * Returns true when the all the expressions in the current node as well as all of its children
+ * Returns true when the current node (all the expressions in it) is deterministic.
+ */
+ def deterministicNode: Boolean = _deterministicNode()
+
+ private val _deterministicNode =
+ new BestEffortLazyVal[JBoolean](() => expressions.forall(_.deterministic))
+
+ /**
+ * Returns true when all the expressions in the current node as well as all of its children
* are deterministic
*/
def deterministic: Boolean = _deterministic()
- private val _deterministic = new BestEffortLazyVal[JBoolean](() =>
- expressions.forall(_.deterministic) && children.forall(_.deterministic))
+ private val _deterministic =
+ new BestEffortLazyVal[JBoolean](() => deterministicNode && children.forall(_.deterministic))
Review Comment:
The points which I mentioned hold true or not is what I am trying to validate..
The cost of calculating if a QueryPlan node is inDeterministic or not will trickle down to the leaf... and because at least in Planner stage , its Plan's which are manipulated, the inDeterrministic identification will require tree travesal to leaf ... In case of Expression's , it does not cross the QyeryPlan node boundary.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2070814828
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
Please check this lines in RDD.scala:
https://github.com/apache/spark/blob/085bfc21ce0e162eb15257649ec1a3dc6f25c5be/core/src/main/scala/org/apache/spark/rdd/RDD.scala#L2111-L2114
This is where INDETERMINATE level is inherited from one stage to another.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2069193802
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
IMO, this is going to create a lot of potential issues... especially related to performance.
IMO, this is sort of attempting to provide predictable behaviour for an inDeterminate value which is not worth the trouble.
The perf issues which could potentially arise are:-
1) As mentioned in previous comments, QueryPlans willl be more frequently copied in tree traversal, causing extra cost for flag determination
2) A node which may not be affected at all or may not utilize the inDeterminate expression, would become inDeterminate with cascading effect
3) Given that there is already a race condition , which I have identified in Dag Scheduler, of which @attilapiros suggestion is to deal aggresively by failing queries instead of retry, more queries ( & mostly unnecessarily) will fail instead of retries.
4) If I understand the RoundRobin partitioning issue correctly, I have a feeling that , it can be solved just on the lines of how HashPartition containing inDeterminate expression is handled ( in my PR for 515016), i.e by treating RoundRobin Partitioning as inDeterministic.
5) Making changes in MapRDD etc which is in core layer, with extra inDeterministic flag is going to confuse and unusable in terms of spark's core, because only SQL has metadata to tell RDD that its behaviour can be inDeterministic. in core RDD , a user can write any transformation which might have inDeterminancy , but the MapRDD's flag would convey wrong info.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2069198691
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
My take is not to try to give any order / predictable behaviour to an inDeterministic expression and just solve the root issue... that is rows do not get missed/added due to inDtereminancy created by Partitioner and limit its effect to the ShuffleStage's RDD only. and make the RoundRobin partitioning also inDeterministic to solve the round robin problem ( if I understood it correctly).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2069403852
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -103,13 +103,21 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
AttributeSet(expressions) -- producedAttributes)
/**
- * Returns true when the all the expressions in the current node as well as all of its children
+ * Returns true when the current node (all the expressions in it) is deterministic.
+ */
+ def deterministicNode: Boolean = _deterministicNode()
+
+ private val _deterministicNode =
+ new BestEffortLazyVal[JBoolean](() => expressions.forall(_.deterministic))
+
+ /**
+ * Returns true when all the expressions in the current node as well as all of its children
* are deterministic
*/
def deterministic: Boolean = _deterministic()
- private val _deterministic = new BestEffortLazyVal[JBoolean](() =>
- expressions.forall(_.deterministic) && children.forall(_.deterministic))
+ private val _deterministic =
+ new BestEffortLazyVal[JBoolean](() => deterministicNode && children.forall(_.deterministic))
Review Comment:
`Expression.hasIndeterminism` kind of tracking in https://github.com/apache/spark/pull/50029 seems fragile to me because plan nodes can create new attributes and those should have their own `hasIndeterminism` logic implemented.
E.g. if you check `UnionBase` then you will see that its output attributes are either inherited from the first leg or a new `AttributeReference` is created, but in both cases they should take into account `hasIndeterminism` of all legs, which is missing from https://github.com/apache/spark/pull/50029.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2848206602
> @ahshahid
>
> > I thought there would be a result stage after every map stage implying that there can be no two consequent Map stages.
>
> No, a job always have one result stage and can have zero or more shuffle map stages.
>
> Whenever you have question like this you can run a simple spark job and check the logging or in this case even the UI.
>
> ```
> $ ./bin/spark-shell
> scala> sc.setLogLevel("DEBUG")
> scala> sc.parallelize(1 to 100, 10).groupBy(i => i).map(_._1).groupBy(i=>i).map(_._1).collect
> ...
> 25/05/02 14:13:32 DEBUG DAGScheduler: submitStage(ResultStage 2 (name=collect at <console>:24;jobs=0))
> 25/05/02 14:13:32 DEBUG DAGScheduler: missing: List(ShuffleMapStage 1)
> 25/05/02 14:13:32 DEBUG DAGScheduler: submitStage(ShuffleMapStage 1 (name=groupBy at <console>:24;jobs=0))
> 25/05/02 14:13:32 DEBUG DAGScheduler: missing: List(ShuffleMapStage 0)
> 25/05/02 14:13:32 DEBUG DAGScheduler: submitStage(ShuffleMapStage 0 (name=groupBy at <console>:24;jobs=0))
> 25/05/02 14:13:32 DEBUG DAGScheduler: missing: List()
> ...
> ```
Thanks @attilapiros for example to validate and answering the question.
I was also wondering why an InputAdapter's RDD (in case of SQL, which would represent the leaf after every Shuffle stage..right?) , should not always have Determinancy as true, which should break the chain... What value / usecase requires the inDeterminancy flag to be carried over the stage boundaries...
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-3186267672
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2069998646
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
1., 2. This PR doesn't change deterministic calculation of plan nodes so it shouldn't cause more frequent plan copies.
4. IMO the point of https://github.com/apache/spark/pull/22112 is that RoundRobin partitioning is not necessairly non-deterministic, it is just order sensitive (RDD.repartition() or DF.repartition()+`spark.sql.execution.sortBeforeRepartition=false`), so the deterministic level of a stage that is followed by a RoundRobin partitioning depends on the previous stage.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2067052580
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
From what I saw earlier, previously objections were raised for changes in core..
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2069390537
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -103,13 +103,21 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
AttributeSet(expressions) -- producedAttributes)
/**
- * Returns true when the all the expressions in the current node as well as all of its children
+ * Returns true when the current node (all the expressions in it) is deterministic.
+ */
+ def deterministicNode: Boolean = _deterministicNode()
+
+ private val _deterministicNode =
+ new BestEffortLazyVal[JBoolean](() => expressions.forall(_.deterministic))
+
+ /**
+ * Returns true when all the expressions in the current node as well as all of its children
* are deterministic
*/
def deterministic: Boolean = _deterministic()
- private val _deterministic = new BestEffortLazyVal[JBoolean](() =>
- expressions.forall(_.deterministic) && children.forall(_.deterministic))
+ private val _deterministic =
+ new BestEffortLazyVal[JBoolean](() => deterministicNode && children.forall(_.deterministic))
Review Comment:
This PR doesn't change `deterministic()` calculation of plan nodes, just extracts part of that logic to a new `deterministicNode()` method to check a node itself introduces indeterminism.
That method is then used in `ShuffleExchangeExec.isDeterministicStage()` to check any of the descendant nodes below a `ShuffleExchangeExec` until the next exchange (stage boundary) introduces indeterminism.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2072300212
##########
core/src/main/scala/org/apache/spark/rdd/MapPartitionsRDD.scala:
##########
@@ -35,13 +35,15 @@ import org.apache.spark.{Partition, TaskContext}
* @param isOrderSensitive whether or not the function is order-sensitive. If it's order
* sensitive, it may return totally different result when the input order
* is changed. Mostly stateful functions are order-sensitive.
+ * @param isNonDeterministic Whether the function is non-deterministic.
*/
private[spark] class MapPartitionsRDD[U: ClassTag, T: ClassTag](
var prev: RDD[T],
f: (TaskContext, Int, Iterator[T]) => Iterator[U], // (TaskContext, partition index, iterator)
preservesPartitioning: Boolean = false,
isFromBarrier: Boolean = false,
- isOrderSensitive: Boolean = false)
+ isOrderSensitive: Boolean = false,
+ isNonDeterministic: Boolean = false)
Review Comment:
Extend and override `getOutputDeterministicLevel` instead
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2068884836
##########
core/src/main/scala/org/apache/spark/rdd/RDD.scala:
##########
Review Comment:
Yeah, but I would argue that this is very similar to https://github.com/apache/spark/pull/22112 where `isOrderSensitive` was introduced.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #50757: [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level
URL: https://github.com/apache/spark/pull/50757
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "attilapiros (via GitHub)" <gi...@apache.org>.
attilapiros commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2848147158
> I thought there would be a result stage after every map stage implying that there can be no two consequent Map stages.
No, a job always have one result stage and can have zero or more shuffle map stages.
Whenever you have question like this you can run a simple spark job and check the logging or in this case even the UI.
```
$ ./bin/spark-shell
scala> sc.setLogLevel("DEBUG")
scala> sc.parallelize(1 to 100, 10).groupBy(i => i).map(_._1).groupBy(i=>i).map(_._1).collect
...
25/05/02 14:13:32 DEBUG DAGScheduler: submitStage(ResultStage 2 (name=collect at <console>:24;jobs=0))
25/05/02 14:13:32 DEBUG DAGScheduler: missing: List(ShuffleMapStage 1)
25/05/02 14:13:32 DEBUG DAGScheduler: submitStage(ShuffleMapStage 1 (name=groupBy at <console>:24;jobs=0))
25/05/02 14:13:32 DEBUG DAGScheduler: missing: List(ShuffleMapStage 0)
25/05/02 14:13:32 DEBUG DAGScheduler: submitStage(ShuffleMapStage 0 (name=groupBy at <console>:24;jobs=0))
25/05/02 14:13:32 DEBUG DAGScheduler: missing: List()
...
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2848394126
There are not indeterminate @ahshahid - they are constant folded, and based on the time of execution of the query.
This is not to say there cant be indeterminate expressions.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2848411912
I see... Some how I remembered date related SQL functions to be
inDerterministic, and not allowing projects to collapse etc..
Anyways , I suppose intent is conveyed..
On Fri, May 2, 2025, 8:14 PM Mridul M ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In core/src/main/scala/org/apache/spark/rdd/MapPartitionsRDD.scala
> <https://github.com/apache/spark/pull/50757#discussion_r2072300212>:
>
> > */
> private[spark] class MapPartitionsRDD[U: ClassTag, T: ClassTag](
> var prev: RDD[T],
> f: (TaskContext, Int, Iterator[T]) => Iterator[U], // (TaskContext, partition index, iterator)
> preservesPartitioning: Boolean = false,
> isFromBarrier: Boolean = false,
> - isOrderSensitive: Boolean = false)
> + isOrderSensitive: Boolean = false,
> + isNonDeterministic: Boolean = false)
>
> Extend and override getOutputDeterministicLevel instead
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/spark/pull/50757#pullrequestreview-2813131166>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AC6XG2EEDUV7RKGNQSPETWD24QX7TAVCNFSM6AAAAAB4DOZR32VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQMJTGEZTCMJWGY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "ahshahid (via GitHub)" <gi...@apache.org>.
ahshahid commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2845911576
A stage is inDerterministic or not should not depend on it's parent stage
but only on the behaviour of the partitioner of the current stage.
On Thu, May 1, 2025, 2:23 PM Asif Shahid ***@***.***> wrote:
> I am out right now.. will check back.. but what I m trying to say is that
> a map stage should be marked inDeterminate iff the partitioner is using
> indeterministic Val. And afaik only SQL has metadata to convey that info...
> I don't know in what situation a non SQL rdd can get that info and that too
> correctly.
>
> On Thu, May 1, 2025, 2:12 PM Attila Zsolt Piros ***@***.***>
> wrote:
>
>> ***@***.**** commented on this pull request.
>> ------------------------------
>>
>> On core/src/main/scala/org/apache/spark/rdd/RDD.scala
>> <https://github.com/apache/spark/pull/50757#discussion_r2070814828>:
>>
>> Please check this lines in RDD.scala:
>>
>> https://github.com/apache/spark/blob/085bfc21ce0e162eb15257649ec1a3dc6f25c5be/core/src/main/scala/org/apache/spark/rdd/RDD.scala#L2111-L2114
>>
>> This is where INDETERMINATE level is inherited from one stage to another.
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <https://github.com/apache/spark/pull/50757#discussion_r2070814828>, or
>> unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AC6XG2HINSI2JX6BTVEQSMD24KE4TAVCNFSM6AAAAAB4DOZR32VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQMJQG42DANRYGM>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>>
>
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51016][SQL] Non-deterministic SQL expressions should set indeterminate map stage output level [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #50757:
URL: https://github.com/apache/spark/pull/50757#issuecomment-2850902956
Skipping shuffle checksum is a good idea if we know the map stage is deterministic for sure.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org