-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-51268][SQL] Make TreeNode
lock-free
#50019
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cc @JoshRosen and @cloud-fan |
@@ -67,15 +67,16 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]] | |||
def outputOrdering: Seq[SortOrder] = Nil | |||
|
|||
// Override `treePatternBits` to propagate bits for its expressions. | |||
override lazy val treePatternBits: BitSet = { | |||
private val _treePatternBits = new BestEffortLazyVal[BitSet](() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
treePatternBits
was kept lazy val
in previous PR, do you @cloud-fan remember it was intended or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curiously, Since the treePatternBits
only init once, how it keep the correctness if the expressions transformed with rules?
What changes were proposed in this pull request?
Make
TreeNode
lock-free, by replacelazy val
withBestEffortLazyVal
Why are the changes needed?
In several deadlock issues, we observed that the lock of
TreeNode._hashCode
is the root cause.Does this PR introduce any user-facing change?
no
How was this patch tested?
ci
Was this patch authored or co-authored using generative AI tooling?
no