[
https://issues.apache.org/jira/browse/IGNITE-3700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15447704#comment-15447704
]
Valentin Kulichenko commented on IGNITE-3700:
---------------------------------------------
Hi Kristian,
Here are my comments regarding your pull request.
# You don't have to catch exception and iterate further if one of the entries
failed. Just let it throw the exception right away like it works right now.
Since you were removing successful entries before that, the resulting
collection will be correct.
# Tests should be added and TeamCity CI should be ran (see
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute).
# There are other implementations of {{CacheStore}} in the product. We need to
make the fix in all of them.
> CacheWriter implementations should remove updated entries from the input
> collection
> -----------------------------------------------------------------------------------
>
> Key: IGNITE-3700
> URL: https://issues.apache.org/jira/browse/IGNITE-3700
> Project: Ignite
> Issue Type: Bug
> Components: cache
> Affects Versions: 1.7
> Reporter: Valentin Kulichenko
> Fix For: 1.8
>
>
> According to {{CacheWriter}} JavaDoc, bulk operations ({{writeAll}} and
> {{deleteAll}}) should remove successful entries from the input collection:
> {code}
> * If this operation fails (by throwing an exception) after a partial
> success,
> * the writer must remove any successfully written entries from the entries
> * collection so that the caching implementation knows what succeeded and
> can
> * mutate the cache.
> {code}
> We properly handle this in the cache store manager and throw
> {{CachePartialUpdateException}} with the list of keys not removed from the
> original collection. However, the implementations provided by Ignite
> ({{CacheStoreAdapter}}, {{CacheAbstractJdbcStore}}, etc.) ignore this and
> simply iterate through entries without removing successful ones.
> Proper implementation should use {{Iterator}} and remove the entry after
> successful update:
> {code}
> @Override public void writeAll(Collection<Cache.Entry<? extends Integer, ?
> extends Integer>> entries) throws CacheWriterException {
> Iterator<Cache.Entry<? extends Integer, ? extends Integer>> it =
> entries.iterator();
> while (it.hasNext()) {
> Cache.Entry<? extends Integer, ? extends Integer> entry = it.next();
> // Do actual write (can throw an exception).
> it.remove();
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
