[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16554315#comment-16554315 ] Dmitriy Pavlov commented on IGNITE-8188: Commit h=2e1b4b9cd250f75b09e3641794efb2d6523812b4 - I appologize, commit has incorrect comments, but I hope we can keep it as is. > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16554313#comment-16554313 ] ASF GitHub Bot commented on IGNITE-8188: Github user asfgit closed the pull request at: https://github.com/apache/ignite/pull/4236 > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16554136#comment-16554136 ] Sergey Chugunov commented on IGNITE-8188: - [~NSAmelchev], ZooKeeper suites seems to pass, I don't see any failures that may be caused by your change: [suite1|https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_ZooKeeperDiscovery1_IgniteTests24Java8=pull%2F4236%2Fhead=buildTypeStatusDiv], [suite2|https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_ZooKeeperDiscovery2_IgniteTests24Java8=pull%2F4236%2Fhead=buildTypeStatusDiv]. Lets go ahead with the merge, [~dpavlov], could you please take a look? > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16552679#comment-16552679 ] Sergey Chugunov commented on IGNITE-8188: - [~NSAmelchev], Indeed there is an issue with *getChildrenIfPathExists* method's semantics, lets fix it in IGNITE-8189 as you proposed. I restarted ZooKeeper suites on TC after your latest changes, if no new test failures show up lets merge your change. Thank you for contribution! > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16550684#comment-16550684 ] Amelchev Nikita commented on IGNITE-8188: - [~sergey-chugunov] I have investigated _ZookeeperDiscoverySpiTest.testCommunicationFailureResolve_CachesInfo1_ test. The fail reason is _getChildrenIfPathExists_ method throws NoNode exception. This is in _deleteFutureData_ method where I have removed catching exception after review. I suggest revert it and resolve this problem in IGNITE-8189. I have updated this PR. What do you think? > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16550275#comment-16550275 ] Amelchev Nikita commented on IGNITE-8188: - [~sergey-chugunov], Thank you. I have fixed it. Ran [TC tests|https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F4236%2Fhead] again. Could you review? > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16549342#comment-16549342 ] Sergey Chugunov commented on IGNITE-8188: - [~NSAmelchev], There are two more places in the code that create batches as ArrayLists, I left comments in UpSource review at both lines. Please fix them and we'll proceed with merging. > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16548944#comment-16548944 ] Amelchev Nikita commented on IGNITE-8188: - [~sergey-chugunov], Thank for your comments. I have fixed PR and ran [tests on TC again|https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8_IgniteTests24Java8=pull%2F4236%2Fhead]. One test failed _ZookeeperDiscoverySpiTest.testCommunicationFailureResolve_CachesInfo1_ and it isn't my changes fail by the log. Local test run OK. Could you review? > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16547847#comment-16547847 ] Sergey Chugunov commented on IGNITE-8188: - [~NSAmelchev], Overall improvement looks good to me as well but I left minor comment in UpSource. Also method *ZkDistributedCollectDataFuture#deleteFutureData* still has a catch block for NoNodeException: I think it should be removed as *deleteAll* won't throw it anymore. Lets fix these comments, check TC and merge the change if everything is OK. > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16527569#comment-16527569 ] Vitaliy Biryukov commented on IGNITE-8188: -- [~NSAmelchev], LGTM. > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16527548#comment-16527548 ] Amelchev Nikita commented on IGNITE-8188: - [~sergey-chugunov], I have implemented methods as discussed earlier. Could you review, please? > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > Fix For: 2.7 > > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16524943#comment-16524943 ] Sergey Chugunov commented on IGNITE-8188: - [~NSAmelchev], Both your suggestions make sense to me. I believe that methods called *deleteAll* or *createAll* should do all necessary actions to perform requested actions no matters in fast way (using multi) or by fall-back to one-by-one operations. Lets try to implement these methods in the following way: # they should do max size checks for batching operations and split batches into smaller pieces if necessary; # if batching operation fails for any reason, fallback to one-by-one mode should happen right within these methods. > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16520266#comment-16520266 ] Amelchev Nikita commented on IGNITE-8188: - [sergey-chugunov], I have some questions about the issue, If I understood correctly this methods should need to check request for this limit and if it is exceeded do nothing? I have looked at using these methods and when a batch operation fails it processes one-by-one everywhere. I have suggestions: 1) If the size of the request is exceeded, divide them into valid size batches and try to process by batches. 2) Make methods "max-size" + "NoNodeException/NodeExistsException" safe. If batch operation fails then process one-by-one in this methods. Because request processing one-by-one happens everywhere after failing batch operation it will allow to avoid code duplication and to close other issues(IGNITE-8189, TODO in cleanupPreviousClusterData()). > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-8188) Batching operations should perform check for ZooKeeper request max size
[ https://issues.apache.org/jira/browse/IGNITE-8188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16519275#comment-16519275 ] ASF GitHub Bot commented on IGNITE-8188: GitHub user NSAmelchev opened a pull request: https://github.com/apache/ignite/pull/4236 IGNITE-8188 - split into batches "createAll" and "deleteAll" operations if request size overflow - added tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/NSAmelchev/ignite ignite-8188 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/ignite/pull/4236.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4236 commit df075b8ef16f528c7029dfd01c7aeb7489000b74 Author: NSAmelchev Date: 2018-06-21T12:02:00Z Fix for ignite-8188 > Batching operations should perform check for ZooKeeper request max size > --- > > Key: IGNITE-8188 > URL: https://issues.apache.org/jira/browse/IGNITE-8188 > Project: Ignite > Issue Type: Improvement > Components: zookeeper >Reporter: Sergey Chugunov >Assignee: Amelchev Nikita >Priority: Major > > As ZooKeeper documentation > [says|https://zookeeper.apache.org/doc/r3.4.3/api/org/apache/zookeeper/ZooKeeper.html#multi(java.lang.Iterable)] > batching *multi* operation has a limit for size of a single request. > ZookeeperClient batching methods *createAll* and *deleteAll* should check > this limit and fall back to execute operations one by one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
