[SPARK-51016][SQL] Fix for incorrect results on retry for Left Outer Join with indeterministic join keys

[ahshahid]

What changes were proposed in this pull request?

1. Added a new lazy boolean exprValHasIndeterministicCharacter in the Expression class, to indicate whether the expression's evaluation result was obtained using some indeterministic calculation.

The main difference between existing deterministic boolean and this is , that the former determines if same expression is evaluated more than once in a given iteration would its value change or not. Also on that basis, any leaf expression would always be deterministic.

But the latter indicates if an expression's evaluation used/uses any non deterministic component. So here even the leaf expression ( like Attribute) can have non deterministic character flag as true, if its pointing to some result obtained via indeterministic calculation.

Apart from that code of ShuffleDependency is augmented to indicate if the hashing expression is using any non deterministic component.

2. In the DagScheduler and Stage, the actions of successful task completion is done in a Read Lock for the active Stage. While the actions of Failure task, which result in retry of the stage's task are done under Write lock for that stage.

Because ReadWriteLocks are not upgrdable ( from read to write), and in a particular case of successful task completion , the thread owning the Read Lock in Stage, invokes the code of making a new Stage attempt, seeking Write Lock, a ThreadLocal is introduced in the Stage class, which keeps track of Threads which have already taken a Read Lock, so that they do not attempt to acquire Write Lock

3. With the above changes, it turns out that two existing tests have wrong assertions , IMO, as all partitions are not subjected to be retried.

Why are the changes needed?

There are 3 issues which this PR is addressing:

1. The method Stage.isIndeterminate is not returning true in case the ShuffleDependency is using Partitioner based on an inDeterministic Attribute. The bug is at Stage and RDD level. An expression which is using an Attribute derived from an inDeterministic Expression, looses the information, that its Output is using an InDeterministic component.

2. Once the above issue is fixed, that exposes a race condition, where a successful task completion concurrent with a task failure , for an inDeterminate stage, results in a situation , where instead of re-executing all partitions, only some are retried. This results in data loss.
   The race condition identified is as follows:
   a) A successful result stage task, is yet to mark in the boolean array tracking partitions success/failure as true/false.
   b) A concurrent failed result task, belonging to an InDeterminate stage, idenitfies all the stages which needs/ can be rolled back. For Result Stage, it looks into the array of successful partitions. As none is marked as true, the ResultStage and dependent stages are delegated to thread pool for retry.
   c) Between the time of collecting stages to rollback and re-try of stages, the successful task marks boolean as true.
   d) The Retry of Stage, as a result, misses the partition marked as successful, for retry.

3. An existing test, has incorrect assertions regarding the number of partitions being retried , for an inDeterminate stage.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added unit tests / Bug test to validate the behaviour of InDeterministic Stages
Have a functional test which exposes following:

1. Data Loss due to buggy Stage.isInDeterminate
2. Race condition causing Data Loss , even if above 1) is fixed.

I am attaching 2 files for bug test

1. bugrepro.patch
   This is needed to coax the single VM test to reproduce the issue. It has lots of interception and tweaks to ensure that system is able to hit the data loss situation.
   ( like each partition writes only a shuffle file containing keys evaluating to same hashCode and deleting the shuffle file at right time etc)
2. The BugTest itself.

a) If the bugrepro.patch is applied to current master and the BugTest run, it will fail immediately with assertion failure where instead of 12 rows, 6 rows show up in result.

b) If the bugrepro.patch is applied to this bug fix branch , but commit taken till 790aaf0, which only contains the fix for Stage.isInDeterminate , then the BugTest will fail after one or two or more iterations, indicating the race condition in DataScheduler/Stage interaction.

c) But if the same BugTest is run on latest commit, it will pass in all the 100 iteration.
bugrepro.patch
BugTest.txt

Was this patch authored or co-authored using generative AI tooling?

No