From 185eb9b59a9676f641af8bac8e8373ad4dfd5dc6 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Sat, 18 Apr 2015 09:26:50 -0700 Subject: [PATCH] kafka-1994; Evaluate performance effect of chroot check on Topic creation; patched by Ashish Singh; reviewed by Gwen Shapira and Jun Rao --- core/src/main/scala/kafka/utils/ZkUtils.scala | 50 ++++++++++++------- .../test/scala/unit/kafka/zk/ZKPathTest.scala | 10 +++- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/core/src/main/scala/kafka/utils/ZkUtils.scala b/core/src/main/scala/kafka/utils/ZkUtils.scala index b03172a257d..5685a1eddb2 100644 --- a/core/src/main/scala/kafka/utils/ZkUtils.scala +++ b/core/src/main/scala/kafka/utils/ZkUtils.scala @@ -231,7 +231,7 @@ object ZkUtils extends Logging { */ def makeSurePersistentPathExists(client: ZkClient, path: String) { if (!client.exists(path)) - new ZkPath(client).createPersistent(path, true) // won't throw NoNodeException or NodeExistsException + ZkPath.createPersistent(client, path, true) //won't throw NoNodeException or NodeExistsException } /** @@ -240,7 +240,7 @@ object ZkUtils extends Logging { private def createParentPath(client: ZkClient, path: String): Unit = { val parentDir = path.substring(0, path.lastIndexOf('/')) if (parentDir.length != 0) { - new ZkPath(client).createPersistent(parentDir, true) + ZkPath.createPersistent(client, parentDir, true) } } @@ -248,13 +248,12 @@ object ZkUtils extends Logging { * Create an ephemeral node with the given path and data. Create parents if necessary. */ private def createEphemeralPath(client: ZkClient, path: String, data: String): Unit = { - val zkPath = new ZkPath(client) try { - zkPath.createEphemeral(path, data) + ZkPath.createEphemeral(client, path, data) } catch { case e: ZkNoNodeException => { createParentPath(client, path) - zkPath.createEphemeral(path, data) + ZkPath.createEphemeral(client, path, data) } } } @@ -333,19 +332,18 @@ object ZkUtils extends Logging { * Create an persistent node with the given path and data. Create parents if necessary. */ def createPersistentPath(client: ZkClient, path: String, data: String = ""): Unit = { - val zkPath = new ZkPath(client) try { - zkPath.createPersistent(path, data) + ZkPath.createPersistent(client, path, data) } catch { case e: ZkNoNodeException => { createParentPath(client, path) - zkPath.createPersistent(path, data) + ZkPath.createPersistent(client, path, data) } } } def createSequentialPersistentPath(client: ZkClient, path: String, data: String = ""): String = { - new ZkPath(client).createPersistentSequential(path, data) + ZkPath.createPersistentSequential(client, path, data) } /** @@ -360,7 +358,7 @@ object ZkUtils extends Logging { case e: ZkNoNodeException => { createParentPath(client, path) try { - new ZkPath(client).createPersistent(path, data) + ZkPath.createPersistent(client, path, data) } catch { case e: ZkNodeExistsException => client.writeData(path, data) @@ -431,7 +429,7 @@ object ZkUtils extends Logging { } catch { case e: ZkNoNodeException => { createParentPath(client, path) - new ZkPath(client).createEphemeral(path, data) + ZkPath.createEphemeral(client, path, data) } case e2: Throwable => throw e2 } @@ -829,24 +827,40 @@ class ZKConfig(props: VerifiableProperties) { val zkSyncTimeMs = props.getInt("zookeeper.sync.time.ms", 2000) } -class ZkPath(client: ZkClient) { - if (!client.exists("/")) { - throw new ConfigException("Zookeeper namespace does not exist") +object ZkPath { + @volatile private var isNamespacePresent: Boolean = false + + def checkNamespace(client: ZkClient) { + if(isNamespacePresent) + return + + if (!client.exists("/")) { + throw new ConfigException("Zookeeper namespace does not exist") + } + isNamespacePresent = true } - def createPersistent(path: String, data: Object) { + def resetNamespaceCheckedState { + isNamespacePresent = false + } + + def createPersistent(client: ZkClient, path: String, data: Object) { + checkNamespace(client) client.createPersistent(path, data) } - def createPersistent(path: String, createParents: Boolean) { + def createPersistent(client: ZkClient, path: String, createParents: Boolean) { + checkNamespace(client) client.createPersistent(path, createParents) } - def createEphemeral(path: String, data: Object) { + def createEphemeral(client: ZkClient, path: String, data: Object) { + checkNamespace(client) client.createEphemeral(path, data) } - def createPersistentSequential(path: String, data: Object): String = { + def createPersistentSequential(client: ZkClient, path: String, data: Object): String = { + checkNamespace(client) client.createPersistentSequential(path, data) } } diff --git a/core/src/test/scala/unit/kafka/zk/ZKPathTest.scala b/core/src/test/scala/unit/kafka/zk/ZKPathTest.scala index 1bc45b11ace..d42108eacf7 100644 --- a/core/src/test/scala/unit/kafka/zk/ZKPathTest.scala +++ b/core/src/test/scala/unit/kafka/zk/ZKPathTest.scala @@ -19,7 +19,7 @@ package unit.kafka.zk import junit.framework.Assert import kafka.consumer.ConsumerConfig -import kafka.utils.{TestUtils, ZKStringSerializer, ZkUtils} +import kafka.utils.{ZkPath, TestUtils, ZKStringSerializer, ZkUtils} import kafka.zk.ZooKeeperTestHarness import org.I0Itec.zkclient.ZkClient import org.apache.kafka.common.config.ConfigException @@ -38,6 +38,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { config.zkConnectionTimeoutMs, ZKStringSerializer) try { + ZkPath.resetNamespaceCheckedState ZkUtils.createPersistentPath(zkClient, path) fail("Failed to throw ConfigException for missing zookeeper root node") } catch { @@ -51,6 +52,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { var zkClient = new ZkClient(zkConnect, zkSessionTimeoutMs, config.zkConnectionTimeoutMs, ZKStringSerializer) try { + ZkPath.resetNamespaceCheckedState ZkUtils.createPersistentPath(zkClient, path) } catch { case exception: Throwable => fail("Failed to create persistent path") @@ -66,6 +68,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { config.zkConnectionTimeoutMs, ZKStringSerializer) try { + ZkPath.resetNamespaceCheckedState ZkUtils.makeSurePersistentPathExists(zkClient, path) fail("Failed to throw ConfigException for missing zookeeper root node") } catch { @@ -79,6 +82,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { var zkClient = new ZkClient(zkConnect, zkSessionTimeoutMs, config.zkConnectionTimeoutMs, ZKStringSerializer) try { + ZkPath.resetNamespaceCheckedState ZkUtils.makeSurePersistentPathExists(zkClient, path) } catch { case exception: Throwable => fail("Failed to create persistent path") @@ -94,6 +98,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { config.zkConnectionTimeoutMs, ZKStringSerializer) try { + ZkPath.resetNamespaceCheckedState ZkUtils.createEphemeralPathExpectConflict(zkClient, path, "somedata") fail("Failed to throw ConfigException for missing zookeeper root node") } catch { @@ -107,6 +112,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { var zkClient = new ZkClient(zkConnect, zkSessionTimeoutMs, config.zkConnectionTimeoutMs, ZKStringSerializer) try { + ZkPath.resetNamespaceCheckedState ZkUtils.createEphemeralPathExpectConflict(zkClient, path, "somedata") } catch { case exception: Throwable => fail("Failed to create ephemeral path") @@ -122,6 +128,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { config.zkConnectionTimeoutMs, ZKStringSerializer) try { + ZkPath.resetNamespaceCheckedState ZkUtils.createSequentialPersistentPath(zkClient, path) fail("Failed to throw ConfigException for missing zookeeper root node") } catch { @@ -137,6 +144,7 @@ class ZKPathTest extends JUnit3Suite with ZooKeeperTestHarness { var actualPath: String = "" try { + ZkPath.resetNamespaceCheckedState actualPath = ZkUtils.createSequentialPersistentPath(zkClient, path) } catch { case exception: Throwable => fail("Failed to create persistent path")