soarez commented on code in PR #14578:
URL: https://github.com/apache/kafka/pull/14578#discussion_r1367768321
##########
core/src/main/scala/kafka/server/BrokerMetadataCheckpoint.scala:
##########
@@ -166,6 +166,7 @@ case class MetaProperties(
}
}
+// Return only the RawMetaProperties that are the same across all directoris.
Review Comment:
```suggestion
// Return only the RawMetaProperties that are the same across all
directories.
```
##########
core/src/main/scala/kafka/server/BrokerMetadataCheckpoint.scala:
##########
@@ -184,6 +185,8 @@ object BrokerMetadataCheckpoint extends Logging {
try {
brokerCheckpoint.read() match {
case Some(properties) =>
+ // XXX Should we check for duplicates here
Review Comment:
Is `LogManager.directoryIds()` a better place to check for duplicates?
##########
core/src/main/scala/kafka/server/KafkaServer.scala:
##########
@@ -1025,9 +1026,13 @@ class KafkaServer(
*/
private def checkpointBrokerMetadata(brokerMetadata: ZkMetaProperties) = {
for (logDir <- config.logDirs if logManager.isLogDirOnline(new
File(logDir).getAbsolutePath)) {
+ val props = brokerMetadata.toProperties.clone().asInstanceOf[Properties]
+ // If the Uuid is not set then we set it here because originally we
ignored
+ // directories with no meta.properties file and we are creating it here.
+ props.setProperty("directory.id",
logManager.directoryId(logDir).getOrElse(Uuid.randomUuid()).toString)
Review Comment:
LogManager is already generating a directory ID if missing, in
`directoryIds`.
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
