From 79b24a7df9db2d33a948ebca365aa2f48bdb095b Mon Sep 17 00:00:00 2001 From: Cheng Pan Date: Fri, 8 Dec 2023 17:55:25 +0800 Subject: [PATCH] [KYUUBI #5833] Rename service registered endpoint key from serviceUri to serverUri MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # :mag: 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: image ## 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 :bookmark: - [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 0a3910426 [Cheng Pan] Rename service register endpoint key from serviceUri to serverUri Authored-by: Cheng Pan Signed-off-by: Cheng Pan --- .../test/scala/org/apache/kyuubi/ctl/ControlCliSuite.scala | 4 ++-- .../apache/kyuubi/ha/client/etcd/EtcdDiscoveryClient.scala | 2 +- .../kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClient.scala | 2 +- .../org/apache/kyuubi/ha/client/DiscoveryClientTests.scala | 4 ++-- .../ha/client/zookeeper/ZookeeperDiscoveryClientSuite.scala | 2 +- .../org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliSuite.scala b/kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliSuite.scala index 43a694a08..b0319e497 100644 --- a/kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliSuite.scala +++ b/kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliSuite.scala @@ -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""") } diff --git a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/etcd/EtcdDiscoveryClient.scala b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/etcd/EtcdDiscoveryClient.scala index d979804f4..7edc7e8a3 100644 --- a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/etcd/EtcdDiscoveryClient.scala +++ b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/etcd/EtcdDiscoveryClient.scala @@ -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 diff --git a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClient.scala b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClient.scala index 2db7d89d6..a06087d3a 100644 --- a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClient.scala +++ b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClient.scala @@ -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 = diff --git a/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/DiscoveryClientTests.scala b/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/DiscoveryClientTests.scala index 9caf38646..53c0586f5 100644 --- a/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/DiscoveryClientTests.scala +++ b/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/DiscoveryClientTests.scala @@ -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 => @@ -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 => diff --git a/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClientSuite.scala b/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClientSuite.scala index dd78e1fb8..34ed05593 100644 --- a/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClientSuite.scala +++ b/kyuubi-ha/src/test/scala/org/apache/kyuubi/ha/client/zookeeper/ZookeeperDiscoveryClientSuite.scala @@ -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 => diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala index 8c5ccc3be..0951d8272 100644 --- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala +++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala @@ -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))