Copilot commented on code in PR #20464:
URL: https://github.com/apache/kafka/pull/20464#discussion_r2318793769
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/DefaultStreamsRebalanceListener.java:
##########
@@ -91,16 +107,21 @@ public Optional<Exception> onTasksAssigned(final
StreamsRebalanceData.Assignment
taskManager.handleRebalanceComplete();
} catch (final Exception exception) {
return Optional.of(exception);
+ } finally {
+ tasksAssignedSensor.record(time.milliseconds() - start);
}
return Optional.empty();
}
@Override
public Optional<Exception> onAllTasksLost() {
+ final long start = time.milliseconds();
try {
taskManager.handleLostAll();
} catch (final Exception exception) {
return Optional.of(exception);
+ } finally {
+ tasksLostSensor.record(time.milliseconds() - start);
}
Review Comment:
Similar to onTasksAssigned, this method records latency metrics even when
exceptions occur. The finally block will execute regardless of whether the
operation completed successfully or failed, which may not provide accurate
latency measurements for successful operations.
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/DefaultStreamsRebalanceListener.java:
##########
@@ -78,6 +93,7 @@ public Optional<Exception> onTasksRevoked(final
Set<StreamsRebalanceData.TaskId>
@Override
public Optional<Exception> onTasksAssigned(final
StreamsRebalanceData.Assignment assignment) {
+ final long start = time.milliseconds();
try {
Review Comment:
The start time is captured outside the try block, but if an exception occurs
during the method execution, the sensor will still record the latency in the
finally block. This could lead to recording metrics for failed operations that
didn't complete normally, potentially skewing the latency measurements.
--
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]