[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-22 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-950035138 Thanks @baibaichen and @chenzhx for addressing my comment. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-21 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-949302897 Otherwise, I am going to propose revert this because this patch apparently has not been tested. -- This is an automated message from the Apache Git Service. To respond to

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-21 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-949108489 ping @chenzhx, please update how you tested this in the PR description. -- This is an automated message from the Apache Git Service. To respond to the message, please log

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-19 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-946796710 @chenzhx please describe how you tested this patch, and what user facing behaviour was made on the deadlock. -- This is an automated message from the Apache Git Service.

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-19 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-946795548 I understand the change and I agree there's no issue on the change itself but we should probably keep the PR description correct. It has no test and at least it should

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-19 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-946791600 This PR fixes the deadlock issue as the PR description explains, and it should explain how this patch was tested on the deadlock issue. -- This is an automated message

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-19 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-946738713 Yes I checked. Does the existing test cases cover this change? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-19 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-946699776 Wenchen, please don't merge a PR when there is a standing comment. Existing unittedts don't cover this change -- This is an automated message from the Apache Git

[GitHub] [spark] HyukjinKwon commented on pull request #34292: [SPARK-37017][SQL] Reduce the scope of synchronized to prevent potential deadlock

2021-10-18 Thread GitBox
HyukjinKwon commented on pull request #34292: URL: https://github.com/apache/spark/pull/34292#issuecomment-946325105 Please elabourate problem and address my standing comment: https://github.com/apache/spark/pull/34292#issuecomment-945235842. -- This is an automated message from the