"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 <stku...@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 <dhar...@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.
