From 840060ceae5983c77e0e3569a780ae9a705410ee Mon Sep 17 00:00:00 2001 From: hongdongdong Date: Thu, 8 Dec 2022 10:17:54 +0800 Subject: [PATCH] [KYUUBI #3917] Optimize discovery makePath api ### _Why are the changes needed?_ Optimize discovery makePath api. ### _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.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #3917 from hddong/optimize-ds-path-api. Closes #3917 a152373e [hongdongdong] Update 9dc37929 [hongdongdong] Optimize discovery makePath api Authored-by: hongdongdong Signed-off-by: hongdongdong --- .../org/apache/kyuubi/ctl/util/CtlUtils.scala | 2 +- .../apache/kyuubi/ha/client/DiscoveryPaths.scala | 6 +----- .../scala/org/apache/kyuubi/engine/EngineRef.scala | 10 ++++++---- .../kyuubi/server/api/v1/AdminResource.scala | 2 +- .../org/apache/kyuubi/engine/EngineRefTests.scala | 14 +++++++------- .../kyuubi/server/api/v1/AdminResourceSuite.scala | 12 ++++++------ .../kyuubi/server/rest/client/AdminCtlSuite.scala | 2 +- .../server/rest/client/AdminRestApiSuite.scala | 2 +- 8 files changed, 24 insertions(+), 26 deletions(-) diff --git a/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/CtlUtils.scala b/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/CtlUtils.scala index 194f691e5..fdcc127f1 100644 --- a/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/CtlUtils.scala +++ b/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/CtlUtils.scala @@ -52,7 +52,7 @@ object CtlUtils { s"${cliConfig.zkOpts.version}_" + s"${engineShareLevel}_${engineType}", cliConfig.engineOpts.user, - Array(engineSubdomain)) + engineSubdomain) } } diff --git a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/DiscoveryPaths.scala b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/DiscoveryPaths.scala index 4116fd6cf..987a88dda 100644 --- a/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/DiscoveryPaths.scala +++ b/kyuubi-ha/src/main/scala/org/apache/kyuubi/ha/client/DiscoveryPaths.scala @@ -20,11 +20,7 @@ package org.apache.kyuubi.ha.client import org.apache.curator.utils.ZKPaths object DiscoveryPaths { - def makePath(parent: String, firstChild: String): String = { - ZKPaths.makePath(parent, firstChild) - } - - def makePath(parent: String, firstChild: String, restChildren: Array[String]): String = { + def makePath(parent: String, firstChild: String, restChildren: String*): String = { ZKPaths.makePath(parent, firstChild, restChildren: _*) } } diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala index 8f96cdb56..1b6664c96 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala @@ -102,7 +102,8 @@ private[kyuubi] class EngineRef( DiscoveryPaths.makePath( s"${serverSpace}_${KYUUBI_VERSION}_${shareLevel}_$engineType", "seq_num", - Array(appUser, clientPoolName)) + appUser, + clientPoolName) DiscoveryClientProvider.withDiscoveryClient(conf) { client => client.getAndIncrement(snPath) } @@ -141,8 +142,8 @@ private[kyuubi] class EngineRef( private[kyuubi] lazy val engineSpace: String = { val commonParent = s"${serverSpace}_${KYUUBI_VERSION}_${shareLevel}_$engineType" shareLevel match { - case CONNECTION => DiscoveryPaths.makePath(commonParent, appUser, Array(engineRefId)) - case _ => DiscoveryPaths.makePath(commonParent, appUser, Array(subdomain)) + case CONNECTION => DiscoveryPaths.makePath(commonParent, appUser, engineRefId) + case _ => DiscoveryPaths.makePath(commonParent, appUser, subdomain) } } @@ -158,7 +159,8 @@ private[kyuubi] class EngineRef( DiscoveryPaths.makePath( s"${serverSpace}_${shareLevel}_$engineType", "lock", - Array(appUser, subdomain)) + appUser, + subdomain) discoveryClient.tryWithLock( lockPath, timeout + (LOCK_TIMEOUT_SPAN_FACTOR * timeout).toLong)(f) diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala index 37eb36afe..16653be32 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala @@ -175,6 +175,6 @@ private[v1] class AdminResource extends ApiRequestContext with Logging { DiscoveryPaths.makePath( s"${serverSpace}_${engine.getVersion}_${engine.getSharelevel}_${engine.getEngineType}", engine.getUser, - Array(engine.getSubdomain)) + engine.getSubdomain) } } diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefTests.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefTests.scala index 0cd8a3dd5..94ab19516 100644 --- a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefTests.scala +++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefTests.scala @@ -73,7 +73,7 @@ trait EngineRefTests extends KyuubiFunSuite { DiscoveryPaths.makePath( s"kyuubi_${KYUUBI_VERSION}_${CONNECTION}_${engineType}", user, - Array(id))) + id)) assert(engine.defaultEngineName === s"kyuubi_${CONNECTION}_${engineType}_${user}_$id") } } @@ -87,7 +87,7 @@ trait EngineRefTests extends KyuubiFunSuite { DiscoveryPaths.makePath( s"kyuubi_${KYUUBI_VERSION}_${USER}_$FLINK_SQL", user, - Array("default"))) + "default")) assert(appName.defaultEngineName === s"kyuubi_${USER}_${FLINK_SQL}_${user}_default_$id") Seq(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN, KyuubiConf.ENGINE_SHARE_LEVEL_SUB_DOMAIN).foreach { @@ -99,7 +99,7 @@ trait EngineRefTests extends KyuubiFunSuite { DiscoveryPaths.makePath( s"kyuubi_${KYUUBI_VERSION}_${USER}_${FLINK_SQL}", user, - Array("abc"))) + "abc")) assert(appName2.defaultEngineName === s"kyuubi_${USER}_${FLINK_SQL}_${user}_abc_$id") } } @@ -114,7 +114,7 @@ trait EngineRefTests extends KyuubiFunSuite { DiscoveryPaths.makePath( s"kyuubi_${KYUUBI_VERSION}_GROUP_SPARK_SQL", primaryGroupName, - Array("default"))) + "default")) assert(engineRef.defaultEngineName === s"kyuubi_GROUP_SPARK_SQL_${primaryGroupName}_default_$id") @@ -127,7 +127,7 @@ trait EngineRefTests extends KyuubiFunSuite { DiscoveryPaths.makePath( s"kyuubi_${KYUUBI_VERSION}_${GROUP}_${SPARK_SQL}", primaryGroupName, - Array("abc"))) + "abc")) assert(engineRef2.defaultEngineName === s"kyuubi_${GROUP}_${SPARK_SQL}_${primaryGroupName}_abc_$id") } @@ -142,7 +142,7 @@ trait EngineRefTests extends KyuubiFunSuite { DiscoveryPaths.makePath( s"kyuubi_${KYUUBI_VERSION}_${SERVER}_${FLINK_SQL}", user, - Array("default"))) + "default")) assert(appName.defaultEngineName === s"kyuubi_${SERVER}_${FLINK_SQL}_${user}_default_$id") conf.set(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN.key, "abc") @@ -151,7 +151,7 @@ trait EngineRefTests extends KyuubiFunSuite { DiscoveryPaths.makePath( s"kyuubi_${KYUUBI_VERSION}_${SERVER}_${FLINK_SQL}", user, - Array("abc"))) + "abc")) assert(appName2.defaultEngineName === s"kyuubi_${SERVER}_${FLINK_SQL}_${user}_abc_$id") } 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 d58c1d00a..7a31b8c24 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 @@ -78,7 +78,7 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper { val engineSpace = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_USER_SPARK_SQL", Utils.currentUser, - Array("default")) + "default") withDiscoveryClient(conf) { client => engine.getOrCreate(client) @@ -124,7 +124,7 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper { val engineSpace = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_CONNECTION_SPARK_SQL", Utils.currentUser, - Array(id)) + id) withDiscoveryClient(conf) { client => engine.getOrCreate(client) @@ -161,7 +161,7 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper { val engineSpace = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_USER_SPARK_SQL", Utils.currentUser, - Array("")) + "") withDiscoveryClient(conf) { client => engine.getOrCreate(client) @@ -205,21 +205,21 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper { val engineSpace = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_CONNECTION_SPARK_SQL", Utils.currentUser, - Array("")) + "") val id1 = UUID.randomUUID().toString val engine1 = new EngineRef(conf.clone, Utils.currentUser, "grp", id1, null) val engineSpace1 = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_CONNECTION_SPARK_SQL", Utils.currentUser, - Array(id1)) + id1) val id2 = UUID.randomUUID().toString val engine2 = new EngineRef(conf.clone, Utils.currentUser, "grp", id2, null) val engineSpace2 = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_CONNECTION_SPARK_SQL", Utils.currentUser, - Array(id2)) + id2) withDiscoveryClient(conf) { client => engine1.getOrCreate(client) diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminCtlSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminCtlSuite.scala index 7a968dca8..f7cbb2001 100644 --- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminCtlSuite.scala +++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminCtlSuite.scala @@ -59,7 +59,7 @@ class AdminCtlSuite extends RestClientTestHelper with TestPrematureExit { val engineSpace = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_USER_SPARK_SQL", user, - Array("default")) + "default") withDiscoveryClient(conf) { client => engine.getOrCreate(client) diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminRestApiSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminRestApiSuite.scala index 685096dfd..ab1a10202 100644 --- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminRestApiSuite.scala +++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/AdminRestApiSuite.scala @@ -52,7 +52,7 @@ class AdminRestApiSuite extends RestClientTestHelper { val engineSpace = DiscoveryPaths.makePath( s"kyuubi_test_${KYUUBI_VERSION}_USER_SPARK_SQL", user, - Array("default")) + "default") withDiscoveryClient(conf) { client => engine.getOrCreate(client)