kevin-wu24 commented on code in PR #20707:
URL: https://github.com/apache/kafka/pull/20707#discussion_r2461320960
##########
metadata/src/main/java/org/apache/kafka/metadata/bootstrap/BootstrapMetadata.java:
##########
@@ -60,11 +60,10 @@ public static BootstrapMetadata fromVersions(
featureNames.sort(String::compareTo);
for (String featureName : featureNames) {
short level = featureVersions.get(featureName);
- if (level > 0) {
- records.add(new ApiMessageAndVersion(new FeatureLevelRecord().
- setName(featureName).
- setFeatureLevel(level), (short) 0));
- }
+ // Include all feature levels, including level 0 which may disable
features
+ records.add(new ApiMessageAndVersion(new FeatureLevelRecord().
+ setName(featureName).
+ setFeatureLevel(level), (short) 0));
Review Comment:
This should not change. The default level of features is 0, and that is why
we don't add a record for them when the level is 0.
##########
core/src/main/scala/kafka/server/KafkaRaftServer.scala:
##########
@@ -181,9 +181,9 @@ object KafkaRaftServer {
}
// Load the BootstrapMetadata.
- val bootstrapDirectory = new BootstrapDirectory(config.metadataLogDir)
- val bootstrapMetadata = bootstrapDirectory.read()
- (metaPropsEnsemble, bootstrapMetadata)
+ // val bootstrapDirectory = new BootstrapDirectory(config.metadataLogDir)
+ // val bootstrapMetadata = bootstrapDirectory.read()
+ (metaPropsEnsemble, null)
Review Comment:
We should remove `bootstrapMetadata` since it is just being passed down to
`QuorumController` eventually. We initialize it in `QuorumController`.
##########
metadata/src/main/java/org/apache/kafka/metadata/bootstrap/BootstrapDirectory.java:
##########
@@ -65,7 +68,10 @@ public BootstrapMetadata read() throws Exception {
throw new RuntimeException("No such directory as " +
directoryPath);
}
}
- Path binaryBootstrapPath = Paths.get(directoryPath,
BINARY_BOOTSTRAP_FILENAME);
+ Path binaryBootstrapPath = Paths.get(directoryPath,
String.format("%s-%d",
+ CLUSTER_METADATA_TOPIC_PARTITION.topic(),
+ CLUSTER_METADATA_TOPIC_PARTITION.partition()),
+ BINARY_BOOTSTRAP_CHECKPOINT_FILENAME);
if (!Files.exists(binaryBootstrapPath)) {
return readFromConfiguration();
Review Comment:
If we are at L76, that means we do 0-0.checkpoint doesn't exist. This is
where we should read from `bootstrap.checkpoint`. If that doesn't exist too,
then we call `readFromConfiguration()`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
