davidradl commented on code in PR #26708:
URL: https://github.com/apache/flink/pull/26708#discussion_r2161604759


##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/operators/join/lookup/keyordered/Epoch.java:
##########
@@ -72,6 +74,7 @@ public void setOutput(Consumer<StreamElementQueueEntry<OUT>> 
outputConsumer) {
 
     public void decrementCount() {
         ongoingRecordCount--;
+        Preconditions.checkState(ongoingRecordCount >= 0);

Review Comment:
   I am looking for the details of the inconsistency. I cannot find a 
description in the PR or the Jira. It seems odd to call decrementCount. Some 
thoughts: 
   -I wonder why the caller cannot check that the decrementCount should not be 
called if there is nothing to decrement.   
   - It would seem more fitting to do the precondition check before the 
decrement. As it is pre check.  Preconditions.checkState(ongoingRecordCount > 
0);
   - if this is to cope with multiple threads accessing the variable then the 
logic should be made thread safe. 



-- 
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]

Reply via email to