ijuma commented on a change in pull request #9967:
URL: https://github.com/apache/kafka/pull/9967#discussion_r566168366
##########
File path: core/src/main/scala/kafka/server/BrokerMetadataCheckpoint.scala
##########
@@ -21,38 +21,203 @@ import java.io._
import java.nio.file.{Files, NoSuchFileException}
import java.util.Properties
+import kafka.common.{InconsistentBrokerMetadataException, KafkaException}
+import kafka.server.RawMetaProperties._
import kafka.utils._
+import org.apache.kafka.common.Uuid
import org.apache.kafka.common.utils.Utils
-case class BrokerMetadata(brokerId: Int,
- clusterId: Option[String]) {
+import scala.collection.mutable
+import scala.jdk.CollectionConverters._
+
+object RawMetaProperties {
+ val ClusterIdKey = "cluster.id"
+ val BrokerIdKey = "broker.id"
+ val NodeIdKey = "node.id"
+ val VersionKey = "version"
+}
+
+case class RawMetaProperties(props: Properties = new Properties()) {
Review comment:
Does it make sense for this to be a case class? We override `toString`,
so what do we actually get from making it a case class here? Do we rely on the
generated equals/hashCode?
##########
File path: clients/src/main/java/org/apache/kafka/common/Uuid.java
##########
@@ -106,6 +106,9 @@ public String toString() {
*/
public static Uuid fromString(String str) {
ByteBuffer uuidBytes =
ByteBuffer.wrap(Base64.getUrlDecoder().decode(str));
+ if (uuidBytes.remaining() != 16) {
+ throw new IllegalArgumentException("Failed to parse `" + str + "`
as a base64-encoded UUID");
Review comment:
Also, would it be better to check the string size before decoding? If
there's a bug and a really large string is passed, it's a bit safer to error
before parsing (and to only include the first N characters in the exception).
##########
File path: clients/src/main/java/org/apache/kafka/common/Uuid.java
##########
@@ -106,6 +106,9 @@ public String toString() {
*/
public static Uuid fromString(String str) {
ByteBuffer uuidBytes =
ByteBuffer.wrap(Base64.getUrlDecoder().decode(str));
+ if (uuidBytes.remaining() != 16) {
+ throw new IllegalArgumentException("Failed to parse `" + str + "`
as a base64-encoded UUID");
Review comment:
It is worth being a bit more explicit about the issue, maybe something
like "Expected 16 bytes, received Y bytes."? It may be a bit difficult to
figure out why the parsing failed otherwise (since the string will have only
valid characters).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
