Maxim, thank you! Great job! PR looks good to me.
Who else want to take a look? В Ср, 07/08/2019 в 17:01 +0300, Maxim Muzafarov пишет: > Igniters, > > I think the PR [2] of the issue [1] is ready to be merged. > Thanks to Nikolay Izhikov for the great review! > The IEP-28 confluence page [4] is also updated with new API details. > > Will anyone take an additional review of this improvement? > > Some limitations and drawbacks of the current implementation which I > will fix in additional issues: > - CommunicationSpi connections\opened ports are completely reused with > the current value of port range 100. It is better to have a separate > configuration of port ranges for such type of connections (probably > not); > - Files send between cluster nodes by chunks of size defined in > DFLT_CHUNK_SIZE_BYTES constant. Currently, it's not configurable > parameter it is better to move it to configuration; > - Transmission bandwidth limitations are implemented but excluded from > current PR to simplify the review. Will be added in a separate ticket; > > [1] https://issues.apache.org/jira/browse/IGNITE-10619 > [2] https://github.com/apache/ignite/pull/5619 > [3] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1035 > [4] > https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing#IEP-28:Clusterpeer-2-peerbalancing-Filetransferbetweennodes > > On Thu, 30 May 2019 at 20:01, Maxim Muzafarov <maxmu...@gmail.com> wrote: > > > > Folks, > > > > > > The PR [2] has been updated and comments [1] have been addressed. > > If you're interested in this feature, please join the review [3] and > > discussion. > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10619 > > [2] https://github.com/apache/ignite/pull/5619 > > [3] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1035 > > > > On Tue, 21 May 2019 at 16:07, Maxim Muzafarov <maxmu...@gmail.com> wrote: > > > > > > Alexey, > > > > > > Thank you for your feedback. I'll take a bit of time to re-think an > > > API and its usage according to your suggestions and back to you soon > > > with the detailed response. > > > > > > But in general, I'd like to focus your attention on the following idea > > > of this PR, which does not fully correspond to the title of this topic > > > (perhaps I have to change the topic title, but I can't do it anymore). > > > This change is not about providing the raw socket channel to the > > > Supplier for transferring binary data through it, but creating an > > > internal service for sending\receiving files between nodes which can > > > be reused by other Apache Ignite components. I think it's better to > > > move this file transfer machinery outside the supply-demand > > > communication protocol and make it an independent abstraction > > > (service) for sending\receiving any Ignites internal files. > > > From the IEP-28 perspective, the hierarchy of such objects can be: > > > Channel > GridIoManager > Send file Service > Preloader (backup > > > partition and send it to remote the Demander node). > > > > > > Please, correct me if I'm missing something or have some > > > misunderstanding of Ignite internals. > > > > > > On Mon, 20 May 2019 at 11:56, Alexey Goncharuk > > > <alexey.goncha...@gmail.com> wrote: > > > > > > > > Maxim, > > > > > > > > I left response in the ticket. > > > > > > > > пт, 17 мая 2019 г. в 12:04, Maxim Muzafarov <maxmu...@gmail.com>: > > > > > > > > > Igniters, > > > > > > > > > > > > > > > I've implemented the file transfer machinery between grid nodes over > > > > > Communication SPI covered by JIRA [1] and as the first part of IEP-28 > > > > > [3]. Please, consider my PR [2] to be reviewed and included in the > > > > > Apache Ignite project source code. > > > > > > > > > > These changes will allow developer (and the user in future) uploading > > > > > arbitrary files from one cluster node to another. It is a mandatory > > > > > feature for the new rebalance by partition files procedure (you can > > > > > find all the details of it on IEP-28 [3]) but also can be used, for > > > > > example, as a new way of resource deployment (e.g. on > > > > > UriDeploymentSPI). > > > > > > > > > > The starting point of PR reviewing and examples of using the file > > > > > transfer can be -- > > > > > IgniteFileTransmitProcessorSelfTest#testFileHandlerBase. Any feedback > > > > > is really appreciated. Please, don't be silent. > > > > > > > > > > > > > > > PR [2] contains two major changes (components): > > > > > > > > > > 1) Adding channel support for the Communication SPI, which allows > > > > > from now: > > > > > - establishing channel connections to the remote node to an arbitrary > > > > > topic (GridTopic) with predefined channel processing policy; > > > > > - listening to incoming channel creation events and registering > > > > > connection handlers on the particular node; > > > > > - an arbitrary set of channel attributes configured on connection > > > > > handshake (exchanging the channel attributes ); > > > > > The main API changes in the communication component can be found here > > > > > [5]. > > > > > > > > > > 2) Creating a new FileTransmitProcessor, which provide an API for > > > > > sending and receiving files. It supports: > > > > > - using different approaches of incoming data handling – buffered and > > > > > direct (zero-copy approach of FileChannel#transferTo); > > > > > - transferring data by chunks of predefined size with saving > > > > > intermediate results; > > > > > - re-establishing connection if an error occurs and continue file > > > > > upload\download; > > > > > - limiting connection bandwidth (upload and download) at runtime; > > > > > The write\read file handlers API can be found here [6]. > > > > > > > > > > As we discussed before, I've kept all these as the part of the > > > > > internal API, so I'm expecting the merge will be quite safe. > > > > > > > > > > Summary, > > > > > > > > > > JIRA: [1] > > > > > PR: [2] > > > > > Upsource: [4] > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10619 > > > > > [2] https://github.com/apache/ignite/pull/5619 > > > > > [3] > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing#IEP-28:Clusterpeer-2-peerbalancing-Filetransferbetweennodes > > > > > [4] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1035 > > > > > [5] > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing#IEP-28:Clusterpeer-2-peerbalancing-API > > > > > [6] > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing#IEP-28:Clusterpeer-2-peerbalancing-API.1 > > > > > > > > > > > > > > > On Tue, 2 Apr 2019 at 15:37, Nikolay Izhikov <nizhi...@apache.org> > > > > > wrote: > > > > > > > > > > > > Hello, Maxim. > > > > > > > > > > > > Thanks for the update. > > > > > > > > > > > > Please, let me know when PR will be ready. > > > > > > I will do the review. > > > > > > > > > > > > В Вт, 02/04/2019 в 14:52 +0300, Maxim Muzafarov пишет: > > > > > > > Ivan, > > > > > > > > > > > > > > I tend to agree with you. Let's keep changes as an internal Apache > > > > > > > > > > Ignite API. > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > I need your support by reviewing the issue [1] with my changes of > > > > > > > CommunicationSpi about establishing direct socket connections > > > > > > > between > > > > > > > cluster nodes. I've prepared the draft PR [2] with such changes. > > > > > > > I'll > > > > > > > add more JUnit tests to this PR and I'll provide all the > > > > > > > implementation details. > > > > > > > Can anyone help me with the review? > > > > > > > > > > > > > > The issue [1] is related to my work over IEP-28 of rebalancing > > > > > > > cache > > > > > > > data by sending cache partition files (the persistence must be > > > > > > > enabled). You can find the rebalancing process details on the > > > > > > > IEP-28 > > > > > > > page [3]. In general, the whole rebalance by partition files > > > > > > > process > > > > > > > can be divided on five components: > > > > > > > - establishing socket connections ~ 3000 lines (CommunicationSpi) > > > > > > > - create consistent cache partition file ~ 6000 lines > > > > > > > (Checkpointer) > > > > > > > - downloading, uploading partition files ~ 4000 lines > > > > > > > (CachePartitionUploader, CachePartitionDownloader) > > > > > > > - handling cache puts during the partition file download ~ 3000 > > > > > > > lines > > > > > > > (CachupWALManager) > > > > > > > - rebuilding indexes ~ 2000 lines > > > > > > > > > > > > > > All these changes together (single PR) are big and complex, so > > > > > > > better > > > > > > > to review them step by step. And I'd like to find the support of > > > > > > > the > > > > > > > community and ask the review of changes in CommunicationSpi > > > > > > > module of > > > > > > > the Apache Ignite first. > > > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10619 > > > > > > > [2] https://github.com/apache/ignite/pull/5619 > > > > > > > [3] > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing > > > > > > > > > > > > > > On Tue, 5 Mar 2019 at 17:52, Павлухин Иван <vololo...@gmail.com> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > Maxim, > > > > > > > > > > > > > > > > My humble opinion. If there is no convenient means to implement > > > > > > > > partition file sending today then we should introduce > > > > > > > > something. And > > > > > > > > keeping such facility private is much easier, because > > > > > > > > introduction of > > > > > > > > new public API is a significantly more complex task. > > > > > > > > > > > > > > > > пт, 1 мар. 2019 г. в 19:44, Maxim Muzafarov > > > > > > > > <maxmu...@gmail.com>: > > > > > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > > > > > Apache Ignite has a very suitable messaging user interface > > > > > > > > > [1] for > > > > > > > > > topic-based communication between nodes (or a specific group > > > > > > > > > of > > > > > > > > > > nodes > > > > > > > > > within a cluster). The messaging functionality in Ignite is > > > > > > > > > > provided > > > > > > > > > via IgniteMessaging interface. It allows: > > > > > > > > > - send a message to a certain topic > > > > > > > > > - register local\remote listeners > > > > > > > > > > > > > > > > > > I really like this feature, but the disadvantage here is when > > > > > > > > > the > > > > > > > > > > user > > > > > > > > > wants to transfer a large amount of binary data (e.g. files) > > > > > > > > > > between > > > > > > > > > nodes he must create a complex logic to wrap it into > > > > > > > > > messages. I > > > > > > > > > > think > > > > > > > > > Ignite could have an interface e.g. IgniteChannels which will > > > > > > > > > > allow: > > > > > > > > > - register local\remote listeners for channel created\destroy > > > > > > > > > > events. > > > > > > > > > - create a channel connection (a wrapped socket channel) to a > > > > > > > > > > certain > > > > > > > > > node\group of nodes and the desired topic > > > > > > > > > > > > > > > > > > As another suitable case where such a feature can be applied > > > > > > > > > is > > > > > > > > > internal usage for Apache Ignite needs. I can mention here the > > > > > > > > > > task of > > > > > > > > > cluster rebalancing by sending cache partition files between > > > > > > > > > nodes. > > > > > > > > > I've posted a small description of it on the IEP-28 page [2]. > > > > > > > > > > > > > > > > > > > > > > > > > > > WDYT about it? > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > API (assumed) > > > > > > > > > > > > > > > > > > IgniteChannels chnls = ignite0.channels(); > > > > > > > > > chnls.remoteListen(TOPIC.MY_TOPIC, new RemoteListener()); > > > > > > > > > > > > > > > > > > IgniteSocketChannel ch0 = chnls.channel(node, TOPIC.MY_TOPIC); > > > > > > > > > ch0.writeInt(bigFile.size()); > > > > > > > > > ch0.transferTo(FileChannel.open(bigFile.path(), > > > > > > > > > > StandardOpenOption.READ)) > > > > > > > > > > > > > > > > > > > > > > > > > > > /** */ > > > > > > > > > > > > > > > > > > private class RemoteListener > > > > > > > > > implements IgniteBiPredicate<UUID, IgniteSocketChannel> { > > > > > > > > > > > > > > > > > > @IgniteInstanceResource > > > > > > > > > private Ignite ignite; > > > > > > > > > > > > > > > > > > @Override public boolean apply( > > > > > > > > > UUID nodeId, > > > > > > > > > IgniteSocketChannel ch > > > > > > > > > ) { > > > > > > > > > int size = ch.readInt(); > > > > > > > > > ignite.fileSystem("base") > > > > > > > > > .create("bigfile.mpg") > > > > > > > > > .transferFrom(ch, size); > > > > > > > > > return true; > > > > > > > > > } > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] https://apacheignite.readme.io/docs/messaging > > > > > > > > > [2] > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing#IEP-28:Clusterpeer-2-peerbalancing-CommunicationSpi > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Best regards, > > > > > > > > Ivan Pavlukhin
signature.asc
Description: This is a digitally signed message part
