Alexey, For me, all three look quite critical as they: address a memory leak, address a usability/performance issue with no workaround, and address a regression in 2.8. If we want to limit the scope, I would still include the memory leak fix, though.
Thoughts? пн, 12 окт. 2020 г. в 13:07, Alex Plehanov <plehanov.a...@gmail.com>: > Guys, > > I've found 3 more tickets which were targeted to 2.9 but merged only to the > master branch ([1], [2], [3]). > Do we need these tickets in 2.9? Are they critical? > > [1]: https://issues.apache.org/jira/browse/IGNITE-12350 > [2]: https://issues.apache.org/jira/browse/IGNITE-13376 > [3]: https://issues.apache.org/jira/browse/IGNITE-13280 > > вс, 11 окт. 2020 г. в 16:11, Nikolay Izhikov <nizhi...@apache.org>: > > > Igniters, > > > > IGNITE-13553 fixed and cherry-picked to 2.9. > > We can continue with the release. > > > > > 10 окт. 2020 г., в 00:00, Denis Magda <dma...@apache.org> написал(а): > > > > > > Nikolay, > > > > > > re: the minor improvements. I'm not against of including those if we > can > > > prepare the docs before the vote starts. Presently, the docs are > "frozen" > > > for the 2.9 release, but I can scratch some time and take part in the > > docs > > > review the next week. > > > > > > - > > > Denis > > > > > > > > > On Fri, Oct 9, 2020 at 12:40 AM Nikolay Izhikov <nizhi...@apache.org> > > wrote: > > > > > >> Hello, Igniters. > > >> > > >> I prepared a patch [1] for blocker ticket [2] - «Server node fail and > > >> stops in case wrong datatype put in indexed field» > > >> Can someone, please, help me with the review? > > >> > > >> [1] https://github.com/apache/ignite/pull/8330 > > >> [2] https://issues.apache.org/jira/browse/IGNITE-13553 > > >> > > >> ================== > > >> > > >> I propose to include several minor tickets to the scope of the 2.9 > that > > >> increase Ignite User Experience > > >> > > >> CMD tools improvements: > > >> > > >> IGNITE-13488 - Command to print metric value > > >> IGNITE-13426 - Command to print system view content > > >> IGNITE-13422 - Parameter to explicitly enable experimental commands > > >> > > >> IGNITE-13380 - Output IgniteSystemProperties via ignite.sh > > >> > > >> New system views: > > >> > > >> IGNITE-13409 Metastorage and DistributedMetastorage viewы. > > >> IGNITE-13408 BinaryMetadata view. > > >> > > >>> 9 окт. 2020 г., в 04:04, Denis Magda <dma...@apache.org> написал(а): > > >>> > > >>> @Alex Plehanov <plehanov.a...@gmail.com>, > > >>> > > >>> If it still makes sense and not too late, you can cherry-pick the > > commit > > >>> with the new docs into the 2.9 branch: > > >>> > > >> > > > https://github.com/apache/ignite/commit/073488ac97517bbaad9f6b94b781fc404646f191 > > >>> > > >>> Anyway, it's not crucial as long as we agreed to make an exception > for > > >> this > > >>> release. The docs were already published to the website and assembled > > >> from > > >>> the master. Once the vote passes, I'll make them visible and > traceable > > >> from > > >>> the website menus. > > >>> > > >>> - > > >>> Denis > > >>> > > >>> > > >>> On Tue, Oct 6, 2020 at 7:20 AM Alexey Goncharuk < > > >> alexey.goncha...@gmail.com> > > >>> wrote: > > >>> > > >>>> Saikat, > > >>>> > > >>>> The plan sounds good to me! Do you have an idea for the timeline of > > the > > >>>> module releases? Let me know if I can help in any way. > > >>>> > > >>>> Also, I was looking into the modularization IEP and noticed that > OSGI > > >>>> module is discussed to be moved to the extensions, but there is no > > >>>> corresponding ticket. Should I create one? I will be happy to help > > with > > >>>> moving this module to extensions. > > >>>> > > >>>> вт, 29 сент. 2020 г. в 03:03, Saikat Maitra < > saikat.mai...@gmail.com > > >: > > >>>> > > >>>>> Hi Alexey, > > >>>>> > > >>>>> All the modules as planned in IEP-36 > > >>>>> > > >>>> > > >> > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-36:+Modularization > > >>>>> are migrated to ignite-extensions repository. > > >>>>> > > >>>>> We can definitely plan to release ignite-extensions modules. > > >>>>> > > >>>>> I have a pending PR related to the kafka module and the PR is in > > review > > >>>>> https://github.com/apache/ignite/pull/8222 but its corresponding > PR > > >> has > > >>>>> been merged in ignite-extensions repository. > > >>>>> > > >>>>> My thoughts / action items for the migration efforts and releases > > were > > >>>>> as follows: > > >>>>> > > >>>>> 1. Merge the changes for > https://github.com/apache/ignite/pull/8222 > > >>>>> 2. Fix the upload nightly packages task for ignite-core to help fix > > the > > >>>>> travis build failure in ignite-extensions repository. > > >>>>> 3. Review and update the README.md files for the ignite-extensions > > >>>> modules > > >>>>> as required. > > >>>>> 4. Update the docs for ignite-extensions modules in ignite-website. > > >>>>> 5. Release each module separately and share updates. > > >>>>> > > >>>>> Please let me know if you have feedback. > > >>>>> > > >>>>> Regards, > > >>>>> Saikat > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> On Mon, Sep 28, 2020 at 7:36 AM Petr Ivanov <mr.wei...@gmail.com> > > >> wrote: > > >>>>> > > >>>>>> It seems that gitbox.apache.org is not available on CI/CD. > > >>>>>> > > >>>>>> I will try to update VCS to GitHub. > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>>> On 28 Sep 2020, at 14:07, Alex Plehanov <plehanov.a...@gmail.com > > > > >>>>>> wrote: > > >>>>>>> > > >>>>>>> Guys, > > >>>>>>> > > >>>>>>> I've tried to build release candidate using TC [1]. But: > > >>>>>>> 1. I can't choose a branch in this TC task (there is no such > > field). > > >>>>>>> 2. On the "default" branch I got an error: Failed to collect > > changes, > > >>>>>>> error: List remote refs failed: > > >>>>>>> org.apache.http.conn.ConnectTimeoutException: Connect to > > >>>>>>> gitbox.apache.org:443 [gitbox.apache.org/52.202.80.70] failed: > > >>>> connect > > >>>>>>> timed out, VCS root: "GitBox [asf/ignite]" {instance id=300, > parent > > >>>>>>> internal id=74, parent id=GitBoxAsfIgnite, description: " > > >>>>>>> https://gitbox.apache.org/repos/asf/ignite.git#refs/heads/master > "} > > >>>>>>> > > >>>>>>> Is there some problem with this TC task? What am I doing wrong? > > >>>>>>> > > >>>>>>> [1] : > > >>>>>>> > > >>>>>> > > >>>> > > >> > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=Releases_ApacheIgniteMain_ReleaseBuild > > >>>>>>> > > >>>>>>> пн, 28 сент. 2020 г. в 13:15, Alexey Goncharuk < > > >>>>>> alexey.goncha...@gmail.com>: > > >>>>>>> > > >>>>>>>> Saikat, Nikolay, > > >>>>>>>> > > >>>>>>>> We have migrated a bunch of modules to ignite-extensions > recently. > > >>>>>> Given > > >>>>>>>> that these modules will not be available in Ignite 2.9 anymore > > (will > > >>>>>>>> they?), should we also plan to release the extensions? > > >>>>>>>> > > >>>>>>>> ср, 23 сент. 2020 г. в 21:49, Alex Plehanov < > > >> plehanov.a...@gmail.com > > >>>>> : > > >>>>>>>> > > >>>>>>>>> Igniters, > > >>>>>>>>> > > >>>>>>>>> I've prepared release notes for the 2.9 release [1]. Can anyone > > >>>> review > > >>>>>>>> it, > > >>>>>>>>> please? > > >>>>>>>>> > > >>>>>>>>> [1]: https://github.com/apache/ignite/pull/8273 > > >>>>>>>>> > > >>>>>>>>> вт, 22 сент. 2020 г. в 09:39, Alex Plehanov < > > >>>> plehanov.a...@gmail.com > > >>>>>>> : > > >>>>>>>>> > > >>>>>>>>>> Guys, > > >>>>>>>>>> > > >>>>>>>>>> I've filled the ticket with reproducer [1] for the discovery > > bug. > > >>>>>> This > > >>>>>>>>> bug > > >>>>>>>>>> caused by [2] ticket. We discussed with Vladimir Steshin > > privately > > >>>>>> and > > >>>>>>>>>> decided to revert this ticket. I will do it today (after TC > bot > > >>>> visa) > > >>>>>>>> if > > >>>>>>>>>> there are no objections. > > >>>>>>>>>> > > >>>>>>>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-13465 > > >>>>>>>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-13134 > > >>>>>>>>>> > > >>>>>>>>>> пн, 21 сент. 2020 г. в 11:08, Alex Plehanov < > > >>>> plehanov.a...@gmail.com > > >>>>>>> : > > >>>>>>>>>> > > >>>>>>>>>>> Guys, > > >>>>>>>>>>> > > >>>>>>>>>>> During internal testing, we've found a critical bug with > > >>>>>>>>>>> discovery (cluster falls apart if two nodes segmented > > >>>> sequentially). > > >>>>>>>>> This > > >>>>>>>>>>> problem is not reproduced in 2.8.1. I think we should fix it > > >>>>>>>>>>> before release. Under investigation now. I'll let you know > when > > >> we > > >>>>>> get > > >>>>>>>>>>> something. > > >>>>>>>>>>> > > >>>>>>>>>>> чт, 17 сент. 2020 г. в 00:51, Andrey Gura <ag...@apache.org > >: > > >>>>>>>>>>> > > >>>>>>>>>>>>> So what do you think? Should we proceed with a 'hacked' > > version > > >>>> of > > >>>>>>>>> the > > >>>>>>>>>>>> message factory in 2.9 and go for the runtime message > > generation > > >>>> in > > >>>>>>>>> later > > >>>>>>>>>>>> release, or keep the code clean and fix the regression in > the > > >>>> next > > >>>>>>>>> releases? > > >>>>>>>>>>>>> Andrey, can you take a look at my change? I think it is > > fairly > > >>>>>>>>>>>> straightforward and does not change the semantics, just > skips > > >> the > > >>>>>>>>> factory > > >>>>>>>>>>>> closures for certain messages. > > >>>>>>>>>>>> > > >>>>>>>>>>>> IMHO 2.5% isn't too much especially because it isn't actual > > for > > >>>> all > > >>>>>>>>>>>> workloads (I didn't get any significant drops during > > >>>> benchmarking). > > >>>>>>>> So > > >>>>>>>>>>>> I prefer the runtime generation in later releases. > > >>>>>>>>>>>> > > >>>>>>>>>>>> On Mon, Sep 14, 2020 at 12:41 PM Alexey Goncharuk > > >>>>>>>>>>>> <alexey.goncha...@gmail.com> wrote: > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> Alexey, Andrey, Igniters, > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> So what do you think? Should we proceed with a 'hacked' > > version > > >>>> of > > >>>>>>>>> the > > >>>>>>>>>>>> message factory in 2.9 and go for the runtime message > > generation > > >>>> in > > >>>>>>>>> later > > >>>>>>>>>>>> release, or keep the code clean and fix the regression in > the > > >>>> next > > >>>>>>>>> releases? > > >>>>>>>>>>>>> Andrey, can you take a look at my change? I think it is > > fairly > > >>>>>>>>>>>> straightforward and does not change the semantics, just > skips > > >> the > > >>>>>>>>> factory > > >>>>>>>>>>>> closures for certain messages. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> Personally, I would prefer fixing the regression given that > > we > > >>>>>> also > > >>>>>>>>>>>> introduced tracing in this release. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> пт, 11 сент. 2020 г. в 12:09, Alex Plehanov < > > >>>>>>>> plehanov.a...@gmail.com > > >>>>>>>>>> : > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> Alexey, > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> We've benchmarked by yardstick commits 130376741bf vs > > >>>> ed52559eb95 > > >>>>>>>>> and > > >>>>>>>>>>>> the performance of ed52559eb95 is better for about 2.5% on > > >>>> average > > >>>>>> on > > >>>>>>>>> our > > >>>>>>>>>>>> environment (about the same results we got benchmarking > > 65c30ec6 > > >>>> vs > > >>>>>>>>>>>> 0606f03d). We've made 24 runs for each commit of > > >>>>>>>>>>>> IgnitePutTxImplicitBenchmark (we got maximum drop for 2.9 on > > >> this > > >>>>>>>>>>>> benchmark), 200 seconds warmup, 300 seconds benchmark, 6 > > >>>> servers, 5 > > >>>>>>>>> clients > > >>>>>>>>>>>> 50 threads each. Yardstick results for this configuration: > > >>>>>>>>>>>>>> Commit 130376741bf: avg TPS=164096, avg latency=9173464 ns > > >>>>>>>>>>>>>> Commit ed52559eb95: avg TPS=168283, avg latency=8945908 ns > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> пт, 11 сент. 2020 г. в 09:51, Artem Budnikov < > > >>>>>>>>>>>> a.budnikov.ign...@gmail.com>: > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> Hi Everyone, > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> I posted an instruction on how to publish the docs on > > >>>>>>>>>>>> ignite.apache.org/docs [1]. When you finish with Ignite > 2.9, > > >> you > > >>>>>> can > > >>>>>>>>>>>> update the docs by following the instruction. > Unfortunately, I > > >>>>>> won't > > >>>>>>>> be > > >>>>>>>>>>>> able to spend any time on this project any longer. You can > > send > > >>>>>> your > > >>>>>>>>> pull > > >>>>>>>>>>>> requests and questions about the documentation to Denis > Magda. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> -Artem > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> [1] : > > >>>>>>>>>>>> > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+to+Document > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> On Thu, Sep 10, 2020 at 2:45 PM Alexey Goncharuk < > > >>>>>>>>>>>> alexey.goncha...@gmail.com> wrote: > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> Alexey, > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> I've tried to play with message factories locally, but > > >>>>>>>>>>>> unfortunately, I > > >>>>>>>>>>>>>>>> cannot spot the difference between old and new > > >> implementation > > >>>>>> in > > >>>>>>>>>>>>>>>> distributed benchmarks. I pushed an implementation of > > >>>>>>>>>>>> MessageFactoryImpl > > >>>>>>>>>>>>>>>> with the old switch statement to the > > ignite-2.9-revert-12568 > > >>>>>>>>> branch > > >>>>>>>>>>>>>>>> (discussed this with Andrey Gura, the change should be > > >>>>>>>> compatible > > >>>>>>>>>>>> with the > > >>>>>>>>>>>>>>>> new metrics as we still use the register() mechanics). > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> Can you check if this change makes any difference > > >>>>>>>> performance-wise > > >>>>>>>>>>>> in your > > >>>>>>>>>>>>>>>> environment? If yes, we can go with runtime code > > generation > > >>>> in > > >>>>>>>> the > > >>>>>>>>>>>> long > > >>>>>>>>>>>>>>>> term: register classes and generate a dynamic message > > >> factory > > >>>>>>>> with > > >>>>>>>>>>>> a switch > > >>>>>>>>>>>>>>>> statement once all messages are registered (not in 2.9 > > >>>> though, > > >>>>>>>>>>>> obviously). > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> ср, 9 сент. 2020 г. в 14:53, Alex Plehanov < > > >>>>>>>>> plehanov.a...@gmail.com > > >>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> Hello guys, > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> I've tried to optimize tracing implementation (ticket > > [1]), > > >>>> it > > >>>>>>>>>>>> reduced the > > >>>>>>>>>>>>>>>>> drop, but not completely removed it. > > >>>>>>>>>>>>>>>>> Ivan Rakov, Alexander Lapin, can you please review the > > >>>> patch? > > >>>>>>>>>>>>>>>>> Ivan Artiukhov, can you please benchmark the patch [2] > > >>>> against > > >>>>>>>>>>>> 2.8.1 > > >>>>>>>>>>>>>>>>> release on your environment? > > >>>>>>>>>>>>>>>>> With this patch on our environment, it's about a 3% > drop > > >>>> left, > > >>>>>>>>>>>> it's close > > >>>>>>>>>>>>>>>>> to measurement error and I think such a drop is not a > > >>>>>>>>>>>> showstopper. Guys, > > >>>>>>>>>>>>>>>>> WDYT? > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> Also, I found that compatibility is broken for JDBC > thin > > >>>>>>>> driver > > >>>>>>>>>>>> between 2.8 > > >>>>>>>>>>>>>>>>> and 2.9 versions (ticket [3]). I think it's a blocker > and > > >>>>>>>> should > > >>>>>>>>>>>> be > > >>>>>>>>>>>>>>>>> fixed in 2.9. I've prepared the patch. > > >>>>>>>>>>>>>>>>> Taras Ledkov, can you please review this patch? > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> And one more ticket I propose to include into 2.9 [4] > > (NIO > > >>>>>>>>> message > > >>>>>>>>>>>>>>>>> send problem in some circumstances). I will cherry-pick > > it > > >>>> if > > >>>>>>>>>>>> there is no > > >>>>>>>>>>>>>>>>> objection. > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-13411 > > >>>>>>>>>>>>>>>>> [2] https://github.com/apache/ignite/pull/8223 > > >>>>>>>>>>>>>>>>> [3] https://issues.apache.org/jira/browse/IGNITE-13414 > > >>>>>>>>>>>>>>>>> [4] https://issues.apache.org/jira/browse/IGNITE-13361 > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> пн, 7 сент. 2020 г. в 14:14, Maxim Muzafarov < > > >>>>>>>> mmu...@apache.org > > >>>>>>>>>> : > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> Alexey, > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> I propose to include [1] issue to the 2.9 release. > Since > > >>>>>>>> this > > >>>>>>>>>>>> issue is > > >>>>>>>>>>>>>>>>>> related to the new master key change functionality > which > > >>>>>>>>>>>> haven't been > > >>>>>>>>>>>>>>>>>> released yet I think it will be safe to cherry-pick > > commit > > >>>>>>>> to > > >>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>> release branch. > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> [1] > https://issues.apache.org/jira/browse/IGNITE-13390 > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> On Tue, 1 Sep 2020 at 12:13, Nikolay Izhikov < > > >>>>>>>>>>>> nizhi...@apache.org> > > >>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> Hello, Igniters. > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> Alexey, please, include one more Python thin client > fix > > >>>>>>>> [1] > > >>>>>>>>>>>> into the > > >>>>>>>>>>>>>>>>> 2.9 > > >>>>>>>>>>>>>>>>>> release > > >>>>>>>>>>>>>>>>>>> It fixes kinda major issue - "Python client returns > > >> fields > > >>>>>>>>> in > > >>>>>>>>>>>> wrong > > >>>>>>>>>>>>>>>>>> order since the 2 row when fields_count>10" > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> [1] > https://issues.apache.org/jira/browse/IGNITE-12809 > > >>>>>>>>>>>>>>>>>>> [2] > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>> > > >>>> > > >> > > > https://github.com/apache/ignite/commit/38025ee4167f05eaa2d6a2c5c2ab70c83a462cfc > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> 31 авг. 2020 г., в 19:23, Alexey Goncharuk < > > >>>>>>>>>>>>>>>>> alexey.goncha...@gmail.com> > > >>>>>>>>>>>>>>>>>> написал(а): > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> Alexey, thanks, got it. I am not sure we can > optimize > > >>>>>>>>>>>> anything out of > > >>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>> message factory with suppliers (at least I have no > > ideas > > >>>>>>>>>>>> right now), > > >>>>>>>>>>>>>>>>> so > > >>>>>>>>>>>>>>>>>>>> most likely the only move here is to switch back to > > the > > >>>>>>>>>>>> switch > > >>>>>>>>>>>>>>>>> approach > > >>>>>>>>>>>>>>>>>>>> somehow preserving the metrics part. Probably, > > inlining > > >>>>>>>>> the > > >>>>>>>>>>>> Ignite > > >>>>>>>>>>>>>>>>>> messages > > >>>>>>>>>>>>>>>>>>>> to the IgniteMessageFactoryImpl should do the trick. > > Let > > >>>>>>>>> me > > >>>>>>>>>>>> explore > > >>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>> code a bit. > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> P.S. I am surprised by the impact this part makes > for > > >>>>>>>> the > > >>>>>>>>>>>>>>>>> performance. > > >>>>>>>>>>>>>>>>>>>> Message creation is indeed on the hot path, but a > > single > > >>>>>>>>>>>> virtual call > > >>>>>>>>>>>>>>>>>>>> should not make that much of a difference given the > > >>>>>>>> amount > > >>>>>>>>>>>> of other > > >>>>>>>>>>>>>>>>>> work > > >>>>>>>>>>>>>>>>>>>> happening during the message processing. > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> пн, 31 авг. 2020 г. в 18:33, Alex Plehanov < > > >>>>>>>>>>>> plehanov.a...@gmail.com > > >>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> Alexey, sorry, I wrongly interpreted our benchmark > > >>>>>>>>> results. > > >>>>>>>>>>>>>>>>> Actually, > > >>>>>>>>>>>>>>>>>> we > > >>>>>>>>>>>>>>>>>>>>> were looking for a drop using bi-sect in the range > > >>>>>>>>> between > > >>>>>>>>>>>> e6a7f93 > > >>>>>>>>>>>>>>>>>> (first > > >>>>>>>>>>>>>>>>>>>>> commit in the 2.9 branch after 2.8 branch cut) and > > >>>>>>>>>>>> 6592dfa5 (last > > >>>>>>>>>>>>>>>>>> commit in > > >>>>>>>>>>>>>>>>>>>>> the 2.9 branch). And we found these two problematic > > >>>>>>>>>>>> commits. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> Perhaps only IGNITE-13060 (Tracing) is responsible > > for > > >>>>>>>> a > > >>>>>>>>>>>> drop > > >>>>>>>>>>>>>>>>> between > > >>>>>>>>>>>>>>>>>>>>> 2.8.1 and 2.9 (we have benchmarked 2.8.1 vs 2.9 > with > > >>>>>>>>>>>> reverted > > >>>>>>>>>>>>>>>>>> IGNITE-13060 > > >>>>>>>>>>>>>>>>>>>>> now and performance looks the same) > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> Ticket IGNITE-12568 (MessageFactory refactoring) is > > not > > >>>>>>>>>>>> related to > > >>>>>>>>>>>>>>>>>> drop > > >>>>>>>>>>>>>>>>>>>>> between 2.8.1 and 2.9, but still has some > performance > > >>>>>>>>>>>> problem, and > > >>>>>>>>>>>>>>>>> we > > >>>>>>>>>>>>>>>>>> can > > >>>>>>>>>>>>>>>>>>>>> win back IGNITE-13060 drop by this ticket. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> Do we need more investigation on IGNITE-13060 or we > > >>>>>>>> leave > > >>>>>>>>>>>> it as is? > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> What should we do with IGNITE-12568 (MessageFactory > > >>>>>>>>>>>> refactoring)? > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> пн, 31 авг. 2020 г. в 13:25, Alexey Goncharuk < > > >>>>>>>>>>>>>>>>>> alexey.goncha...@gmail.com > > >>>>>>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> Alexey, > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> While investigating, I found that IGNITE-12568 has > > an > > >>>>>>>>>>>> incorrect fix > > >>>>>>>>>>>>>>>>>>>>>> version and is actually present in ignite-2.8.1 > > branch > > >>>>>>>>>>>> [1], so it > > >>>>>>>>>>>>>>>>>> cannot be > > >>>>>>>>>>>>>>>>>>>>>> the source of the drop against 2.8.1. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> P.S. Looks like we need to enforce a more accurate > > >>>>>>>> work > > >>>>>>>>>>>> with fix > > >>>>>>>>>>>>>>>>>> versions > > >>>>>>>>>>>>>>>>>>>>>> or develop some sort of tooling to verify the fix > > >>>>>>>>>>>> versions. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> --AG > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> [1] > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>> > > >>>> > > >> > > > https://github.com/apache/ignite/commit/3e492bd23851856bbd0385c6a419892d0bba2a34 > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> пн, 31 авг. 2020 г. в 12:42, Alexey Goncharuk < > > >>>>>>>>>>>>>>>>>> alexey.goncha...@gmail.com > > >>>>>>>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> пт, 28 авг. 2020 г. в 11:16, Alex Plehanov < > > >>>>>>>>>>>>>>>>> plehanov.a...@gmail.com > > >>>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> Guys, > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> We have benchmarked 2.9 without IGNITE-13060 and > > >>>>>>>>>>>> IGNITE-12568 > > >>>>>>>>>>>>>>>>>> (reverted > > >>>>>>>>>>>>>>>>>>>>>>>> it > > >>>>>>>>>>>>>>>>>>>>>>>> locally) and got the same performance as on > 2.8.1 > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> IGNITE-13060 (Tracing) - some code was added to > > hot > > >>>>>>>>>>>> paths, to > > >>>>>>>>>>>>>>>>> trace > > >>>>>>>>>>>>>>>>>>>>>>>> these > > >>>>>>>>>>>>>>>>>>>>>>>> hot paths, it's clear why we have performance > drop > > >>>>>>>>> here. > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> IGNITE-12568 (MessageFactory refactoring) - > > >>>>>>>>> switch/case > > >>>>>>>>>>>> block was > > >>>>>>>>>>>>>>>>>>>>>>>> refactored to an array of message suppliers. The > > >>>>>>>>>>>> message factory > > >>>>>>>>>>>>>>>>>> is on > > >>>>>>>>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>>>>> hot path, which explains why this commit has an > > >>>>>>>> impact > > >>>>>>>>>>>> on total > > >>>>>>>>>>>>>>>>>>>>>>>> performance. > > >>>>>>>>>>>>>>>>>>>>>>>> I've checked JIT assembly output, done some JMH > > >>>>>>>>>>>> microbenchmarks, > > >>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>>>>>>> found > > >>>>>>>>>>>>>>>>>>>>>>>> that old implementation of > MessageFactory.create() > > >>>>>>>>>>>> about 30-35% > > >>>>>>>>>>>>>>>>>> faster > > >>>>>>>>>>>>>>>>>>>>>>>> than > > >>>>>>>>>>>>>>>>>>>>>>>> the new one. The reason - approach with > > switch/case > > >>>>>>>>> can > > >>>>>>>>>>>>>>>>> effectively > > >>>>>>>>>>>>>>>>>>>>>>>> inline > > >>>>>>>>>>>>>>>>>>>>>>>> message creation code, but with an array of > > >>>>>>>> suppliers > > >>>>>>>>>>>> relatively > > >>>>>>>>>>>>>>>>>> heavy > > >>>>>>>>>>>>>>>>>>>>>>>> "invokeinterface" cannot be skipped. I've tried > to > > >>>>>>>>>>>> rewrite the > > >>>>>>>>>>>>>>>>> code > > >>>>>>>>>>>>>>>>>>>>>>>> using > > >>>>>>>>>>>>>>>>>>>>>>>> an abstract class for suppliers instead of an > > >>>>>>>>> interface > > >>>>>>>>>>>> (to > > >>>>>>>>>>>>>>>>>>>>>>>> replace "invokeinterface" with the > > "invokevirtual"), > > >>>>>>>>>>>> but it gives > > >>>>>>>>>>>>>>>>>> back > > >>>>>>>>>>>>>>>>>>>>>>>> only > > >>>>>>>>>>>>>>>>>>>>>>>> 10% of method performance and in this case, code > > >>>>>>>> looks > > >>>>>>>>>>>> ugly > > >>>>>>>>>>>>>>>>>> (lambdas > > >>>>>>>>>>>>>>>>>>>>>>>> can't > > >>>>>>>>>>>>>>>>>>>>>>>> be used). Currently, I can't find any more ways > to > > >>>>>>>>>>>> optimize the > > >>>>>>>>>>>>>>>>>> current > > >>>>>>>>>>>>>>>>>>>>>>>> approach (except return to the switch/case > block). > > >>>>>>>>>>>> Andrey Gura, > > >>>>>>>>>>>>>>>>> as > > >>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>>>>> author of IGNITE-12568, maybe you have some > ideas > > >>>>>>>>> about > > >>>>>>>>>>>>>>>>>> optimization? > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> Perhaps we should revert IGNITE-12568, but there > > are > > >>>>>>>>>>>> some metrics > > >>>>>>>>>>>>>>>>>>>>>>>> already > > >>>>>>>>>>>>>>>>>>>>>>>> created, which can't be rewritten using old > > message > > >>>>>>>>>>>> factory > > >>>>>>>>>>>>>>>>>>>>>>>> implementation > > >>>>>>>>>>>>>>>>>>>>>>>> (IGNITE-12756). Guys, WDYT? > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> Alexey, > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> I see that IGNITE-12756 (metrics improvements) is > > >>>>>>>>>>>> already released > > >>>>>>>>>>>>>>>>>> in > > >>>>>>>>>>>>>>>>>>>>>>> Ignite 2.8.1 while IGNITE-12568 (message factory) > > is > > >>>>>>>>>>>> only present > > >>>>>>>>>>>>>>>>>> in Ignite > > >>>>>>>>>>>>>>>>>>>>>>> 2.9. Let's revert both IGNITE-12568 and whichever > > new > > >>>>>>>>>>>> metrics > > >>>>>>>>>>>>>>>>>> created for > > >>>>>>>>>>>>>>>>>>>>>>> 2.9 that depend on the new message factory to > > unblock > > >>>>>>>>>>>> the release > > >>>>>>>>>>>>>>>>>> and deal > > >>>>>>>>>>>>>>>>>>>>>>> with the optimizations in 2.10? > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>> > > >>>>>> > > >>>> > > >> > > >> > > > > >
