You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "wernerdv (via GitHub)" <gi...@apache.org> on 2025/02/17 20:52:07 UTC
[PR] MINOR: Cleanup metadata module [kafka]
wernerdv opened a new pull request, #18937:
URL: https://github.com/apache/kafka/pull/18937
Removed unused code and fixed IDEA warnings.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18937:
URL: https://github.com/apache/kafka/pull/18937#issuecomment-2726942869
@wernerdv could you please fix the conflicts? thanks!
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r2019968079
##########
metadata/src/main/java/org/apache/kafka/metadata/util/SnapshotFileReader.java:
##########
@@ -208,8 +208,4 @@ public void close() throws Exception {
beginShutdown("closing");
queue.close();
}
-
- public CompletableFuture<Void> caughtUpFuture() {
Review Comment:
The commit [0b4e9a](https://github.com/apache/kafka/commit/0b4e9afee2ace7edf6ff8690e070100b98627836#diff-9d62585f89bdb5cc9f1a714f64d7901b9950161326605c0b2124413b3015cbb5L291) removed a commented-out line `//snapshotFileReader.caughtUpFuture().get();`.
It seems that does not need to use this method in MetadataShell.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966554092
##########
metadata/src/main/java/org/apache/kafka/metadata/PartitionRegistration.java:
##########
@@ -466,11 +466,4 @@ public String toString() {
", partitionEpoch=" + partitionEpoch +
")";
}
-
- public boolean hasSameAssignment(PartitionRegistration registration) {
Review Comment:
the most recent commit that referred to this method https://github.com/apache/kafka/commit/85bfdf4127e135eb85f01be6285dca4094e8ac79
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on PR #18937:
URL: https://github.com/apache/kafka/pull/18937#issuecomment-2718631246
@chia7712 Hello, could you please take a look when you have some free time?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966550827
##########
metadata/src/main/java/org/apache/kafka/controller/errors/ControllerExceptions.java:
##########
@@ -40,21 +40,6 @@ public static boolean isTimeoutException(Throwable exception) {
return exception instanceof TimeoutException;
}
- /**
- * Check if an exception is a NotController exception.
- *
- * @param exception The exception to check.
- * @return True if the exception is a NotController exception.
- */
- public static boolean isNotControllerException(Throwable exception) {
Review Comment:
the most recent commit that referred to this method https://github.com/apache/kafka/commit/146a6976aed0d9f90c70b6f21dca8b887cc34e71
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r2019968079
##########
metadata/src/main/java/org/apache/kafka/metadata/util/SnapshotFileReader.java:
##########
@@ -208,8 +208,4 @@ public void close() throws Exception {
beginShutdown("closing");
queue.close();
}
-
- public CompletableFuture<Void> caughtUpFuture() {
Review Comment:
The commit [0b4e9a](https://github.com/apache/kafka/commit/0b4e9afee2ace7edf6ff8690e070100b98627836#diff-9d62585f89bdb5cc9f1a714f64d7901b9950161326605c0b2124413b3015cbb5L291) removed a commented-out line in MetadataShell#waitUntilCaughtUp - `//snapshotFileReader.caughtUpFuture().get();`.
It seems that does not need to use this method in MetadataShell.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966553603
##########
metadata/src/main/java/org/apache/kafka/image/ClusterImage.java:
##########
@@ -66,10 +66,6 @@ public Map<Integer, ControllerRegistration> controllers() {
return controllers;
}
- public boolean containsBroker(int brokerId) {
Review Comment:
the most recent commit that referred to this method https://github.com/apache/kafka/commit/85bfdf4127e135eb85f01be6285dca4094e8ac79
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r2019750691
##########
metadata/src/main/java/org/apache/kafka/metadata/util/SnapshotFileReader.java:
##########
@@ -208,8 +208,4 @@ public void close() throws Exception {
beginShutdown("closing");
queue.close();
}
-
- public CompletableFuture<Void> caughtUpFuture() {
Review Comment:
Should it be used by `MetadataShell`?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966552907
##########
metadata/src/main/java/org/apache/kafka/metadata/LeaderAndIsr.java:
##########
@@ -133,20 +126,6 @@ public List<Integer> isr() {
.collect(Collectors.toList());
}
- public boolean equalsAllowStalePartitionEpoch(LeaderAndIsr other) {
Review Comment:
the most recent commit that referred to this method https://github.com/apache/kafka/commit/62c2880a584dee52af52d74d7f0ec44475528c72
The method was used in KafkaController, which was removed in commit https://github.com/apache/kafka/commit/0e502e0b477a5af5e7c545ddab03de41ad2be481
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966548309
##########
metadata/src/main/java/org/apache/kafka/image/TopicDelta.java:
##########
@@ -129,20 +129,6 @@ public TopicImage apply() {
return new TopicImage(image.name(), image.id(), newPartitions);
}
- public boolean hasPartitionsWithAssignmentChanges() {
Review Comment:
The last commit that includes the use of this method:
https://github.com/apache/kafka/commit/8b5d755bf674e0d700fc87e792b4bf4791c298c9
In the following commit, the reference to this method was removed:
https://github.com/apache/kafka/commit/85bfdf4127e135eb85f01be6285dca4094e8ac79
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #18937:
URL: https://github.com/apache/kafka/pull/18937
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r2020616189
##########
metadata/src/main/java/org/apache/kafka/metadata/util/SnapshotFileReader.java:
##########
@@ -208,8 +208,4 @@ public void close() throws Exception {
beginShutdown("closing");
queue.close();
}
-
- public CompletableFuture<Void> caughtUpFuture() {
Review Comment:
Thank you for sharing this information. It appears we overlooked this aspect during the implementation of the shell tool. It is likely acceptable to remove the method for the time being, as we can reinstate it in the future.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on PR #18937:
URL: https://github.com/apache/kafka/pull/18937#issuecomment-2664058848
@cmccabe @mumrah Hello, please take a look.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on PR #18937:
URL: https://github.com/apache/kafka/pull/18937#issuecomment-2703132016
@mumrah Could you please take a look when you have some free time?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on PR #18937:
URL: https://github.com/apache/kafka/pull/18937#issuecomment-2761342324
@chia7712 Please review when you have time. This is a small PR removing unused code.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18937:
URL: https://github.com/apache/kafka/pull/18937#issuecomment-2672446385
Oh, also please refrain from force pushing now that the review has started :)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966554660
##########
metadata/src/main/java/org/apache/kafka/metadata/VersionRange.java:
##########
@@ -56,13 +56,6 @@ public boolean contains(short version) {
return version >= min && version <= max;
}
- /**
- * Check if a given version range has overlap with this one
- */
- public boolean intersects(VersionRange other) {
Review Comment:
the most recent commit that referred to this method https://github.com/apache/kafka/commit/7b7eb6243f020d9ad54fd40cc290f41e52abc6ca
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966554660
##########
metadata/src/main/java/org/apache/kafka/metadata/VersionRange.java:
##########
@@ -56,13 +56,6 @@ public boolean contains(short version) {
return version >= min && version <= max;
}
- /**
- * Check if a given version range has overlap with this one
- */
- public boolean intersects(VersionRange other) {
Review Comment:
the most recent commit that referred to this method https://github.com/apache/kafka/commit/85bfdf4127e135eb85f01be6285dca4094e8ac79
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966553844
##########
metadata/src/main/java/org/apache/kafka/metadata/LeaderAndIsr.java:
##########
@@ -75,10 +72,6 @@ public LeaderAndIsr(
this.partitionEpoch = partitionEpoch;
}
- public static LeaderAndIsr duringDelete(List<Integer> isr) {
Review Comment:
the most recent commit that referred to this method https://github.com/apache/kafka/commit/5a1fb1588d229116a410b56ac1a80aa8b933f773
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #18937:
URL: https://github.com/apache/kafka/pull/18937#discussion_r1966548309
##########
metadata/src/main/java/org/apache/kafka/image/TopicDelta.java:
##########
@@ -129,20 +129,6 @@ public TopicImage apply() {
return new TopicImage(image.name(), image.id(), newPartitions);
}
- public boolean hasPartitionsWithAssignmentChanges() {
Review Comment:
The last commit that includes the use of this method:
https://github.com/apache/kafka/commit/8b5d755bf674e0d700fc87e792b4bf4791c298c9
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Cleanup metadata module [kafka]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on PR #18937:
URL: https://github.com/apache/kafka/pull/18937#issuecomment-2727249907
> @wernerdv could you please fix the conflicts? thanks!
@chia7712 Done.
The test `QuorumControllerTest#testMinIsrUpdateWithElr()` failed when running on JDK 17, but locally all tests in the QuorumControllerTest class succeed.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org