Re: [PR] [FLINK-37153] Monitor late event count in temporal join [flink]
davidradl commented on PR #25999: URL: https://github.com/apache/flink/pull/25999#issuecomment-2612860451 Reviewed by Chi on 23/01/2025 Need a committer to review -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-37153] Monitor late event count in temporal join [flink]
flinkbot commented on PR #25999: URL: https://github.com/apache/flink/pull/25999#issuecomment-2596014863 ## CI report: * 37241bce72db6788141798ca01889e5443de07ea UNKNOWN Bot commands The @flinkbot bot supports the following commands: - `@flinkbot run azure` re-run the last Azure build -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-37153] Monitor late event count in temporal join [flink]
grzegorz8 commented on code in PR #25999: URL: https://github.com/apache/flink/pull/25999#discussion_r1918734383 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/operators/join/temporal/TemporalRowTimeJoinOperator.java: ## @@ -84,6 +85,9 @@ public class TemporalRowTimeJoinOperator extends BaseTwoInputStreamOperatorWithS private static final String RIGHT_STATE_NAME = "right"; private static final String REGISTERED_TIMER_STATE_NAME = "timer"; private static final String TIMERS_STATE_NAME = "timers"; +private static final String RIGHT_LATE_ELEMENTS_METRIC_NAME = "rightNumLateRecords"; + +private transient Counter rightNumLateRecords; Review Comment: > I notice that we already have numLateRecordsDropped metric. Is this sufficient? Yes, but not for temporal join. There is numLateRecordsDropped metric in window operators or cep operator. What is more, late events ARE NOT dropped in this operator, that is why I named it **rightNumLateRecords~~Dropped~~**. > You are proposing a new metric for a subset of operators (ones with 2 inputs) and only for one side, where all the processing is the same for the left and right sides at the moment. I just wanted to prompt a discussion with this PR :) We can easily add such metric for left side as well. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-37153] Monitor late event count in temporal join [flink]
davidradl commented on code in PR #25999: URL: https://github.com/apache/flink/pull/25999#discussion_r1918687738 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/operators/join/temporal/TemporalRowTimeJoinOperator.java: ## @@ -84,6 +85,9 @@ public class TemporalRowTimeJoinOperator extends BaseTwoInputStreamOperatorWithS private static final String RIGHT_STATE_NAME = "right"; private static final String REGISTERED_TIMER_STATE_NAME = "timer"; private static final String TIMERS_STATE_NAME = "timers"; +private static final String RIGHT_LATE_ELEMENTS_METRIC_NAME = "rightNumLateRecords"; + +private transient Counter rightNumLateRecords; Review Comment: I notice that we already have numLateRecordsDropped metric. Is this sufficient? You are proposing a new metric for a subset of operators (ones with 2 inputs) and only for one side, where all the processing is the same for the left and right sides at the moment. I wonder if the metric should be when the late record is dropped in line with the existing metric We would need documentation to be added for this. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
