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/02/23 09:05:06 UTC
[GitHub] [spark] gaborgsomogyi opened a new pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
gaborgsomogyi opened a new pull request #31622:
URL: https://github.com/apache/spark/pull/31622
### What changes were proposed in this pull request?
Some of the built-in JDBC connection providers are changing the JVM security context to do the authentication which is fine. The problematic part is that executors can be reused by another query. The following situation leads to incorrect behaviour:
* Query1 opens JDBC connection and changes JVM security context in Executor1
* Query2 tries to open JDBC connection but it realizes there is already an entry for that DB type in Executor1
* Query2 is not changing JVM security context and uses Query1 keytab and principal
* Query2 fails with authentication error
In this PR I've changed to code such a way that JVM security context is changed all the time but only temporarily until the connection built-up and then rolled back. Since `getConnection` is synchronised with `SecurityConfigurationLock` it ends-up in correct behaviour without any race.
### Why are the changes needed?
Incorrect JVM security context handling.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing unit + integration tests.
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785487395
**[Test build #135436 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135436/testReport)** for PR 31622 at commit [`fd923f8`](https://github.com/apache/spark/commit/fd923f82fd3b698279470883c606f201658c9892).
* 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
[GitHub] [spark] AmplabJenkins commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784099752
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39952/
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785488301
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135436/
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785330417
**[Test build #135436 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135436/testReport)** for PR 31622 at commit [`fd923f8`](https://github.com/apache/spark/commit/fd923f82fd3b698279470883c606f201658c9892).
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785339865
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40016/
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784030109
**[Test build #135372 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135372/testReport)** for PR 31622 at commit [`3d77273`](https://github.com/apache/spark/commit/3d7727336403ba0130a4055e03529161c9c2d36e).
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-974926577
Do you mind elaborating how and which code block introduces the bottleneck? The synchronization seems already there before this PR.
--
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
[GitHub] [spark] tdg5 commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-974350953
Help!
It seems like no one considered the reality that some apps may rely on being able to establish many connections simultaneously for performance reasons. This change forces concurrency to 1 when establishing database connections and that strikes me as a **significant** user impacting change.
Can anyone propose a workaround for this? I have an app that makes connections to thousands of databases and I can't upgrade to any version >3.1.x because of this **significant** bottleneck.
https://issues.apache.org/jira/browse/SPARK-37391
ðŸ˜
--
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
[GitHub] [spark] maropu commented on a change in pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31622:
URL: https://github.com/apache/spark/pull/31622#discussion_r581657146
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/DB2ConnectionProvider.scala
##########
@@ -34,15 +35,22 @@ private[sql] class DB2ConnectionProvider extends SecureConnectionProvider {
override def getConnection(driver: Driver, options: Map[String, String]): Connection = {
val jdbcOptions = new JDBCOptions(options)
- setAuthenticationConfigIfNeeded(driver, jdbcOptions)
- UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal, jdbcOptions.keytab)
- .doAs(
- new PrivilegedExceptionAction[Connection]() {
- override def run(): Connection = {
- DB2ConnectionProvider.super.getConnection(driver, options)
+ val parent = Configuration.getConfiguration
+ try {
+ setAuthenticationConfig(parent, driver, jdbcOptions)
+ UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal,
+ jdbcOptions.keytab)
+ .doAs(
+ new PrivilegedExceptionAction[Connection]() {
+ override def run(): Connection = {
+ DB2ConnectionProvider.super.getConnection(driver, options)
+ }
}
- }
- )
+ )
+ } finally {
+ logDebug("Restoring original security configuration")
+ Configuration.setConfiguration(parent)
+ }
Review comment:
It looks like this is a boilerplate code, so could we handle this outside `getConnection`? IIUC, in the current approach, 3rd-party developers need to write the same code in a user-defined provider overriding `ConnectionProvider. getConnection`?
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785330417
**[Test build #135436 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135436/testReport)** for PR 31622 at commit [`fd923f8`](https://github.com/apache/spark/commit/fd923f82fd3b698279470883c606f201658c9892).
----------------------------------------------------------------
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] gaborgsomogyi commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785766049
Thank you all for taking care!
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785484213
Merged to master and branch-3.1.
----------------------------------------------------------------
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] tdg5 commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-975686621
@srowen I believe the problem is actually introduced here: https://github.com/apache/spark/pull/29024/files#diff-345beef18081272d77d91eeca2d9b5534ff6e642245352f40f4e9c9b8922b085R58
sorry for the confusion. I'm working on an update for the JIRA with more detail that should be available in a few minutes.
--
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
[GitHub] [spark] AmplabJenkins commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784235918
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135372/
----------------------------------------------------------------
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] gaborgsomogyi commented on a change in pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31622:
URL: https://github.com/apache/spark/pull/31622#discussion_r581907430
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/DB2ConnectionProvider.scala
##########
@@ -34,15 +35,22 @@ private[sql] class DB2ConnectionProvider extends SecureConnectionProvider {
override def getConnection(driver: Driver, options: Map[String, String]): Connection = {
val jdbcOptions = new JDBCOptions(options)
- setAuthenticationConfigIfNeeded(driver, jdbcOptions)
- UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal, jdbcOptions.keytab)
- .doAs(
- new PrivilegedExceptionAction[Connection]() {
- override def run(): Connection = {
- DB2ConnectionProvider.super.getConnection(driver, options)
+ val parent = Configuration.getConfiguration
+ try {
+ setAuthenticationConfig(parent, driver, jdbcOptions)
+ UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal,
+ jdbcOptions.keytab)
+ .doAs(
+ new PrivilegedExceptionAction[Connection]() {
+ override def run(): Connection = {
+ DB2ConnectionProvider.super.getConnection(driver, options)
+ }
}
- }
- )
+ )
+ } finally {
+ logDebug("Restoring original security configuration")
+ Configuration.setConfiguration(parent)
+ }
Review comment:
Valid concern. While I was doing the changes I've tried to move out getting the parent and restoring the original outside of `getConnection` but the code was kinda' odd. Let me explain why. I see mainly 2 options to do this:
1.
```
val parent = Configuration.getConfiguration
try {
getConnection(driver, options) // <-- Here another Configuration.getConfiguration call is a little bit overkill
} finally {
logDebug("Restoring original security configuration")
Configuration.setConfiguration(parent)
}
```
This option is technically correct because `SecurityConfigurationLock` makes sure obtaining the parent gives back the same result.
2.
```
val parent = Configuration.getConfiguration
try {
getConnection(parent, driver, options) // Creating such API change is also weird
} finally {
logDebug("Restoring original security configuration")
Configuration.setConfiguration(parent)
}
```
If we would like to eliminate this boilerplate stuff I would bet on the first option. WDYT?
Or if you have better idea welcome.
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31622:
URL: https://github.com/apache/spark/pull/31622#discussion_r581938058
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/DB2ConnectionProvider.scala
##########
@@ -34,15 +35,22 @@ private[sql] class DB2ConnectionProvider extends SecureConnectionProvider {
override def getConnection(driver: Driver, options: Map[String, String]): Connection = {
val jdbcOptions = new JDBCOptions(options)
- setAuthenticationConfigIfNeeded(driver, jdbcOptions)
- UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal, jdbcOptions.keytab)
- .doAs(
- new PrivilegedExceptionAction[Connection]() {
- override def run(): Connection = {
- DB2ConnectionProvider.super.getConnection(driver, options)
+ val parent = Configuration.getConfiguration
+ try {
+ setAuthenticationConfig(parent, driver, jdbcOptions)
+ UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal,
+ jdbcOptions.keytab)
+ .doAs(
+ new PrivilegedExceptionAction[Connection]() {
+ override def run(): Connection = {
+ DB2ConnectionProvider.super.getConnection(driver, options)
+ }
}
- }
- )
+ )
+ } finally {
+ logDebug("Restoring original security configuration")
+ Configuration.setConfiguration(parent)
+ }
Review comment:
Ah, I see. Let's use the option 1. for now. Even if it is a development api, I think it is better not to change it as much as possible
----------------------------------------------------------------
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] gaborgsomogyi commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784213003
cc @maropu @HyukjinKwon
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784209797
**[Test build #135372 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135372/testReport)** for PR 31622 at commit [`3d77273`](https://github.com/apache/spark/commit/3d7727336403ba0130a4055e03529161c9c2d36e).
* 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
[GitHub] [spark] AmplabJenkins commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785488301
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135436/
----------------------------------------------------------------
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] gaborgsomogyi commented on a change in pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #31622:
URL: https://github.com/apache/spark/pull/31622#discussion_r581952822
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/DB2ConnectionProvider.scala
##########
@@ -34,15 +35,22 @@ private[sql] class DB2ConnectionProvider extends SecureConnectionProvider {
override def getConnection(driver: Driver, options: Map[String, String]): Connection = {
val jdbcOptions = new JDBCOptions(options)
- setAuthenticationConfigIfNeeded(driver, jdbcOptions)
- UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal, jdbcOptions.keytab)
- .doAs(
- new PrivilegedExceptionAction[Connection]() {
- override def run(): Connection = {
- DB2ConnectionProvider.super.getConnection(driver, options)
+ val parent = Configuration.getConfiguration
+ try {
+ setAuthenticationConfig(parent, driver, jdbcOptions)
+ UserGroupInformation.loginUserFromKeytabAndReturnUGI(jdbcOptions.principal,
+ jdbcOptions.keytab)
+ .doAs(
+ new PrivilegedExceptionAction[Connection]() {
+ override def run(): Connection = {
+ DB2ConnectionProvider.super.getConnection(driver, options)
+ }
}
- }
- )
+ )
+ } finally {
+ logDebug("Restoring original security configuration")
+ Configuration.setConfiguration(parent)
+ }
Review comment:
On the same side, started to change.
----------------------------------------------------------------
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] tdg5 commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-975632632
@HyukjinKwon , oops, you are correct, I got confused at which PR I was looking at. The problem is actually introduced here: https://github.com/apache/spark/pull/29024/files#diff-345beef18081272d77d91eeca2d9b5534ff6e642245352f40f4e9c9b8922b085R58
--
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
[GitHub] [spark] tdg5 commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-975696894
Moved my whining over to https://github.com/apache/spark/pull/29024/files#r754441783. Sorry for the noise on this PR.
--
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
[GitHub] [spark] SparkQA removed a comment on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784030109
**[Test build #135372 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135372/testReport)** for PR 31622 at commit [`3d77273`](https://github.com/apache/spark/commit/3d7727336403ba0130a4055e03529161c9c2d36e).
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784235918
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135372/
----------------------------------------------------------------
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] srowen commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-975678842
I'm not immediately seeing the problem. this removed a lot of synchronization and introduced it at create() only
--
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
[GitHub] [spark] tdg5 edited a comment on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
tdg5 edited a comment on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-974350953
Help!
It seems like no one considered the reality that some apps may rely on being able to establish many connections simultaneously for performance reasons. This change forces concurrency to 1 when establishing database connections and that strikes me as a **significant** user impacting change.
Can anyone propose a workaround for this? I have an app that makes connections to thousands of databases and I can't upgrade to any version >3.1.x because of this **significant** bottleneck.
https://issues.apache.org/jira/browse/SPARK-37391
ðŸ˜
EDIT: oops, I got confused the problem is actually introduced here: https://github.com/apache/spark/pull/29024/files#diff-345beef18081272d77d91eeca2d9b5534ff6e642245352f40f4e9c9b8922b085R58
--
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
[GitHub] [spark] SparkQA commented on pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-785347945
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40016/
----------------------------------------------------------------
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 closed pull request #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #31622:
URL: https://github.com/apache/spark/pull/31622
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784092740
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39952/
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784099752
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39952/
----------------------------------------------------------------
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 #31622: [SPARK-34497][SQL] Fix built-in JDBC connection providers to restore JVM security context changes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31622:
URL: https://github.com/apache/spark/pull/31622#issuecomment-784099720
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39952/
----------------------------------------------------------------
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