You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by GitBox <gi...@apache.org> on 2021/08/18 15:38:47 UTC
[GitHub] [incubator-kyuubi] yaooqinn opened a new pull request #958: Ensure two connections in user mode share the same engine
yaooqinn opened a new pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.readthedocs.io/en/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901223201
cc @timothy65535 @pan3793
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] timothy65535 edited a comment on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
timothy65535 edited a comment on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901537537
Hi @yaooqinn, got your point.
> ensure two connections in user mode share the same engine
I think it is not the time to add this test case, it is the third point mentioned on https://github.com/apache/incubator-kyuubi/issues/918 .
After we support the engine pool feature, we can add this test case, like
```
test("ensure two connections in user mode share the same engine") {
val conf = KyuubiConf(false)
conf.set("kyuubi.engine.pool.size", 1)
var r1: String = null
var r2: String = null
new Thread {
override def run(): Unit = withJdbcStatement() { statement =>
val res = statement.executeQuery("set spark.app.name")
assert(res.next())
r1 = res.getString("value")
}
}.start()
new Thread {
override def run(): Unit = withJdbcStatement() { statement =>
val res = statement.executeQuery("set spark.app.name")
assert(res.next())
r2 = res.getString("value")
}
}.start()
eventually(timeout(120.seconds), interval(100.milliseconds)) {
assert(r1 != null && r2 != null)
}
assert(r1 === r2)
}
```
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn removed a comment on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn removed a comment on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901565834
Another option is that we use `subDomain` for registering engines, and on the retrieving side, we can recursively pick all engines
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn closed pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn closed pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn edited a comment on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn edited a comment on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901539986
> Hi @yaooqinn, got your point.
>
> > ensure two connections in user mode share the same engine
>
> I think it is not the time to add this test case, it is the third point mentioned on #918 .
>
> After we support the engine pool feature, we can add this test case, like
>
> ```
> test("ensure two connections in user mode share the same engine") {
>
> val conf = KyuubiConf(false)
> conf.set("kyuubi.engine.pool.size", 1)
>
> var r1: String = null
> var r2: String = null
> new Thread {
> override def run(): Unit = withJdbcStatement() { statement =>
> val res = statement.executeQuery("set spark.app.name")
> assert(res.next())
> r1 = res.getString("value")
> }
> }.start()
>
> new Thread {
> override def run(): Unit = withJdbcStatement() { statement =>
> val res = statement.executeQuery("set spark.app.name")
> assert(res.next())
> r2 = res.getString("value")
> }
> }.start()
>
> eventually(timeout(120.seconds), interval(100.milliseconds)) {
> assert(r1 != null && r2 != null)
> }
>
> assert(r1 === r2)
> }
> ```
why does this test have to wait engine pool？
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] timothy65535 commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
timothy65535 commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901544022
> why does this test have to wait engine pool？
In current codebase, kyuubi can not support multiple engines under USER share level.
Kyuubi supports `CONNECTION`, `USER`, `SERVER` share levels, we want to use kyuubi as hiveserver which can support multiple servers under same zookeeper namespace to share load pressure, client can pick hiveserver randomly. However, due to the influence of the second point above, kyuubi can not create other new engine under same user.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901563422
I don't why your case has to rely on a feature called engine pool.
For high client concurrency, `client can pick hiveserver randomly` you said, this is a feature that Kyuubi does have provided, called `High Availability`, you can exposing all Kyuubi server instances to a single namespace for all clients to get. This is the hive thing you mentioned above.
And then we provide `connection` mode, which a user can use to create as many engines as they want according to `this semantic`, let's call it `connection per engine(CPE)`. Underline here, this mode is against JDBC client connection pool (CP), as CP mode should share everything for each connection, but certainly, CPE will create different Spark applications for each connection, and all applications are isolated. Generally speaking, we can also name it `an engine pool`, which does not cache engine instances but bare computing resources, them dynamically creates-uses-terminates engine with the resources.
Relatively speaking, the `user` mode is a perfect thing for CP, as it shares the same engine for all connections, and we do have a `subDomain` for this mode if users want the engine to be only used by connections within a particular CP, not other connections.
For the feature of `engine pool`, this is a feature definitely NOT for `connection` mode but `user` or `server`. It does break the CP use case, but it may be good for some particular use cases that do need **a group of long-running engines** within a `USER/subDomain` for extremely high performance and not care about the things for share just like `connection` mode does.
A `subDomain` is also a kind of MANUAL engine pool. It helps a user create more engines in `user` mode but does not have an auto-mechanism, e.g. sequentially / randomly / load-based picking for `subDomain or engines`. Or, maybe we can apply such auto-mechanism to creating and picking `subDomain` to implement the feature of the engine pool.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901565834
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] timothy65535 commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
timothy65535 commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901547947
> > client can pick hiveserver randomly.
>
> this is equivalent to `client can pick Kyuubi server randomly`
Right, it's not a concurrent issue, it's a pick strategy issue.
If we use `getServerHost` like following, kyuubi will can not create a new more engine under the same USER share level as before.
```
EngineRef.java
private def create(zkClient: CuratorFramework): (String, Int) = tryWithLock(zkClient) {
// TODO: improve this after support engine pool
var engineRef = getServerHost(zkClient, engineSpace)
// Get the engine address ahead if another process has succeeded
if (engineRef.nonEmpty) return engineRef.get
...
}
```
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901539986
> Hi @yaooqinn, got your point.
>
> > ensure two connections in user mode share the same engine
>
> I think it is not the time to add this test case, it is the third point mentioned on #918 .
>
> After we support the engine pool feature, we can add this test case, like
>
> ```
> test("ensure two connections in user mode share the same engine") {
>
> val conf = KyuubiConf(false)
> conf.set("kyuubi.engine.pool.size", 1)
>
> var r1: String = null
> var r2: String = null
> new Thread {
> override def run(): Unit = withJdbcStatement() { statement =>
> val res = statement.executeQuery("set spark.app.name")
> assert(res.next())
> r1 = res.getString("value")
> }
> }.start()
>
> new Thread {
> override def run(): Unit = withJdbcStatement() { statement =>
> val res = statement.executeQuery("set spark.app.name")
> assert(res.next())
> r2 = res.getString("value")
> }
> }.start()
>
> eventually(timeout(120.seconds), interval(100.milliseconds)) {
> assert(r1 != null && r2 != null)
> }
>
> assert(r1 === r2)
> }
> ```
why can not this test have to wait engine pool？
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] timothy65535 commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
timothy65535 commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901537537
Hi @yaooqinn, got your point.
> ensure two connections in user mode share the same engine
I think it is not the time to add this test case, it is the third point mentioned under https://github.com/apache/incubator-kyuubi/issues/918 .
After we support the engine pool feature, we can add this test case, like
```
test("ensure two connections in user mode share the same engine") {
val conf = KyuubiConf(false)
conf.set("kyuubi.engine.pool.size", 1)
var r1: String = null
var r2: String = null
new Thread {
override def run(): Unit = withJdbcStatement() { statement =>
val res = statement.executeQuery("set spark.app.name")
assert(res.next())
r1 = res.getString("value")
}
}.start()
new Thread {
override def run(): Unit = withJdbcStatement() { statement =>
val res = statement.executeQuery("set spark.app.name")
assert(res.next())
r2 = res.getString("value")
}
}.start()
eventually(timeout(120.seconds), interval(100.milliseconds)) {
assert(r1 != null && r2 != null)
}
assert(r1 === r2)
}
```
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901545793
> client can pick hiveserver randomly.
this is equivalent to `client can pick Kyuubi server randomly`
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901593997
Hi @timothy65535 have you addressed #925 ? It would be great if you can share your story there:)
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] timothy65535 edited a comment on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
timothy65535 edited a comment on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901547947
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] timothy65535 commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
timothy65535 commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901546095
Hi @yaooqinn
It's also ok we add this test case first, we need to come back and modify it after we finish supporting the engine pool.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901223201
cc @timothy65535 @pan3793
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] timothy65535 commented on pull request #958: [TEST] Ensure two connections in user mode share the same engine
Posted by GitBox <gi...@apache.org>.
timothy65535 commented on pull request #958:
URL: https://github.com/apache/incubator-kyuubi/pull/958#issuecomment-901572270
@yaooqinn Thanks for detail mechanism about kyuubi engine, I understand what you mean very much.
And agree following points
- For the feature of engine pool, this is a feature is design for `USER` mode.
- A subDomain is also a kind of MANUAL engine pool. It helps a user create more engines in user mode but does not have an auto-mechanism, e.g. sequentially / randomly / load-based picking for subDomain or engines. Or, maybe we can apply such auto-mechanism to creating and picking subDomain to implement the feature of the engine pool.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org