[jira] [Comment Edited] (IGNITE-8406) Update IgniteDataStreamer.flush() javadoc
[
https://issues.apache.org/jira/browse/IGNITE-8406?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16488740#comment-16488740
]
Ivan Fedotov edited comment on IGNITE-8406 at 5/24/18 10:03 AM:
[~dpavlov], Hello!
Could you please look at this ticket?
It seems to me that javaDoc of IgniteDataStreamer not entirely correct.
Mapping keys to node happens in load0 method[1] which invoked in addData method
while loading data from buffers always happens in flush[2] and tryFlush[3]
methods. Just about same situation in close(boolean) method. So I think minor
javaDoc changes are appropriate here.
Also according to conversation on user list[4] it would be better to explicitly
indicate about listeners.
[1][https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerImpl.java#L872]
[2][https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerImpl.java#L1117]
[3][https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerImpl.java#L1227]
[4][http://apache-ignite-users.70518.x6.nabble.com/IgniteDataStreamer-flush-returns-before-all-futures-are-completed-td21330.html]
was (Author: ivanan.fed):
[~dpavlov], Hello!
Could you please see at this ticket?
It seems to me that javaDoc of IgniteDataStreamer not entirely correct.
Mapping keys to node happens in load0 method[1] which invoked in addData method
while loading data from buffers always happens in flush[2] and tryFlush[3]
methods. Just about same situation in close(boolean) method. So I think minor
javaDoc changes are appropriate here.
Also according to conversation on user list[4] it would be better to explicitly
indicate about listeners.
[1][https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerImpl.java#L872]
[2][https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerImpl.java#L1117]
[3][https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerImpl.java#L1227]
[4][http://apache-ignite-users.70518.x6.nabble.com/IgniteDataStreamer-flush-returns-before-all-futures-are-completed-td21330.html]
> Update IgniteDataStreamer.flush() javadoc
> -
>
> Key: IGNITE-8406
> URL: https://issues.apache.org/jira/browse/IGNITE-8406
> Project: Ignite
> Issue Type: Task
> Components: streaming
>Affects Versions: 2.4
>Reporter: Andrey Kuznetsov
>Assignee: Ivan Fedotov
>Priority: Minor
> Fix For: 2.6
>
>
> Current {{flush()}} implementation can throw {{CacheException}} if one or
> more futures previously returned by {{addData()}} have been completed
> exceptionally. This behavior should be described in {{IgniteDataStreamer}}
> javadoc. Also it's worth noting explicitly that all futures completion upon
> return from {{flush}} does not imply all those future listeners have been
> completed by this moment.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (IGNITE-8406) Update IgniteDataStreamer.flush() javadoc
[
https://issues.apache.org/jira/browse/IGNITE-8406?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16456493#comment-16456493
]
David Harvey edited comment on IGNITE-8406 at 4/27/18 2:06 PM:
---
I think we are saying the same thing: The current post condition on flush()
is that all futures are done but the apply function has not necessarily been
called for all of them, and nothing about successful storing of the data can be
inferred by flush() completing without throwing. Simply making those clear
in the Javadoc would be a sufficient solution.
If there was a desire to make a stronger statement about what you can assume
from flush() completing w/o throwing, I think there would need to be a code
minor change.
was (Author: syssoftsol):
The current post condition on flush() is that all futures are done but the
apply function has not necessarily been called for all of them, and nothing
about successful storing of the data can be inferred by flush() completing
without throwing. Simply making those clear in the Javadoc would be a
sufficient solution.
If there was a desire to make a stronger statement about what you can assume
from flush() completing w/o throwing, I think there would need to be a code
minor change.
> Update IgniteDataStreamer.flush() javadoc
> -
>
> Key: IGNITE-8406
> URL: https://issues.apache.org/jira/browse/IGNITE-8406
> Project: Ignite
> Issue Type: Task
> Components: streaming
>Affects Versions: 2.4
>Reporter: Andrey Kuznetsov
>Priority: Minor
> Fix For: 2.6
>
>
> Current {{flush()}} implementation can throw {{CacheException}} if one or
> more futures previously returned by {{addData()}} have been completed
> exceptionally. This behavior should be described in {{IgniteDataStreamer}}
> javadoc.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (IGNITE-8406) Update IgniteDataStreamer.flush() javadoc
[
https://issues.apache.org/jira/browse/IGNITE-8406?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16454795#comment-16454795
]
David Harvey edited comment on IGNITE-8406 at 4/26/18 8:15 PM:
---
I believe that the code will throw a {{CacheException as described, except I
think there is a small window where it will not.}}
* DataStreamerImpl.flush() calls EnterBusy while activeFuts is non empty.
This seems to be the last test of "cancelled" in EnterBusy. If there were
exceptional completions before this, flush() will throw due to the code in
EnterBusy().
* Before doFlush() looks at activeFuts, activeFuts becomes empty because a
buffer asynchronously had an exceptional completion. Because activeFuts is
empty, doFlush and flush will return without an exception, even though some
futures previously returned by addData() have had exceptions thrown.
In addition to the documentation change, an inelegant fix would be to call
"EnterBusy();leaveBusy();" at the end of flush()
was (Author: syssoftsol):
I believe that the code will throw a throw {{CacheException as described,
except I think there is a small window where it will not.}}
* DataStreamerImpl.flush() calls EnterBusy while activeFuts is non empty.
This seems to be the last test of "cancelled" in EnterBusy. If there were
exceptional completions before this, flush() will throw due to the code in
EnterBusy().
* Before doFlush() looks at activeFuts, activeFuts becomes empty because a
buffer asynchronously had an exceptional completion. Because activeFuts is
empty, doFlush and flush will return without an exception, even though some
futures previously returned by addData() have had exceptions thrown.
In addition to the documentation change, an inelegant fix would be to call
"EnterBusy();leaveBusy();" at the end of flush()
> Update IgniteDataStreamer.flush() javadoc
> -
>
> Key: IGNITE-8406
> URL: https://issues.apache.org/jira/browse/IGNITE-8406
> Project: Ignite
> Issue Type: Task
> Components: streaming
>Affects Versions: 2.4
>Reporter: Andrey Kuznetsov
>Priority: Minor
> Fix For: 2.6
>
>
> Current {{flush()}} implementation can throw {{CacheException}} if one or
> more futures previously returned by {{addData()}} have been completed
> exceptionally. This behavior should be described in {{IgniteDataStreamer}}
> javadoc.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
