becketqin commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-725188295
@StephanEwen Thanks a lot for the review. I have addressed most of them. Regarding the questions about sourceIdleTime. >Can we deactivate this during batch processing? It seems less valuable there, and we have been very performance conscious for batch workloads. It seems that the cost of skipping updating this metric could be the same or even more than just updating the idle time which is a local variable, because the skipping code would likely result in an if-else branch that involves reading from a variable and branch prediction logic in the CPU. One change I made in the last update is putting the `sourceIdleTime = -1L` right after we got a non-null recordsWithSplitIds. So the sourceIdleTime will only be updated once per batch, instead of once per record. I think this should have little performance impact on the current code, if any. >The metric measures the time since last record. That metric fluctuates heavily, resets to zero multiple times per second. A lot of the actual information (how much idleness there was in total) will be lost by the time the metric is sampled. The mailbox idleness tries to measure the average idleness fraction of the total processing time. Is that something that would be good for this metric as well, or should it stay as it is? I think the idleness fraction is particularly useful to measure the workload of a thread. In the source case, users usually care more about for how long there is no data processed from a source. I don't think this metric is critical, but could help clarify the latency issues in some cases, especially when some connectors do not emit `fetchEventTimeLag` by default. In that case, when the `emitEventTimeLag` reports a large value, we need to know whether it is because Flink was not able to get the data from the external system, or it is because the `RecordEmitter` took long to process the record. The `sourceIdleTime` would be helpful in that case. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
