soarez commented on code in PR #14628: URL: https://github.com/apache/kafka/pull/14628#discussion_r1377613028
########## core/src/main/scala/kafka/log/LogManager.scala: ########## @@ -267,35 +267,27 @@ class LogManager(logDirs: Seq[File], /** * Retrieves the Uuid for the directory, given its absolute path. */ - def directoryId(dir: String): Option[Uuid] = dirIds.get(dir) + def directoryId(dir: String): Option[Uuid] = directoryIds.get(dir) /** * Determine directory ID for each directory with a meta.properties. - * If meta.properties does not include a directory ID, one is generated and persisted back to meta.properties. - * Directories without a meta.properties don't get a directory ID assigned. */ - private def directoryIds(dirs: Seq[File]): Map[String, Uuid] = { - dirs.flatMap { dir => + private def loadDirectoryIds(dirs: Seq[File]): Map[String, Uuid] = { + val result = mutable.HashMap[String, Uuid]() + dirs.foreach(dir => { try { - val metadataCheckpoint = new BrokerMetadataCheckpoint(new File(dir, KafkaServer.brokerMetaPropsFile)) - metadataCheckpoint.read().map { props => - val rawMetaProperties = new RawMetaProperties(props) - val uuid = rawMetaProperties.directoryId match { - case Some(uuidStr) => Uuid.fromString(uuidStr) - case None => - val uuid = Uuid.randomUuid() - rawMetaProperties.directoryId = uuid.toString - metadataCheckpoint.write(rawMetaProperties.props) - uuid - } - dir.getAbsolutePath -> uuid - }.toMap + val props = PropertiesUtils.readPropertiesFile( + new File(dir, MetaPropertiesEnsemble.META_PROPERTIES_NAME).getAbsolutePath) + val metaProps = new MetaProperties.Builder(props).build() + metaProps.directoryId().ifPresent(directoryId => { + result += (dir.getAbsolutePath -> directoryId) + }) Review Comment: Sounds ok to me 👍 ########## core/src/main/scala/kafka/log/LogManager.scala: ########## @@ -267,35 +267,27 @@ class LogManager(logDirs: Seq[File], /** * Retrieves the Uuid for the directory, given its absolute path. */ - def directoryId(dir: String): Option[Uuid] = dirIds.get(dir) + def directoryId(dir: String): Option[Uuid] = directoryIds.get(dir) /** * Determine directory ID for each directory with a meta.properties. - * If meta.properties does not include a directory ID, one is generated and persisted back to meta.properties. - * Directories without a meta.properties don't get a directory ID assigned. */ - private def directoryIds(dirs: Seq[File]): Map[String, Uuid] = { - dirs.flatMap { dir => + private def loadDirectoryIds(dirs: Seq[File]): Map[String, Uuid] = { + val result = mutable.HashMap[String, Uuid]() + dirs.foreach(dir => { try { - val metadataCheckpoint = new BrokerMetadataCheckpoint(new File(dir, KafkaServer.brokerMetaPropsFile)) - metadataCheckpoint.read().map { props => - val rawMetaProperties = new RawMetaProperties(props) - val uuid = rawMetaProperties.directoryId match { - case Some(uuidStr) => Uuid.fromString(uuidStr) - case None => - val uuid = Uuid.randomUuid() - rawMetaProperties.directoryId = uuid.toString - metadataCheckpoint.write(rawMetaProperties.props) - uuid - } - dir.getAbsolutePath -> uuid - }.toMap + val props = PropertiesUtils.readPropertiesFile( + new File(dir, MetaPropertiesEnsemble.META_PROPERTIES_NAME).getAbsolutePath) + val metaProps = new MetaProperties.Builder(props).build() + metaProps.directoryId().ifPresent(directoryId => { + result += (dir.getAbsolutePath -> directoryId) + }) Review Comment: Sounds ok to me 👍 -- 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