[GitHub] [flink] wpc009 edited a comment on pull request #18475: [FLINK-25728] Protential memory leaks in StreamMultipleInputProcessor

[GitBox]

wpc009 edited a comment on pull request #18475:
URL: https://github.com/apache/flink/pull/18475#issuecomment-1024372868


   > As far as I can tell, the case in the 3rd chart is not working in your 
version. If `Complete` action happens at the very very end (around `to here` 
arrow), there is no guarantee that it will "complete the newly created future". 
As you haven't established happens-before relationship between `Reset` and 
`Complete` actions, `Complete` might see an outdated, already completed, 
version of the `availableFuture`, failing to wake up the task thread.
   
   Can I understand this way? What you are saying is If `Complete` happens at 
the very end. And, not using `volatile` causing the thread that running the 
`notifyCompletion` could not see the new `availableFuture` and it tries to 
complete the obsolete future leaving the newly created future untouched. That 
cause the stream hang-up. 
   
   I would say without `volatile` that could happen. But, before entry 
`notifyCompletion`, the input's future is already completed.  At the `anyOf` 
stage, it will check the future of the input where is correctly completed. And 
thus triggered the following `availableFuture.complete()`. It will not halt. 
   
   I can make the availableFuture `volatile`. In that way, the `Complete` will 
not able to see the obsolete future.
   
   I'm running the `MultipleInputBenchmark` test for hours. No stream hang-up 
is show up.
   <img width="297" alt="image" 
src="https://user-images.githubusercontent.com/2689362/151582292-01aacf62-2bff-4fb7-a160-ec40552e69cd.png";>


-- 
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