Skip to content

Commit

Permalink
[KYUUBI #5833] Rename service registered endpoint key from serviceUri…
Browse files Browse the repository at this point in the history
… to serverUri

# 🔍 Description
## Issue References 🔗

It was reported that https://github.com/beltran/gohive can not parse the Kyuubi-registered Zk endpoint, because it relies on the znode name's `serverUri` key, while Hive JDBC driver does not care about the znode name but the znode data

gohive's behavior:
https://github.com/beltran/gohive/blob/5bd059924846d1c0f7e8167eafde8b78be4a5035/hive.go#L160-L168

Kyuubi Hive JDBC driver behavior:
<img width="1043" alt="image" src="https://github.com/apache/kyuubi/assets/26535726/06740985-82ef-4006-b7d6-de0b5b2c0c7c">

## Describe Your Solution 🔧

Simply change the zonde name's key from `serviceUri` to `serverUri` to keep it compatible with Hive behavior.

I suppose it won't break the usage of old Kyuubi Hive JDBC Driver / Kyuubi Beeline / Hive JDBC Driver / Hive Beeline.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

Tested with Apache Hive 2.3.9 Beeline, works fine.

```
➜  ~ beeline -u 'jdbc:hive2://localhost:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi'
Connecting to jdbc:hive2://localhost:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi
Connected to: Spark SQL (version 3.3.3)
Driver: Hive JDBC (version 2.3.9)
Transaction isolation: TRANSACTION_REPEATABLE_READ
Beeline version 2.3.9 by Apache Hive
0: jdbc:hive2://localhost:2181/> select kyuubi_version();
...
23/12/08 16:40:23 INFO ExecuteStatement:
           Spark application name: kyuubi_CONNECTION_SPARK_SQL_anonymous_5b8bf1dc-dbc8-4623-8aaa-b43dba435f83
                 application ID: local-1702024801170
                 application web UI: http://10.221.99.150:32835
                 master: local[*]
                 deploy mode: client
                 version: 3.3.3
           Start time: 2023-12-08T16:40:00.220
           User: anonymous
23/12/08 16:40:23 INFO ExecuteStatement: Execute in full collect mode
...
+-------------------+
| kyuubi_version()  |
+-------------------+
| 1.9.0-SNAPSHOT    |
+-------------------+
1 row selected (0.815 seconds)
0: jdbc:hive2://localhost:2181/>
```

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [x] Pull request title is okay.
- [x] No license issues.
- [x] Milestone correctly set?
- [x] Test coverage is ok
- [x] Assignees are selected.
- [x] Minimum number of approvals
- [x] No changes are requested

**Be nice. Be informative.**

Closes #5833 from pan3793/gohive.

Closes #5833

0a39104 [Cheng Pan] Rename service register endpoint key from serviceUri to serverUri

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
  • Loading branch information
pan3793 committed Dec 8, 2023
1 parent 1d6e653 commit 79b24a7
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ class ControlCliSuite extends KyuubiFunSuite with TestPrematureExit {
assert(children.size == 2)

assert(children.head.startsWith(
s"serviceUri=localhost:10000;version=$KYUUBI_VERSION;sequence="))
s"serverUri=localhost:10000;version=$KYUUBI_VERSION;sequence="))
assert(children.last.startsWith(
s"serviceUri=localhost:10001;version=$KYUUBI_VERSION;sequence="))
s"serverUri=localhost:10001;version=$KYUUBI_VERSION;sequence="))
children.foreach { child =>
framework.delete(s"""$znodeRoot/$child""")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ class EtcdDiscoveryClient(conf: KyuubiConf) extends DiscoveryClient {
val extraInfo = attributes.map(kv => kv._1 + "=" + kv._2).mkString(";", ";", "")
val pathPrefix = DiscoveryPaths.makePath(
namespace,
s"serviceUri=$instance;version=${version.getOrElse(KYUUBI_VERSION)}" +
s"serverUri=$instance;version=${version.getOrElse(KYUUBI_VERSION)}" +
s"${extraInfo.stripSuffix(";")};${session}sequence=")
val znode = instance

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ class ZookeeperDiscoveryClient(conf: KyuubiConf) extends DiscoveryClient {
val extraInfo = attributes.map(kv => kv._1 + "=" + kv._2).mkString(";", ";", "")
val pathPrefix = ZKPaths.makePath(
namespace,
s"serviceUri=$instance;version=${version.getOrElse(KYUUBI_VERSION)}" +
s"serverUri=$instance;version=${version.getOrElse(KYUUBI_VERSION)}" +
s"${extraInfo.stripSuffix(";")};${session}sequence=")
var localServiceNode: PersistentNode = null
val createMode =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ trait DiscoveryClientTests extends KyuubiFunSuite {
assert(discoveryClient.pathExists(basePath))
val children = discoveryClient.getChildren(basePath)
assert(children.head ===
s"serviceUri=${service.frontendServices.head.connectionUrl};" +
s"serverUri=${service.frontendServices.head.connectionUrl};" +
s"version=$KYUUBI_VERSION;sequence=0000000000")

children.foreach { child =>
Expand Down Expand Up @@ -107,7 +107,7 @@ trait DiscoveryClientTests extends KyuubiFunSuite {
assert(discoveryClient.pathExists(basePath))
val children = discoveryClient.getChildren(basePath)
assert(children.head ===
s"serviceUri=${service.frontendServices.head.connectionUrl};" +
s"serverUri=${service.frontendServices.head.connectionUrl};" +
s"version=$KYUUBI_VERSION;sequence=0000000000")

children.foreach { child =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ abstract class ZookeeperDiscoveryClientSuite extends DiscoveryClientTests
assert(discoveryClient.pathExists(basePath))
val children = discoveryClient.getChildren(basePath)
assert(children.head ===
s"serviceUri=${service.frontendServices.head.connectionUrl};" +
s"serverUri=${service.frontendServices.head.connectionUrl};" +
s"version=$KYUUBI_VERSION;sequence=0000000000")

children.foreach { child =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,8 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper {
assert(restFrontendService.connectionUrl.equals(testServer.getInstance()))
assert(!testServer.getAttributes.isEmpty)
val attributes = testServer.getAttributes
assert(attributes.containsKey("serviceUri") &&
attributes.get("serviceUri").equals(fe.connectionUrl))
assert(attributes.containsKey("serverUri") &&
attributes.get("serverUri").equals(fe.connectionUrl))
assert(attributes.containsKey("version"))
assert(attributes.containsKey("sequence"))
assert("Running".equals(testServer.getStatus))
Expand Down

0 comments on commit 79b24a7

Please sign in to comment.