Documentation issue maybe

[Romain Gilles <ro...@gmail.com>]

Hi all,
I think the code example in following section has a thread safety issue:
https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/stream/testing.html#junit-rule-miniclusterwithclientresource

The class CollectSink is not thread safe as only the write to the values
collection are synchronized but not the read:
CollectSink.values.containsAll(...). Maybe if you use
a CopyOnWriteArrayList instead of an ArrayList and remove the synchronized
key work it should be ok.

Romain

Re: Documentation issue maybe

[Zhenghua Gao <do...@gmail.com>]

Your are right that it's not thread-safety.
I think we can use Collections.synchronizedList() to get a thread-safety
list[1].
And remove the synchronized keyword from the invoke interface.

I have created a ticket to track this[2], please feel free to fix it by
make a pull request.

[1]
https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#synchronizedList(java.util.List)
[2] https://issues.apache.org/jira/browse/FLINK-14650

*Best Regards,*
*Zhenghua Gao*


On Thu, Nov 7, 2019 at 12:12 AM Romain Gilles <ro...@gmail.com>
wrote:

> Hi all,
> I think the code example in following section has a thread safety issue:
> https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/stream/testing.html#junit-rule-miniclusterwithclientresource
>
> The class CollectSink is not thread safe as only the write to the values
> collection are synchronized but not the read:
> CollectSink.values.containsAll(...). Maybe if you use
> a CopyOnWriteArrayList instead of an ArrayList and remove the synchronized
> key work it should be ok.
>
> Romain
>