Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultWriteBehindProcessor: do not add whole batchMap in failureList in case of failure #9733

Closed
dsukhoroslov opened this issue Jan 23, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@dsukhoroslov
Copy link
Contributor

commented Jan 23, 2017

when we use storeAll/deleteAll methods in MapStore interface, it is possible that underlying implementation has stored/deleted some entries from the supplied batch and then gets an exception on some particular entry in the middle of the batch. In order to handle this scenario properly the MapStore could remove from the supplied batch entries which were properly stored in the underlying persistent store. The only thing to improve in the DefaultWiteBehindProcessor is to properly synchronise the map passed to processBatch method with the initial batchMap (https://github.com/hazelcast/hazelcast/blob/master/hazelcast/src/main/java/com/hazelcast/map/impl/mapstore/writebehind/DefaultWriteBehindProcessor.java#L243):

            @Override
            public boolean run() throws Exception {
                callBeforeStoreListeners(batchMap.values());
                final Map map = convertToObject(batchMap);
                try {
                    final boolean result = operationType.processBatch(map, mapStore);
                } catch (Exception ex) {
                    // leave in batchMap only entries which were left in the map...
                   throw ex;
                }
                callAfterStoreListeners(batchMap.values());
                return result;
            }

I can provide PR for this, if you wish.

Thanks, Denis.

@ahmetmircik

This comment has been minimized.

Copy link
Member

commented Jan 23, 2017

Hi @dsukhoroslov, please go ahead and send your pr.

@dsukhoroslov

This comment has been minimized.

Copy link
Contributor Author

commented Jan 23, 2017

in which version you expect to include it? 3.7.5 would be great

@mdogan

This comment has been minimized.

Copy link
Member

commented Jan 23, 2017

@dsukhoroslov; 3.7.5 has passed the QA process and scheduled for release already.

You can send a PR based on 3.7.6 (maintenance-3x branch). Also please make a forward-port for 3.8 (master) too.

@dsukhoroslov

This comment has been minimized.

Copy link
Contributor Author

commented Jan 23, 2017

the PR is #9738. Will port it to master when it'll be agreed.

@ahmetmircik

This comment has been minimized.

Copy link
Member

commented Jan 24, 2017

since this is an enhancement, only a pr for master branch will be ok.

@dsukhoroslov

This comment has been minimized.

Copy link
Contributor Author

commented Jan 24, 2017

but we need it in the 3.7.x version, asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.