[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user zentol commented on the issue: https://github.com/apache/flink/pull/4665 merging. ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user Aitozi commented on the issue: https://github.com/apache/flink/pull/4665 Hi @zentol , can you help merge this pr ? Is this be forgetten ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user Aitozi commented on the issue: https://github.com/apache/flink/pull/4665 is there still anything wrong @zentol ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/4665 LGTM ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user Aitozi commented on the issue: https://github.com/apache/flink/pull/4665 is this ok to be merged in @bowenli86 @zentol @aljoscha ? ð ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/4665 @aljoscha sorry for being late the this CR. I left a couple comments ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/4665 This looks good to merge now. ð ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user zentol commented on the issue: https://github.com/apache/flink/pull/4665 Would be good to rename the metric to `numLateRecordsDropped` to be more consistent with existing metrics. @aljoscha will have to comment on whether the counting is correct or not. It may also be interesting to count the number of late elements that are NOT dropped. ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user Aitozi commented on the issue: https://github.com/apache/flink/pull/4665 Hello,Is it can be merged in @aljoscha @zentol ? ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user Aitozi commented on the issue: https://github.com/apache/flink/pull/4665 @aljoscha yes, i review the code this day, it will jude each window whether late , so the previous method i use will counts more lost data than the actual situation , i have fix the error and re-push, please help me review the code again, thanks. ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/4665 @Aitozi I meant that the place where you're currently counting dropped elements will not yield a correct count because one element might be in several windows. The place were we side-output late data is also the place to count and here we can decide if we want to count late data as dropped if we also side-output it. I think it's a question of personal preference. ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user Aitozi commented on the issue: https://github.com/apache/flink/pull/4665 @aljoscha i agree that the name set to be "numLateElementsDropped", and do you mean that my result should minus the num of element that go to side output which is skipped and lateElement? ---
[GitHub] flink issue #4665: [Flink-7611]add metrics to measure the num of data droppe...
Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/4665 These are not actually the places where you can count dropped data. I would suggest to add this at the very end of `processElement()` where we also check whether we should side-output late data: https://github.com/apache/flink/blob/6642768ad8f8c5d1856742a6d148f7724c20666c/flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/operators/windowing/WindowOperator.java#L406. Also, I think a better metrics name would include "dropped" somehow, because right now it just says late. Something like `numLateElementsDropped`. ---
