You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "cmccabe (via GitHub)" <gi...@apache.org> on 2025/01/08 21:21:12 UTC
[PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
cmccabe opened a new pull request, #18450:
URL: https://github.com/apache/kafka/pull/18450
(no comment)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe commented on code in PR #18450:
URL: https://github.com/apache/kafka/pull/18450#discussion_r1907916903
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -1476,7 +1484,8 @@ class KRaftClusterTest {
val parentDir = log.parentDir
val targetParentDir = broker0.config.logDirs.filter(_ != parentDir).head
val targetDirFile = new File(targetParentDir, log.dir.getName)
- FileUtils.copyDirectory(log.dir, targetDirFile)
+ targetDirFile.mkdir()
+ copyDirectory(log.dir.toString(), targetDirFile.toString())
Review Comment:
This test used commons-io, which is being removed. It's generally redundant with the new Java stuff, but was a ZK dependency.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe commented on code in PR #18450:
URL: https://github.com/apache/kafka/pull/18450#discussion_r1907916903
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -1476,7 +1484,8 @@ class KRaftClusterTest {
val parentDir = log.parentDir
val targetParentDir = broker0.config.logDirs.filter(_ != parentDir).head
val targetDirFile = new File(targetParentDir, log.dir.getName)
- FileUtils.copyDirectory(log.dir, targetDirFile)
+ targetDirFile.mkdir()
+ copyDirectory(log.dir.toString(), targetDirFile.toString())
Review Comment:
This test used commons-io, which is being removed. (commons-io is generally redundant with the new Java stuff, but was a ZK dependency.)
This change fixes the test not to use commons-io.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18450:
URL: https://github.com/apache/kafka/pull/18450#discussion_r1937376383
##########
core/src/main/scala/kafka/zookeeper/ZooKeeperClient.scala:
##########
@@ -59,63 +50,9 @@ class ZooKeeperClient(connectString: String,
time: Time,
metricGroup: String,
metricType: String,
- private[zookeeper] val clientConfig: ZKClientConfig,
name: String) extends Logging {
- private val metricsGroup: KafkaMetricsGroup = new KafkaMetricsGroup(metricGroup, metricType)
-
this.logIdent = s"[ZooKeeperClient $name] "
- private val initializationLock = new ReentrantReadWriteLock()
- private val isConnectedOrExpiredLock = new ReentrantLock()
- private val isConnectedOrExpiredCondition = isConnectedOrExpiredLock.newCondition()
- private val zNodeChangeHandlers = new ConcurrentHashMap[String, ZNodeChangeHandler]().asScala
- private val zNodeChildChangeHandlers = new ConcurrentHashMap[String, ZNodeChildChangeHandler]().asScala
- private val inFlightRequests = new Semaphore(maxInFlightRequests)
- private val stateChangeHandlers = new ConcurrentHashMap[String, StateChangeHandler]().asScala
- private[zookeeper] val reinitializeScheduler = new KafkaScheduler(1, true, s"zk-client-${threadPrefix}reinit-")
- private var isFirstConnectionEstablished = false
-
- private val metricNames = mutable.Set[String]()
-
- // The state map has to be created before creating ZooKeeper since it's needed in the ZooKeeper callback.
- private val stateToMeterMap = {
- import KeeperState._
- val stateToEventTypeMap = Map(
- Disconnected -> "Disconnects",
- SyncConnected -> "SyncConnects",
- AuthFailed -> "AuthFailures",
- ConnectedReadOnly -> "ReadOnlyConnects",
- SaslAuthenticated -> "SaslAuthentications",
- Expired -> "Expires"
- )
- stateToEventTypeMap.map { case (state, eventType) =>
- val name = s"ZooKeeper${eventType}PerSec"
- metricNames += name
- state -> metricsGroup.newMeter(name, eventType.toLowerCase(Locale.ROOT), TimeUnit.SECONDS)
- }
- }
-
- info(s"Initializing a new session to $connectString.")
- // Fail-fast if there's an error during construction (so don't call initialize, which retries forever)
- @volatile private var zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, ZooKeeperClientWatcher,
- clientConfig)
-
- metricsGroup.newGauge("SessionState", () => connectionState.toString)
Review Comment:
@Yunyung Could you please file a minor to add this removed metrics to `zk2kraft.html`?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on code in PR #18450:
URL: https://github.com/apache/kafka/pull/18450#discussion_r1937409376
##########
core/src/main/scala/kafka/zookeeper/ZooKeeperClient.scala:
##########
@@ -59,63 +50,9 @@ class ZooKeeperClient(connectString: String,
time: Time,
metricGroup: String,
metricType: String,
- private[zookeeper] val clientConfig: ZKClientConfig,
name: String) extends Logging {
- private val metricsGroup: KafkaMetricsGroup = new KafkaMetricsGroup(metricGroup, metricType)
-
this.logIdent = s"[ZooKeeperClient $name] "
- private val initializationLock = new ReentrantReadWriteLock()
- private val isConnectedOrExpiredLock = new ReentrantLock()
- private val isConnectedOrExpiredCondition = isConnectedOrExpiredLock.newCondition()
- private val zNodeChangeHandlers = new ConcurrentHashMap[String, ZNodeChangeHandler]().asScala
- private val zNodeChildChangeHandlers = new ConcurrentHashMap[String, ZNodeChildChangeHandler]().asScala
- private val inFlightRequests = new Semaphore(maxInFlightRequests)
- private val stateChangeHandlers = new ConcurrentHashMap[String, StateChangeHandler]().asScala
- private[zookeeper] val reinitializeScheduler = new KafkaScheduler(1, true, s"zk-client-${threadPrefix}reinit-")
- private var isFirstConnectionEstablished = false
-
- private val metricNames = mutable.Set[String]()
-
- // The state map has to be created before creating ZooKeeper since it's needed in the ZooKeeper callback.
- private val stateToMeterMap = {
- import KeeperState._
- val stateToEventTypeMap = Map(
- Disconnected -> "Disconnects",
- SyncConnected -> "SyncConnects",
- AuthFailed -> "AuthFailures",
- ConnectedReadOnly -> "ReadOnlyConnects",
- SaslAuthenticated -> "SaslAuthentications",
- Expired -> "Expires"
- )
- stateToEventTypeMap.map { case (state, eventType) =>
- val name = s"ZooKeeper${eventType}PerSec"
- metricNames += name
- state -> metricsGroup.newMeter(name, eventType.toLowerCase(Locale.ROOT), TimeUnit.SECONDS)
- }
- }
-
- info(s"Initializing a new session to $connectString.")
- // Fail-fast if there's an error during construction (so don't call initialize, which retries forever)
- @volatile private var zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, ZooKeeperClientWatcher,
- clientConfig)
-
- metricsGroup.newGauge("SessionState", () => connectionState.toString)
Review Comment:
Sure. Also thanks to @m1a2st for pointing this out.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18450:
URL: https://github.com/apache/kafka/pull/18450#discussion_r1908070818
##########
core/src/main/scala/kafka/zk/KafkaZkClient.scala:
##########
@@ -53,28 +37,9 @@ import scala.jdk.CollectionConverters._
* easier to migrate away from `ZkUtils` (since removed). We should revisit this. We should also consider whether a
* monolithic [[kafka.zk.ZkData]] is the way to go.
*/
-class KafkaZkClient private[zk] (
- zooKeeperClient: ZooKeeperClient,
- isSecure: Boolean,
- time: Time,
- enableEntityConfigControllerCheck: Boolean
-) extends AutoCloseable with Logging {
-
- private val metricsGroup: KafkaMetricsGroup = new KafkaMetricsGroup("kafka.server", "ZooKeeperClientMetrics")
Review Comment:
@m1a2st please include those removed zk-related metrics in #18365
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18450:
URL: https://github.com/apache/kafka/pull/18450#discussion_r1907911940
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -1476,7 +1484,8 @@ class KRaftClusterTest {
val parentDir = log.parentDir
val targetParentDir = broker0.config.logDirs.filter(_ != parentDir).head
val targetDirFile = new File(targetParentDir, log.dir.getName)
- FileUtils.copyDirectory(log.dir, targetDirFile)
+ targetDirFile.mkdir()
+ copyDirectory(log.dir.toString(), targetDirFile.toString())
Review Comment:
Is this unrelated?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #18450:
URL: https://github.com/apache/kafka/pull/18450
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18435: Remove zookeeper dependencies in build.gradle [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #18450:
URL: https://github.com/apache/kafka/pull/18450#discussion_r1908103404
##########
core/src/main/scala/kafka/zk/KafkaZkClient.scala:
##########
@@ -53,28 +37,9 @@ import scala.jdk.CollectionConverters._
* easier to migrate away from `ZkUtils` (since removed). We should revisit this. We should also consider whether a
* monolithic [[kafka.zk.ZkData]] is the way to go.
*/
-class KafkaZkClient private[zk] (
- zooKeeperClient: ZooKeeperClient,
- isSecure: Boolean,
- time: Time,
- enableEntityConfigControllerCheck: Boolean
-) extends AutoCloseable with Logging {
-
- private val metricsGroup: KafkaMetricsGroup = new KafkaMetricsGroup("kafka.server", "ZooKeeperClientMetrics")
Review Comment:
Sure, I will addressed it.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org