dawidwys commented on a change in pull request #17946:
URL: https://github.com/apache/flink/pull/17946#discussion_r759179156
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
##########
@@ -1882,9 +1882,11 @@ int getNumQueuedRequests() {
public void reportStats(long id, ExecutionAttemptID attemptId,
CheckpointMetrics metrics)
throws CheckpointException {
if (statsTracker != null) {
- attemptMappingProvider
- .getVertex(attemptId)
- .ifPresent(ev -> statsTracker.reportIncompleteStats(id,
ev, metrics));
+ synchronized (lock) {
Review comment:
I am a bit concerned we do synchronize too much. I don't like reporting
the stats happen inside of the lock as well. We should not delay the
checkpointing process for the sake of stats. The stats tracker already supports
dirty reads, which we should not discard I believe.
What do you think about fixing it inside of the
`ExecutionAttemptMappingProvider`? I think we could improve that class to be
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]
