soarez commented on code in PR #14290: URL: https://github.com/apache/kafka/pull/14290#discussion_r1376787303
########## clients/src/main/java/org/apache/kafka/common/errors/InvalidMetadataException.java: ########## @@ -19,7 +19,7 @@ /** * An exception that may indicate the client's metadata is out of date */ -public abstract class InvalidMetadataException extends RetriableException { +public class InvalidMetadataException extends RetriableException { Review Comment: I had made this change to deal with a partition record with different lengths for `replicas` and `directories`. I've now realized that check hadn't been pushed. If we need to keep this abstract for some reason we can create a new Exception type instead. ########## server-common/src/main/java/org/apache/kafka/common/DirectoryId.java: ########## @@ -16,55 +16,166 @@ */ package org.apache.kafka.common; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; -public class DirectoryId { +public class DirectoryId extends Uuid { /** - * A UUID that is used to identify new or unknown dir assignments. + * A DirectoryId that is used to identify new or unknown dir assignments. */ - public static final Uuid UNASSIGNED = new Uuid(0L, 0L); + public static final DirectoryId UNASSIGNED = new DirectoryId(0L, 0L); /** - * A UUID that is used to represent unspecified offline dirs. + * A DirectoryId that is used to represent unspecified offline dirs. */ - public static final Uuid LOST = new Uuid(0L, 1L); + public static final DirectoryId LOST = new DirectoryId(0L, 1L); /** - * A UUID that is used to represent and unspecified log directory, + * A DirectoryId that is used to represent and unspecified log directory, * that is expected to have been previously selected to host an * associated replica. This contrasts with {@code UNASSIGNED_DIR}, * which is associated with (typically new) replicas that may not * yet have been placed in any log directory. */ - public static final Uuid MIGRATING = new Uuid(0L, 2L); + public static final DirectoryId MIGRATING = new DirectoryId(0L, 2L); /** * The set of reserved UUIDs that will never be returned by the random method. */ - public static final Set<Uuid> RESERVED; + public static final Set<DirectoryId> RESERVED; static { - HashSet<Uuid> reserved = new HashSet<>(Uuid.RESERVED); - // The first 100 UUIDs are reserved for future use. + HashSet<DirectoryId> reserved = new HashSet<>(); + // The first 100 DirectoryIds are reserved for future use. for (long i = 0L; i < 100L; i++) { - reserved.add(new Uuid(0L, i)); + reserved.add(new DirectoryId(0L, i)); } RESERVED = Collections.unmodifiableSet(reserved); } + /** + * Constructs a Directory ID from the underlying 128 bits, + * exactly as a {@link Uuid} is constructed. + */ + private DirectoryId(long mostSigBits, long leastSigBits) { + super(mostSigBits, leastSigBits); + } + + /** + * Creates a DirectoryId based on a base64 string encoding used in the toString() method. + */ + public static DirectoryId fromString(String str) { + return DirectoryId.fromUuid(Uuid.fromString(str)); + } + + /** + * Creates a DirectoryId based on a {@link Uuid}. + */ + public static DirectoryId fromUuid(Uuid uuid) { + return new DirectoryId(uuid.getMostSignificantBits(), uuid.getLeastSignificantBits()); + } Review Comment: I've added a test to verify the reserved values, using MSB/LSB as somewhat less indirect test. -- 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