chia7712 commented on code in PR #19040:
URL: https://github.com/apache/kafka/pull/19040#discussion_r1976535715
##########
metadata/src/main/java/org/apache/kafka/controller/FeatureControlManager.java:
##########
@@ -175,10 +167,20 @@ ControllerResult<ApiError> updateFeatures(
}
}
- MetadataVersion metadataVersion() {
+ Optional<MetadataVersion> metadataVersion() {
return metadataVersion.get();
}
+ MetadataVersion metadataVersionOrThrow() {
+ return metadataVersion.get().orElseThrow(() ->
+ new IllegalStateException("Unknown metadata version for
FeatureControlManager: " + this));
+ }
+
+ private MetadataVersion metadataVersionOrThrow(long epoch) {
+ return metadataVersion.get(epoch).orElseThrow(() ->
+ new IllegalStateException("Unknown metadata version for
FeatureControlManager: " + this));
Review Comment:
If you want to print "this", please override the `toString` method
##########
metadata/src/main/java/org/apache/kafka/controller/ActivationRecordsGenerator.java:
##########
@@ -152,13 +153,12 @@ static ControllerResult<Void> recordsForNonEmptyLog(
*/
static ControllerResult<Void> generate(
Review Comment:
please update the docs
##########
metadata/src/main/java/org/apache/kafka/controller/FeatureControlManager.java:
##########
@@ -175,10 +167,20 @@ ControllerResult<ApiError> updateFeatures(
}
}
- MetadataVersion metadataVersion() {
+ Optional<MetadataVersion> metadataVersion() {
return metadataVersion.get();
}
+ MetadataVersion metadataVersionOrThrow() {
+ return metadataVersion.get().orElseThrow(() ->
Review Comment:
```
return metadataVersionOrThrow(SnapshotRegistry.LATEST_EPOCH);
```
##########
metadata/src/main/java/org/apache/kafka/controller/QuorumController.java:
##########
@@ -1156,7 +1156,6 @@ public ControllerResult<Void> generateRecordsAndResult() {
try {
return ActivationRecordsGenerator.generate(
log::warn,
- logReplayTracker.empty(),
Review Comment:
we don't need the `logReplayTracker`, right?
##########
metadata/src/main/java/org/apache/kafka/controller/FeatureControlManager.java:
##########
@@ -361,7 +363,7 @@ FinalizedControllerFeatures finalizedFeatures(long epoch) {
FinalizedControllerFeatures latestFinalizedFeatures() {
Review Comment:
it seems we can inline this method as it is used by `isElrFeatureEnabled`
only
--
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
