You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "JimmyWang6 (via GitHub)" <gi...@apache.org> on 2025/09/09 07:58:10 UTC
[PR] KAFKA-19676: EpochState should override close to avoid throwing IOException [kafka]
JimmyWang6 opened a new pull request, #20508:
URL: https://github.com/apache/kafka/pull/20508
All subclasses of EpochState do not throw an IOException when closing, so catching it is unnecessary. We could override close to remove the IOException declaration.
--
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] KAFKA-19676: EpochState should override close to avoid throwing IOException [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #20508:
URL: https://github.com/apache/kafka/pull/20508
--
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] KAFKA-19676: EpochState should override close to avoid throwing IOException [kafka]
Posted by "JimmyWang6 (via GitHub)" <gi...@apache.org>.
JimmyWang6 commented on code in PR #20508:
URL: https://github.com/apache/kafka/pull/20508#discussion_r2337560824
##########
raft/src/main/java/org/apache/kafka/raft/EpochState.java:
##########
@@ -59,4 +59,9 @@ default Optional<LogOffsetMetadata> highWatermark() {
* User-friendly description of the state
*/
String name();
+
+ @Override
+ default void close() {
+ // intentionally left blank
Review Comment:
@chia7712 I have added comments, please take a another look when you have time, 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] KAFKA-19676: EpochState should override close to avoid throwing IOException [kafka]
Posted by "JimmyWang6 (via GitHub)" <gi...@apache.org>.
JimmyWang6 commented on code in PR #20508:
URL: https://github.com/apache/kafka/pull/20508#discussion_r2335273237
##########
raft/src/main/java/org/apache/kafka/raft/EpochState.java:
##########
@@ -59,4 +59,9 @@ default Optional<LogOffsetMetadata> highWatermark() {
* User-friendly description of the state
*/
String name();
+
+ @Override
+ default void close() {
+ // intentionally left blank
Review Comment:
Sure, thanks for your comment.
--
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] KAFKA-19676: EpochState should override close to avoid throwing IOException [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20508:
URL: https://github.com/apache/kafka/pull/20508#discussion_r2337979712
##########
raft/src/main/java/org/apache/kafka/raft/EpochState.java:
##########
@@ -59,4 +59,10 @@ default Optional<LogOffsetMetadata> highWatermark() {
* User-friendly description of the state
*/
String name();
+
+ @Override
+ default void close() {
+ // Since all subclasses implement the Closeable interface while none throw any IOException,
Review Comment:
It feels odd to add a default implementation just for the sake of documentation. How about using the following style instead?
```java
/**
* Since all subclasses implement the Closeable interface while none throw any IOException,
* this default implementation is provided to eliminate the need for exception handling in the close operation.
*/
@Override
void close();
```
--
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] KAFKA-19676: EpochState should override close to avoid throwing IOException [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20508:
URL: https://github.com/apache/kafka/pull/20508#discussion_r2335044297
##########
raft/src/main/java/org/apache/kafka/raft/EpochState.java:
##########
@@ -59,4 +59,9 @@ default Optional<LogOffsetMetadata> highWatermark() {
* User-friendly description of the state
*/
String name();
+
+ @Override
+ default void close() {
+ // intentionally left blank
Review Comment:
It seems the default implementation is unnecessary, since all subclasses provide their own implementation. Could you also add comments explaining the overrides?
--
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