mumrah commented on code in PR #18963:
URL: https://github.com/apache/kafka/pull/18963#discussion_r1970287405
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogLoader.java:
##########
@@ -169,6 +169,7 @@ public LoadedLogOffsets load() throws IOException {
long offset = LogFileUtils.offsetFromFile(file);
if (offset >= minSwapFileOffset && offset <
maxSwapFileOffset) {
logger.info("Deleting segment files {} that is
compacted but has not been deleted yet.", file.getName());
+ @SuppressWarnings("UnusedLocalVariable")
boolean ignore = file.delete();
Review Comment:
Shouldn't we actually check if the file was deleted?
@mimaison I see you worked the Java conversion of this class. WDYT about
checking the result of `delete` and `renameTo` below?
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdater.java:
##########
@@ -885,7 +885,8 @@ public Set<StreamTask> drainRestoredActiveTasks(final
Duration timeout) {
restoredActiveTasksLock.lock();
try {
while (restoredActiveTasks.isEmpty() && now <= deadline) {
- final boolean elapsed =
restoredActiveTasksCondition.await(deadline - now, TimeUnit.MILLISECONDS);
+ @SuppressWarnings("UnusedLocalVariable")
+ final boolean ignored =
restoredActiveTasksCondition.await(deadline - now, TimeUnit.MILLISECONDS);
Review Comment:
@cadonna do we need to check the result of the await here?
##########
tools/src/main/java/org/apache/kafka/tools/ShareConsumerPerformance.java:
##########
@@ -79,7 +79,8 @@ public static void main(String[] args) {
shareConsumers.forEach(shareConsumer ->
shareConsumersMetrics.add(shareConsumer.metrics()));
}
shareConsumers.forEach(shareConsumer -> {
- Map<TopicIdPartition, Optional<KafkaException>> val =
shareConsumer.commitSync();
+ @SuppressWarnings("UnusedLocalVariable")
+ Map<TopicIdPartition, Optional<KafkaException>> ignored =
shareConsumer.commitSync();
Review Comment:
@AndrewJSchofield do we need to check the result of commitSync?
##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/MonitorableSinkConnector.java:
##########
@@ -60,7 +60,7 @@ public void start(Map<String, String> props) {
@Override
public void put(Collection<SinkRecord> records) {
super.put(records);
- for (SinkRecord ignore : records) {
+ for (@SuppressWarnings("UnusedLocalVariable") SinkRecord ignore :
records) {
Review Comment:
Since `records` is a Collection, can we just do `count += records.size()` ?
##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -960,6 +960,7 @@ public void copyLogSegmentsToRemote(UnifiedLog log) throws
InterruptedException
// back to the caller. It's important to
note that the task being executed is already
// cancelled before the executing thread
is interrupted. The caller is responsible
// for handling the exception gracefully
by checking if the task is already cancelled.
+ @SuppressWarnings("UnusedLocalVariable")
boolean ignored =
copyQuotaManagerLockCondition.await(quotaTimeout().toMillis(),
TimeUnit.MILLISECONDS);
Review Comment:
@abhijeetk88 @satishd, is it safe to ignore the result of the await here?
##########
generator/src/main/java/org/apache/kafka/message/checker/EvolutionVerifier.java:
##########
@@ -74,7 +74,7 @@ static void verifyVersionsMatchTopLevelMessage(
verifyVersionsMatchTopLevelMessage(what, topLevelMessage, field);
}
for (StructSpec struct : topLevelMessage.commonStructs()) {
- for (FieldSpec field : topLevelMessage.fields()) {
+ for (FieldSpec field : struct.fields()) {
Review Comment:
@cmccabe or @mannoopj can you comment?
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java:
##########
@@ -2881,7 +2881,7 @@ private void verifySessionPartitions() {
field.setAccessible(true);
LinkedHashMap<?, ?> sessionPartitions =
(LinkedHashMap<?, ?>)
field.get(handler);
- for (Map.Entry<?, ?> entry :
sessionPartitions.entrySet()) {
+ for (@SuppressWarnings("UnusedLocalVariable")
Map.Entry<?, ?> ignored : sessionPartitions.entrySet()) {
Review Comment:
I tried
```
sessionPartitions.forEach((key, value) -> Thread.yield());
```
and the checkstyle did not complain. It seems that the unused check does not
care about lambda arguments.
--
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
