[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-05-02 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5934 Thanks! Will merge once my travis run is green. ---

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-05-02 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5934 Thanks for your work! Besides my comments, this looks good 👍 ---

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-05-02 Thread sihuazhou
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5934 Hi @StefanRRichter Thanks for your reply, I have updated the PR and add a test to guard this, but I not sure whether the test is indeed required because it looks a bit wired in my mind... ---

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-05-02 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5934 Yes, there is a theoretical problem if the serialization would not be threadsafe. I think currently the silent assumption that holds is that serializers are immutable w.r.t. serialization

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-04-30 Thread sihuazhou
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5934 Thanks for your comments, @StephanEwen , If I am not misunderstanding , we don't need to duplicate the serializer now, because we will have a dedicated optimization for it in the near future, I am

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-04-30 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5934 Concerning serializer snapshots: - We need to move away from Java Serializing the serializers into the config snapshots anyways and should do that in the near future. - I think the

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-04-29 Thread sihuazhou
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5934 Hi @StefanRRichter , I think I feel lost again... when I writing the comments for the serializer about why we don't duplicate it, I found a loophole there. In theory, even tough the serializer is

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-04-28 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5934 I am also a bit torn. Sometimes I am thinking we might just have a pool with as many serializer copies as se can have concurrent checkpoints + savepoints. But then again, it is borderline to

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-04-28 Thread sihuazhou
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5934 About the serializer duplication problem, I think you are right, duplicating a serialize is not always super cheap, so I think maybe the best tradeoff is to not duplicate the serializer to save

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-04-28 Thread sihuazhou
Github user sihuazhou commented on the issue: https://github.com/apache/flink/pull/5934 Hi @StefanRRichter sorry for the unclearly description here. What this PR trying to fix is the mainly relate to the below code which run async: ```java for (Map.Entry

[GitHub] flink issue #5934: [FLINK-9269][state] fix concurrency problem when performi...

2018-04-28 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5934 Hi, can you give some more detail about the actual problem you are trying to fix here? To me it looks like duplicating the serializer only for the meta data should not be required, because