[GitHub] [flink] gaoyunhaii commented on pull request #14558: [FLINK-18685] [runtime] Make MiniClusterJobClient#getAccumulators non-blocking in Streaming mode

[GitBox <gi...@apache.org>]

gaoyunhaii commented on pull request #14558:
URL: https://github.com/apache/flink/pull/14558#issuecomment-758508420


   > @rmetzger I fixed one of the failing tests. But for _UnalignedCheckpointCompatibilityITCase_ savepoint tests, I know we need to get the accumulators from _JobExecutionResult_ for the test to work. But I struggle to find the **proper condition** when to get the accumulators from _JobExecutionResult_ rather than getting them from the _ExecutionGraph_ as usual. I don't know Flink enough yet, can you give me a clue on this condition?
   
   Hi @echauchot, from my point of view, there would be two possible semantics when users want to acquire accumulators:
   1. Users want to retrieve the current value of accumulators immediately.
   2. Users knows the job is bounded and want to acquire the final accumulator values after the job is finished.
   
   Previously `getAccumulator()` takes the second semantics, and now we want to move it to the first semantics to make it consistent with the other client implementation. Therefore, if users still want to use the second semantics, I think they would need to call `getJobExecutionResult().thenApply(JobExecutionResult::getAllAccumulatorResults)` explicitly. Thus for me to fix the failed tests we need to modify the tests since we would not be able to distinguish the first and second semantics automatically. What do you think about it ?


----------------------------------------------------------------
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:
users@infra.apache.org