ignite-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Harvey <dhar...@jobcase.com>
Subject Re: IgniteDataStreamer.flush() returns before all futures are completed
Date Thu, 26 Apr 2018 17:47:08 GMT
"Could you please refine what kind of flaw do you suspect? "
I think the basic flaw is that it is unclear how to write a simple task to
stream in some data, and then confirm that all of that data was
successfully  stored before taking an action to record that.   This may be
simply a documentation issue for flush(), where I can't tell what the
intended design would be for such code.

We ran into this issue because we assumed that we needed to test the status
of all of the futures returned by addData, and we found that the listener
was not always called before flush() returned.

As I dig deeper into the code, I see an attempt to cause flush() to throw
an exception if any exception was thrown on the server for *any* prior
record.   If that is the intent (which is not stated but would be
convenient),  then I think there is a race:

   - DataStreamerImpl.flush() calls EnterBusy while activeFuts is non
   empty.  This seems to be the last test of "cancelled".   If there were
   failed requests before this, flush() would throw.
   - Before doFlush() looks at activeFuts, activeFuts becomes empty,
   because the requests failed.
   - flush() returns without throwing an exception.


-DH


On Thu, Apr 26, 2018 at 10:23 AM, Andrey Kuznetsov <stkuzma@gmail.com>
wrote:

> Hi, David,
>
> As far as a can see from streamer implementation, flush() call
> 1. won't return until all futures returned by addData() are (successfully)
> finished
> 2. will throw an exception if at least one future has been failed.
>
> Could you please refine what kind of flaw do you suspect?
>
> As for callback added by listen(), it is called right _after_ the future
> gets to 'done' state, thus flush() completion does not guarantee callback's
> apply() completion.
>
>
> 2018-04-26 16:37 GMT+03:00 David Harvey <dharvey@jobcase.com>:
>
>> Thanks Yakov.   I think this is more subtle.
>>
>> Our loading via IgniteDatastreamer is idempotent, but this depends on
>> being certain that a batch of work has successfully completed.   It is
>> *not* sufficient for us to listen to the futures returned by addData,
>> then to call flush(), and then to record success if there have been no
>> exceptions.  We must wait until get() is called on every future before
>> recording that the operation was successful.    The fact that the future is
>> done is not sufficient, we need to know that it is done and there is no
>> exception.   We can call flush and then do a future.get() on the incomplete
>> futures, but not as an assert.   (It is valid to assert that fut.isDone(),
>> but that is not sufficient.)
>>
>> Based on by current understanding, I think this is a flaw in Ignite, even
>> if the fix might only be to clarify the comments for flush() to make this
>> behavior clear.
>>
>>
>>
> --
> Best regards,
>   Andrey Kuznetsov.
>

Disclaimer

The information contained in this communication from the sender is confidential. It is intended
solely for use by the recipient and others authorized to receive it. If you are not the recipient,
you are hereby notified that any disclosure, copying, distribution or taking action in relation
of the contents of this information is strictly prohibited and may be unlawful.

This email has been scanned for viruses and malware, and may have been automatically archived
by Mimecast Ltd, an innovator in Software as a Service (SaaS) for business. Providing a safer
and more useful place for your human generated data. Specializing in; Security, archiving
and compliance. To find out more visit the Mimecast website.

Mime
View raw message