By the way, did we have a change to run some existing benchmarks against proposed implementation?
пт, 11 окт. 2019 г. в 17:49, Alexey Goncharuk <alexey.goncha...@gmail.com>: > > Hello Mulugeta, Igniters. > > Thanks for your interest and efforts in integrating the persistent memory > to Ignite. Here are my thoughts on the PR: > > First of all I was questioning whether we want to use the integration with > the pmem library via JNI. Java 14 will have native support for NVME via > native mapped byte buffers [1] since the corresponding tickets are already > resolved [2],[3]. The suggested integration uses JNI calls quite often (if > I read the PR right, there will be tens of JNI calls per a single > operation), which may and most likely will undermine the benefits of using > PM. On the other hand, there is a PoC/project [4] which successfully > demonstrates how the new integration can be used to natively access > persistent memory from Java. Additionally, new Unsafe have several > performance issues - each put* methods has a map lookup, and CAS method > uses a global lock. Removing these issues will change the PR and > architecture dramatically. > > Other than performance and an extra library dependency, there are > additional reasons to use custom implementation of PM persistence > primitives: > > - Ignite significantly benefits from having WAL as a single point of > data for both failure recovery and historical rebalance. As far as I can > guess, the suggested library also uses some sort of journaling in order to > support crash recovery. Since we do not want give up on the historical > rebalance, we would need to write an additional journal, which then raises > a question of how to transact between this journal. > - Ignite is supposed to work with large indexes, a I not aware how LLPL > handles them. Several researches [5],[6] show that since PM memory has > larger access latency than regular RAM, it is beneficial to buffer index > pages in regular memory and store leaf pages in PM. There is a generic > approach to convert in-memory indexes to PM ones [7], I think it makes > sense to investigate whether our existing indexes can be converted to PM. > - There is an active IEP which is targeted for a file-based rebalancing. > In the suggested implementation this optimization would not work. A better > integration should allow to take a snapshot of a partition and transfer it > to another node for fast rebalancing. > > Overall, I think we may take the following path: > > - Target JEP352 APIs; for now keep using the whole region msync for flush > - Introduce PM-based isolated components to support WAL iterator and > transaction management. We can start with WAL, then implement a PM-based > index based on current research, then a tuple heap with free space > management. Each of the components may be developed and tested > independently > - Do Ignite internals refactoring to properly abstract storage layer > from transactions/communication. Currently, there is an abstraction leak in > the storage layer, which makes it aware of particular Ignite implementation > details (Binary Objects, partition eviction, etc) > - Once the storage is separated, we should be able to plug the new > components instead of the "classical" ones > > WDYT? I believe we should create a detailed IEP with concrete proposals on > both PM structures design and Ignite internals changes before making any > further code changes. > > [1] https://openjdk.java.net/jeps/352 > [2] https://bugs.openjdk.java.net/browse/JDK-8221397 > [3] https://bugs.openjdk.java.net/browse/JDK-8221696 > [4] https://github.com/jhalliday/mashona > [5] https://www.usenix.org/system/files/conference/fast17/fast17-lee.pdf > [6] http://www.vldb.org/pvldb/vol8/p786-chen.pdf > [7] https://www.cs.utexas.edu/~vijay/papers/sosp19-recipe.pdf > [8] > https://cwiki.apache.org/confluence/display/IGNITE/IEP-16%3A+Optimization+of+rebalancing > > вт, 8 окт. 2019 г. в 08:15, Alexey Goncharuk <alexey.goncha...@gmail.com>: > > > Igniters, > > > > I would like to resurrect this discussion and will review the change again > > shortly. If anyone want to join the review - you are welcome! > > > > ср, 22 авг. 2018 г. в 18:49, Denis Magda <dma...@apache.org>: > > > >> Hi Dmitry, > >> > >> That's a BSD-3-Clause license if to believe this statement > >> "SPDX-License-Identifier: BSD-3-Clause": > >> https://github.com/pmem/llpl/blob/master/LICENSE > >> > >> This license can be used with ASF software: > >> https://www.apache.org/legal/resolved.html#category-a > >> > >> -- > >> Denis > >> > >> On Wed, Aug 22, 2018 at 9:28 AM Dmitriy Pavlov <dpavlov....@gmail.com> > >> wrote: > >> > >> > Hi Denis, > >> > > >> > Could you please double check if we can refer to any library licensed to > >> > Intel. Can we develop code only version of this support (without > >> shipping > >> > it in release)? > >> > > >> > https://github.com/apache/ignite/pull/4381 is quite huge change, > >> > including 128 files changed, patch review will require resources from > >> > community members to review. I would like to be sure we can include this > >> > patch from the legal point of view. > >> > > >> > Sincerely, > >> > Dmitriy Pavlov > >> > > >> > пт, 3 авг. 2018 г. в 19:23, Dmitriy Pavlov <dpavlov....@gmail.com>: > >> > > >> >> Hi Mulugeta, > >> >> > >> >> I appologise, I've missed that license is already there. So I guess it > >> is > >> >> not standard open-source license, it is seems it is not listed in > >> >> https://www.apache.org/legal/resolved.html#category-a > >> >> > >> >> So there can be legal concern related to including this lib as > >> dependency > >> >> into Apache product. It should not block review, we can later > >> >> consult Secretary/Legal to find out how we can correctly include > >> reference > >> >> to lib. > >> >> > >> >> Sincerely, > >> >> Dmitriy Pavlov > >> >> > >> >> чт, 2 авг. 2018 г. в 0:24, Mammo, Mulugeta <mulugeta.ma...@intel.com>: > >> >> > >> >>> Hi Dmitriy, > >> >>> > >> >>> Do you mean our LLPL library? It has a license, please look here: > >> >>> https://github.com/pmem/llpl > >> >>> > >> >>> Regarding the changes made to Ignite, you may refer to the pull > >> request > >> >>> here: https://github.com/apache/ignite/pull/4381 > >> >>> > >> >>> Thanks, > >> >>> Mulugeta > >> >>> > >> >>> -----Original Message----- > >> >>> From: Dmitriy Pavlov [mailto:dpavlov....@gmail.com] > >> >>> Sent: Wednesday, August 1, 2018 10:49 AM > >> >>> To: dev@ignite.apache.org > >> >>> Subject: Re: Adding experimental support for Intel Optane DC > >> Persistent > >> >>> Memory > >> >>> > >> >>> Hi Mulugeta Mammo, > >> >>> > >> >>> I've just noticed that repository, what you refer is full fork of > >> Ignite. > >> >>> How can I see differences with original Ignite? > >> >>> > >> >>> One more thing, library which you're referencing seems to not contain > >> >>> license, at least github can not parse it. Apache product has > >> limitations > >> >>> which libraries may be used (see > >> >>> https://www.apache.org/legal/resolved.html#category-a and > >> >>> https://www.apache.org/legal/resolved.html#category-b) > >> >>> > >> >>> Could you please comment if there is some legal risk? > >> >>> > >> >>> Sincerely, > >> >>> Dmitriy Pavlov > >> >>> > >> >>> ср, 1 авг. 2018 г. в 20:43, Dmitriy Pavlov <dpavlov....@gmail.com>: > >> >>> > >> >>> > Hi, > >> >>> > > >> >>> > This link works for me > >> >>> > > >> >>> > > >> https://cwiki.apache.org/confluence/display/IGNITE/IEP-26%3A+Adding+Ex > >> >>> > perimental+Support+for+Intel+Optane+DC+Persistent+Memory > >> >>> > > >> >>> > Sincerely, > >> >>> > Dmitriy Pavlov > >> >>> > > >> >>> > чт, 26 июл. 2018 г. в 15:31, Stanislav Lukyanov < > >> >>> stanlukya...@gmail.com>: > >> >>> > > >> >>> >> Ah, ok, it’s just the ‘.’ at the end of the link. Removed it and > >> it’s > >> >>> >> fine. > >> >>> >> > >> >>> >> From: Stanislav Lukyanov > >> >>> >> Sent: 26 июля 2018 г. 15:12 > >> >>> >> To: dev@ignite.apache.org > >> >>> >> Subject: RE: Adding experimental support for Intel Optane DC > >> >>> >> Persistent Memory > >> >>> >> > >> >>> >> Hi, > >> >>> >> > >> >>> >> The link you’ve shared gives me 404. > >> >>> >> Perhaps you need to add a permission for everyone to access the > >> page? > >> >>> >> > >> >>> >> Thanks, > >> >>> >> Stan > >> >>> >> > >> >>> >> From: Mammo, Mulugeta > >> >>> >> Sent: 26 июля 2018 г. 2:44 > >> >>> >> To: dev@ignite.apache.org > >> >>> >> Subject: Adding experimental support for Intel Optane DC Persistent > >> >>> >> Memory > >> >>> >> > >> >>> >> Hi, > >> >>> >> > >> >>> >> I have added a new proposal to support Intel Optane DC Persistent > >> >>> >> Memory for Ignite here: > >> >>> >> > >> https://cwiki.apache.org/confluence/display/IGNITE/Adding+Experimenta > >> >>> >> l+Support+for+Intel+Optane+DC+Persistent+Memory > >> >>> >> . > >> >>> >> > >> >>> >> I'm looking forward to your feedback and collaboration on this. > >> >>> >> > >> >>> >> Thanks, > >> >>> >> Mulugeta > >> >>> >> > >> >>> >> > >> >>> >> > >> >>> >> > >> >>> > >> >> > >> > > -- Best regards, Ivan Pavlukhin
