> We still can’t accept patches that badly affects the performance of > TcpCommuncationMetricsListener. > So we should perform yardstick tests before the merge.
Absolutely all metrics are on the hot path. They inevitably affect performance and this case is the same. May be we should rollback all metrics related changes because we don't have benchmark results& > I can help to run yardstick benchmarks if you don’t have free servers to do > it. I don't need help in benchmarking. Once again, еhe current behavior is incorrect and should be fixed regardless of performance. Or... this functionality should be removed if performance is more important. In case of incorrect behavior it is the best option. My choice: correctness over performance. On Mon, Jan 27, 2020 at 10:02 PM Nikolay Izhikov <nizhi...@apache.org> wrote: > > > I think it could be fixed easily by adding metricsEnabled flag to > > TcpCommunicationSpi. > > We still can’t accept patches that badly affects the performance of > TcpCommuncationMetricsListener. > So we should perform yardstick tests before the merge. > > I can help to run yardstick benchmarks if you don’t have free servers to do > it. > > > > 27 янв. 2020 г., в 21:47, Andrey Gura <ag...@apache.org> написал(а): > > > >>> "If it doesn’t work, it doesn’t matter how fast it doesn’t work." (c) > >> Please, clarify, what do you mean by «doesn’t work»? > >> Are there any unresolved bugs? > > > > Obviously some communication metrics can't be monitored or analyzed > > retrospectively due to changing node ID during node restart. It's bug. > > > >>> User can disable metrics if it will affect performance. > >> Users can’t disable TcpCommunicationListener nor in any release nor in > >> current master so we should change this code carefully > > > > This is another bug. I think it could be fixed easily by adding > > metricsEnabled flag to TcpCommunicationSpi. > > > > On Mon, Jan 27, 2020 at 9:17 PM Nikolay Izhikov <nizhi...@apache.org> wrote: > >> > >> Andrey. > >> > >>> "If it doesn’t work, it doesn’t matter how fast it doesn’t work." (c) > >> > >> Please, clarify, what do you mean by «doesn’t work»? > >> Are there any unresolved bugs? > >> > >>> IGINTE-12576 affects it minimally > >> > >> All I asking for is to confirm this statement with the benchmark results. > >> > >>> User can disable metrics if it will affect performance. > >> > >> Users can’t disable TcpCommunicationListener nor in any release nor in > >> current master so we should change this code carefully > >> > >> https://github.com/apache/ignite/blob/ignite-2.7.6/modules/core/src/main/java/org/apache/ignite/spi/communication/tcp/TcpCommunicationSpi.java#L1178 > >> > >>> 27 янв. 2020 г., в 20:40, Andrey Gura <ag...@apache.org> написал(а): > >>> > >>> Nikolay, > >>> > >>>> But, we must gather yardstick benchmark results for PR(comparing to > >>>> current master) before merge to ensure there is no performance drop. > >>> > >>> "If it doesn’t work, it doesn’t matter how fast it doesn’t work." (c) > >>> > >>> I believe that benchmarks ignite-2.7.6 vs ignite-2.8 will show > >>> noticeable drop in performance for ignite-2.8. But it is cumulative > >>> effect and IGINTE-12576 affects it minimally. > >>> > >>>> Note, that these metrics updated on each communication message. > >>> > >>> Metrics are not free at all. User can disable metrics if it will > >>> affect performance. > >>> > >>> On Mon, Jan 27, 2020 at 8:23 PM Nikolay Izhikov <nizhi...@apache.org> > >>> wrote: > >>>> > >>>> Hello, Andrey. > >>>> > >>>> I’m OK to include these changes to 2.8. > >>>> I don’t review PR, but the ticket description makes sense to me. > >>>> > >>>> But, we must gather yardstick benchmark results for PR(comparing to > >>>> current master) before merge to ensure there is no performance drop. > >>>> Note, that these metrics updated on each communication message. > >>>> > >>>>> 27 янв. 2020 г., в 18:19, Andrey Gura <ag...@apache.org> написал(а): > >>>>> > >>>>> Igniters, > >>>>> > >>>>> I want to add one more issue to the Apache Ignite 2.8 release scope [1]. > >>>>> > >>>>> The problem is impossibility of using communication metrics gathered > >>>>> for nodes in the cluster because node ID will changed in case of > >>>>> restart. Obvious solution is using consistent ID instead of node ID. > >>>>> > >>>>> PR is already implemented and ready for review. > >>>>> > >>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12576 > >>>>> > >>>>> On Fri, Jan 24, 2020 at 4:06 PM Maxim Muzafarov <mmu...@apache.org> > >>>>> wrote: > >>>>>> > >>>>>> Folks, > >>>>>> > >>>>>> > >>>>>> I've cherry-picked these issues [1] [2] to the 2.8 release branch. > >>>>>> > >>>>>> > >>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12540 > >>>>>> Update versions of vulnerable dependencies > >>>>>> > >>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-12486 > >>>>>> Truncation of archived WAL segments doesn't work > >>>>>> > >>>>>> On Thu, 23 Jan 2020 at 11:08, Ivan Bessonov <bessonov...@gmail.com> > >>>>>> wrote: > >>>>>>> > >>>>>>> Hi igniters, > >>>>>>> > >>>>>>> there's a potential data corruption fix that I'd like you to include > >>>>>>> in the > >>>>>>> next release: > >>>>>>> https://issues.apache.org/jira/browse/IGNITE-12486https://issues.apache.org/jira/browse/IGNITE-12486 > >>>>>>> <https://issues.apache.org/jira/browse/IGNITE-12486> > >>>>>>> > >>>>>>> Can you please cherry-pick it? Thank you! > >>>>>>> > >>>>>>> ср, 22 янв. 2020 г. в 17:45, Pavel Tupitsyn <ptupit...@apache.org>: > >>>>>>> > >>>>>>>> Good idea about pre-release build of ignite-2.8 branch. > >>>>>>>> However, I would not name it `rc`, since it is not really a release > >>>>>>>> candidate. Make it `pre0` or something like that. > >>>>>>>> > >>>>>>>> For Ignite.NET I've uploaded pre-release NuGet packages built from > >>>>>>>> current > >>>>>>>> ignite-2.8 branch: > >>>>>>>> https://www.nuget.org/packages/Apache.Ignite/2.8.0-alpha20200122 > >>>>>>>> > >>>>>>>> > >>>>>>>> On Wed, Jan 22, 2020 at 3:09 PM Ilya Kasnacheev > >>>>>>>> <ilya.kasnach...@gmail.com > >>>>>>>>> > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Hello! > >>>>>>>>> > >>>>>>>>> I have committed the bumping of essential dependencies' versions: > >>>>>>>>> https://issues.apache.org/jira/browse/IGNITE-12540 > >>>>>>>>> > >>>>>>>>> Would you mind including this change into the scope of 2.8? No > >>>>>>>>> point of > >>>>>>>>> shipping known problematic JARs in our deliverable. > >>>>>>>>> > >>>>>>>>> Regards, > >>>>>>>>> -- > >>>>>>>>> Ilya Kasnacheev > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> ср, 22 янв. 2020 г. в 14:00, Maxim Muzafarov <mmu...@apache.org>: > >>>>>>>>> > >>>>>>>>>> Alexey, > >>>>>>>>>> > >>>>>>>>>> Sure, I've just thought about it too a few days ago. > >>>>>>>>>> > >>>>>>>>>> On Wed, 22 Jan 2020 at 12:09, Anton Vinogradov <a...@apache.org> > >>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>> Good Idea, this will also check that the release process is alive. > >>>>>>>>>>> > >>>>>>>>>>> On Wed, Jan 22, 2020 at 12:04 PM Alexey Goncharuk < > >>>>>>>>>>> alexey.goncha...@gmail.com> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Folks, Maxim, > >>>>>>>>>>>> > >>>>>>>>>>>> Do you mind if I build the current state of ignite-2.8 branch and > >>>>>>>>>> upload a > >>>>>>>>>>>> maven staging as rc0 (step 4.3.2 of the release process)? I want > >>>>>>>> run > >>>>>>>>>> some > >>>>>>>>>>>> tests for the fixes that are already included to the branch. > >>>>>>>>>>>> > >>>>>>>>>>>> вт, 21 янв. 2020 г. в 14:28, Maxim Muzafarov <mmu...@apache.org>: > >>>>>>>>>>>> > >>>>>>>>>>>>> Folks, > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> I think both of these issues [1] [2] are critical to 2.8 release > >>>>>>>>> and > >>>>>>>>>>>>> we must include them. > >>>>>>>>>>>>> > >>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12547 > >>>>>>>>>>>>> Excessive AtomicLong instantiations lead to GC pressure. > >>>>>>>>>>>>> > >>>>>>>>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-12530 > >>>>>>>>>>>>> Pages list caching can cause IgniteOOME when the checkpoint is > >>>>>>>>>>>>> triggered by "too many dirty pages" reason. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Mon, 20 Jan 2020 at 19:00, Alex Plehanov < > >>>>>>>>> plehanov.a...@gmail.com > >>>>>>>>>>> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Guys, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> There is an issue [1] caused by page list caching [2], which > >>>>>>>> also > >>>>>>>>>>>> affects > >>>>>>>>>>>>>> 2.8 release. IgniteOutOfMemoryException can be thrown in some > >>>>>>>>> cases > >>>>>>>>>>>> (data > >>>>>>>>>>>>>> region is small, a checkpoint is triggered by "too many dirty > >>>>>>>>>> pages" > >>>>>>>>>>>>> reason > >>>>>>>>>>>>>> and pages list cache is rather big). > >>>>>>>>>>>>>> The fix is ready and merged to master, I suggest to include > >>>>>>>> this > >>>>>>>>>> fix to > >>>>>>>>>>>>> 2.8 > >>>>>>>>>>>>>> release. What do you think? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-12530 > >>>>>>>>>>>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-6930 > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> пн, 20 янв. 2020 г. в 12:57, Alexey Goncharuk < > >>>>>>>>>>>>> alexey.goncha...@gmail.com>: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Maxim, > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I took a quick look at IGNITE-12456 and I am not sure it's > >>>>>>>>> about > >>>>>>>>>> data > >>>>>>>>>>>>>>> corruption. In the attached logs blocked system threads are > >>>>>>>>>> reported, > >>>>>>>>>>>>>>> however, there is no enough information to investigate the > >>>>>>>>> issue > >>>>>>>>>> (the > >>>>>>>>>>>>> full > >>>>>>>>>>>>>>> thread dump was not attached). I asked the ticket creator to > >>>>>>>>>> attach > >>>>>>>>>>>>> missing > >>>>>>>>>>>>>>> pieces. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Should we consider moving this ticket to a next release? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> пн, 20 янв. 2020 г. в 08:54, Zhenya Stanilovsky > >>>>>>>>>>>>> <arzamas...@mail.ru.invalid > >>>>>>>>>>>>>>>> : > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Maxim, performance fix issue [1] already in master, if no > >>>>>>>>>>>>> objections, can > >>>>>>>>>>>>>>>> u merge it into 2.8 ? Thanks ! > >>>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12547 > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Igniters, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Here is the actual list of BLOCKER release issues: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> IGNITE-12456 Cluster Data Store grid gets Corrupted for > >>>>>>>> Load > >>>>>>>>>> test > >>>>>>>>>>>>>>>>> *[Unassigned]* OPEN > >>>>>>>>>>>>>>>>> IGNITE-12489 Error during purges by expiration: Unknown > >>>>>>>> page > >>>>>>>>>> type* > >>>>>>>>>>>>>>>>> [Unassigned]* OPEN > >>>>>>>>>>>>>>>>> IGNITE-8641 SpringDataExample should use > >>>>>>>> example-ignite.xml > >>>>>>>>>> config > >>>>>>>>>>>>>>>>> *[Unassigned]* OPEN > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> IGNITE-12398 Apache Ignite Cluster(Amazon S3 Based > >>>>>>>>> Discovery) > >>>>>>>>>>>> Nodes > >>>>>>>>>>>>>>>> getting > >>>>>>>>>>>>>>>>> down [Emmanouil Gkatziouras] OPEN > >>>>>>>>>>>>>>>>> IGNITE-9184 Cluster hangs during concurrent node client > >>>>>>>> and > >>>>>>>>>> server > >>>>>>>>>>>>> nodes > >>>>>>>>>>>>>>>>> restart [Dmitriy Sorokin] IN PROGRESS > >>>>>>>>>>>>>>>>> IGNITE-12553 [IEP-35] public Java metric API Improvement > >>>>>>>>>> [Nikolay > >>>>>>>>>>>>>>> Izhikov] > >>>>>>>>>>>>>>>>> Blocker IN PROGRESS > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> IGNITE-12227 Default auto-adjust baseline enabled flag > >>>>>>>>>> calculated > >>>>>>>>>>>>>>>>> incorrectly [Anton Kalashnikov] PATCH AVAILABLE > >>>>>>>>>>>>>>>>> IGNITE-12470 Pme-free switch feature should be > >>>>>>>> deactivatable > >>>>>>>>>>>> [Sergei > >>>>>>>>>>>>>>>>> Ryzhov] PATCH AVAILABLE > >>>>>>>>>>>>>>>>> IGNITE-12552 [IEP-35] Expose MetricRegistry to the public > >>>>>>>>> API > >>>>>>>>>>>>>>> Improvement > >>>>>>>>>>>>>>>>> [Nikolay Izhikov] PATCH AVAILABLE > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12456 > >>>>>>>>>>>>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-12489 > >>>>>>>>>>>>>>>>> [3] https://issues.apache.org/jira/browse/IGNITE-8641 > >>>>>>>>>>>>>>>>> [8] https://issues.apache.org/jira/browse/IGNITE-12398 > >>>>>>>>>>>>>>>>> [3] https://issues.apache.org/jira/browse/IGNITE-9184 > >>>>>>>>>>>>>>>>> [6] https://issues.apache.org/jira/browse/IGNITE-12553 > >>>>>>>>>>>>>>>>> [7] https://issues.apache.org/jira/browse/IGNITE-12227 > >>>>>>>>>>>>>>>>> [9] https://issues.apache.org/jira/browse/IGNITE-12470 > >>>>>>>>>>>>>>>>> [5] https://issues.apache.org/jira/browse/IGNITE-12552 > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On Sat, 18 Jan 2020 at 19:11, Sergey Antonov < > >>>>>>>>>>>>>>> antonovserge...@gmail.com > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Maxim, > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Conflicts in pr [1] are resolved. TC Run all is started. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> [1] https://github.com/apache/ignite/pull/7238 > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> пт, 17 янв. 2020 г. в 16:04, Sergey Antonov < > >>>>>>>>>>>>>>> antonovserge...@gmail.com > >>>>>>>>>>>>>>>>> : > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Maxim, > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> I will do that on monday (20/01). > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> пт, 17 янв. 2020 г. в 13:08, Maxim Muzafarov < > >>>>>>>>>>>> mmu...@apache.org > >>>>>>>>>>>>>> : > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Sergey, > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Can you, please, resolve the PR conflicts [1] [2]? > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> [1] https://github.com/apache/ignite/pull/7238 > >>>>>>>>>>>>>>>>>>>> [2] > >>>>>>>> https://issues.apache.org/jira/browse/IGNITE-11256 > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> On Thu, 16 Jan 2020 at 16:59, Ilya Kasnacheev < > >>>>>>>>>>>>>>>> ilya.kasnach...@gmail.com > > >>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Hello! > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> I have bumped beanutils and re-ran Cassandra Store > >>>>>>>>>> tests. > >>>>>>>>>>>> Can > >>>>>>>>>>>>> you > >>>>>>>>>>>>>>>>>>>> please > >>>>>>>>>>>>>>>>>>>>> comment on the ticket? > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> I think that fixing ZooKeeper is too much effort > >>>>>>>>>> (there's > >>>>>>>>>>>>> chaos > >>>>>>>>>>>>>>> with > >>>>>>>>>>>>>>>>>>>>> jackson vs. jackson-asl), maybe it should be split > >>>>>>>> up > >>>>>>>>>> as a > >>>>>>>>>>>>>>> separate > >>>>>>>>>>>>>>>>>>>> ticket > >>>>>>>>>>>>>>>>>>>>> to be done later. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Regards, > >>>>>>>>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>>>>>>>> Ilya Kasnacheev > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> ср, 15 янв. 2020 г. в 18:31, Vladimir Pligin < > >>>>>>>>>>>>>>> vova199...@yandex.ru > >>>>>>>>>>>>>>>>> : > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Thanks, Ilya. It would be really great to have > >>>>>>>> your > >>>>>>>>>> patch > >>>>>>>>>>>>>>> included > >>>>>>>>>>>>>>>>>>>> into 2.8 > >>>>>>>>>>>>>>>>>>>>>> scope. > >>>>>>>>>>>>>>>>>>>>>> I'd like to give my two cent as well. For example > >>>>>>>> we > >>>>>>>>>> have > >>>>>>>>>>>>>>>> vulnerable > >>>>>>>>>>>>>>>>>>>>>> dependencies here: > >>>>>>>>>>>>>>>>>>>>>> modules/cassandra/store/pom.xml - > >>>>>>>> commons-beanutils > >>>>>>>>>>>>>>>>>>>>>> modules/zookeeper/pom.xml - transitive Jackson > >>>>>>>> from > >>>>>>>>>>>> Curator > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> I'd suggest to uprgrade > >>>>>>>>>>>> commons-beanutils:commons-beanutils > >>>>>>>>>>>>> to > >>>>>>>>>>>>>>>> 1.9.4 > >>>>>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>>>> override > >>>>>>>> com.fasterxml.jackson.core:jackson-databind > >>>>>>>>>> to > >>>>>>>>>>>> our > >>>>>>>>>>>>>>> common > >>>>>>>>>>>>>>>>>>>> jackson > >>>>>>>>>>>>>>>>>>>>>> version from other modules. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>>>>>>>>> Sent from: > >>>>>>>>>>>>>>>> http://apache-ignite-developers.2346864.n4.nabble.com/ > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>>>>>> BR, Sergey Antonov > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>>>>> BR, Sergey Antonov > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> Sincerely yours, > >>>>>>> Ivan Bessonov > >>>> > >> >
