You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/03/01 04:39:15 UTC
[GitHub] [spark] gabrielenizzoli opened a new pull request #31689: [SPARK-34573][SQL][CORE] avoid global locking in SQLConf object for sqlConfEntries map
gabrielenizzoli opened a new pull request #31689:
URL: https://github.com/apache/spark/pull/31689
### What changes were proposed in this pull request?
In the `SQLConf` object, the `sqlConfEntries` map is globally synchronized (it is a Java `Collections.synchronizedMap`): any operation, including a get, will need to acquire the lock. To avoid having multiple tasks locking on this, a better approach would be to use a map that does not lock on read (like a `ConcurrentHashMap`).
### Why are the changes needed?
Multiple tasks performing something as easy a `DataType.sameType(...)` would be locking on the global `sqlConfEntries` lock of the `Collections.synchronizedMap`.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
No functionality change. Existing unit tests run normally.
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788630818
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135617/
----------------------------------------------------------------
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.
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
[GitHub] [spark] HyukjinKwon commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788656354
Yeah seems good
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA removed a comment on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788477655
**[Test build #135617 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135617/testReport)** for PR 31689 at commit [`4ffa257`](https://github.com/apache/spark/commit/4ffa2576782e410cef4d5cb3d8865362679ff11c).
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-787631796
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788533904
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40196/
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on a change in pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31689:
URL: https://github.com/apache/spark/pull/31689#discussion_r585210921
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -53,22 +53,22 @@ import org.apache.spark.util.Utils
object SQLConf {
- private[sql] val sqlConfEntries = java.util.Collections.synchronizedMap(
- new java.util.HashMap[String, ConfigEntry[_]]())
+ private[sql] val sqlConfEntries =
+ new ConcurrentHashMap[String, ConfigEntry[_]]()
Review comment:
Seems a reasonable fix cc: @srowen @cloud-fan @viirya
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788526952
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40196/
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on pull request #31689: [SPARK-34573][SQL] avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788426186
ok to test
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788655783
Thanks! Merged to master.
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788630818
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135617/
----------------------------------------------------------------
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.
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
[GitHub] [spark] gabrielenizzoli commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
gabrielenizzoli commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788494034
> This change could get away the lock wait you mentioned in the jira? If so, could you describe more in the PR description?
Added more descriptive PR text.
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788513134
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40196/
----------------------------------------------------------------
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.
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
[GitHub] [spark] cloud-fan commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788742467
late LGTM
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788477655
**[Test build #135617 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135617/testReport)** for PR 31689 at commit [`4ffa257`](https://github.com/apache/spark/commit/4ffa2576782e410cef4d5cb3d8865362679ff11c).
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788654024
> @srowen what does this mean? in jenkins the failure is:
The failure is not related to this PR, so it's okay just to ignore it.
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu closed pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
maropu closed pull request #31689:
URL: https://github.com/apache/spark/pull/31689
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on pull request #31689: [SPARK-34573][SQL] avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788427117
This change could get away the lock wait you mentioned in the jira? If so, could you describe more in the PR description?
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31689: [SPARK-34573][SQL][CORE] avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-787631796
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
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
[GitHub] [spark] gabrielenizzoli commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
gabrielenizzoli commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788622499
> Kubernetes integration test status failure
> URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40196/
@srowen what does this mean? in jenkins the failure is:
```
Setting status of 4ffa2576782e410cef4d5cb3d8865362679ff11c to FAILURE with url https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40196/ and message: 'Build finished. '
FileNotFoundException means that the credentials Jenkins is using is probably wrong. Or the user account does not have write access to the repo.
org.kohsuke.github.GHFileNotFoundException: https://api.github.com/repos/apache/spark/statuses/4ffa2576782e410cef4d5cb3d8865362679ff11c {"message":"Not Found","documentation_url":"https://docs.github.com/rest/reference/repos#create-a-commit-status"}
```
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788533904
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40196/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31689: [SPARK-34573][SQL] Avoid global locking in SQLConf object for sqlConfEntries map
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31689:
URL: https://github.com/apache/spark/pull/31689#issuecomment-788627519
**[Test build #135617 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135617/testReport)** for PR 31689 at commit [`4ffa257`](https://github.com/apache/spark/commit/4ffa2576782e410cef4d5cb3d8865362679ff11c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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