rkhachatryan commented on pull request #13648:
URL: https://github.com/apache/flink/pull/13648#issuecomment-713798578


   Thanks a lot for the in-depth analysis and for updating the PR!
   It answered my original question about thread-safety.
   
   But I still have two other concerns:
   1. Visilibity in normal case: none of the felds written in `releaseView` are 
`volatile`. So in normal case (`t1:release` then `t2:createReadView`) `t2` can 
see some inconsistent state. For example, `readView == null`, but 
`isPartialBufferCleanupRequired == false`. Right?
   Maybe call `releaseView()`  from `createReadView()` unconditionally?
   
   2. Overwites when release is slow: won't `t1` overwrite changes to 
`PipelinedSubpartition` made already by `t2`? For example, reset 
`sequenceNumber` after `t2` has sent some data?
   Maybe `PipelinedSubpartition.readerView` should be `AtomicReference` and 
then we can guard `PipelinedApproximateSubpartition.releaseView()` by CAS on 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:
us...@infra.apache.org


Reply via email to