Possible race condition in StreamRecordWriter

[Ufuk Celebi]

Hey all (but mostly the streaming folks),

while refactoring the writers for the runtime changes (FLINK-986) I discovered a possible race condition in StreamRecordWriter [1].

The problem is that the record writer is not thread-safe, but both the streaming task vertex and the OutputFlusher thread use it concurrently.

Am I overlooking something and is it safe to use it this way? If not: should I ensure that record writer is thread-safe with my upcoming changes?

– Ufuk

[1] https://github.com/apache/incubator-flink/blob/b904b0041cf97b2c6181b1985afc457ed01cf626/flink-addons/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/io/StreamRecordWriter.java

[Gyula Fóra]

So both the emit and flush methods have been modified to make sure that
only one accesses the serializer at the same time. (with the synchronized
blocks) So since the output flusher only flushes every so many millis, then
this worked for us so far. (and I think it should work) We haven't had any
issues with this either locally or on the cluster.



On Wed, Oct 15, 2014 at 5:18 PM, Ufuk Celebi <[hidden email]> wrote:

> Hey all (but mostly the streaming folks),
>
> while refactoring the writers for the runtime changes (FLINK-986) I
> discovered a possible race condition in StreamRecordWriter [1].
>
> The problem is that the record writer is not thread-safe, but both the
> streaming task vertex and the OutputFlusher thread use it concurrently.
>
> Am I overlooking something and is it safe to use it this way? If not:
> should I ensure that record writer is thread-safe with my upcoming changes?
>
> – Ufuk
>
> [1]
> https://github.com/apache/incubator-flink/blob/b904b0041cf97b2c6181b1985afc457ed01cf626/flink-addons/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/io/StreamRecordWriter.java

[Ufuk Celebi-2]

On Wednesday, October 15, 2014, Gyula Fóra <[hidden email]> wrote:

> So both the emit and flush methods have been modified to make sure that
> only one accesses the serializer at the same time. (with the synchronized
> blocks)


I didn't see the synchronized blocks, so it should be fine. This also
explains why you copied the methods. Sorry for the false alarm. ;)