Re: PR review help

2023-03-09 Thread Andrey Mashenkov
Hi, Good catch! I've looked at the PR and left a few comments. On Wed, Mar 8, 2023 at 12:08 PM Jian Chen wrote: > Hi Igniters, > > Could someone help to review the PR > https://github.com/apache/ignite/pull/10579 ? > > It's a minor fix for translation about the user get table failed through >

PR review help

2023-03-08 Thread Jian Chen
Hi Igniters, Could someone help to review the PR https://github.com/apache/ignite/pull/10579 ? It's a minor fix for translation about the user get table failed through escaped table names while table name contains consecutive wildcard (underscore). Thanks! Yours Stephen

Re: Re[2]: NUMA aware allocator, PR review request

2021-12-13 Thread Ivan Daschinsky
Thanks all for review, merged into master. пн, 6 дек. 2021 г. в 23:26, Ivan Daschinsky : > You are not wrong, it is built from source, every night. And every TC run. > I don't understand why numa allocator cannot be treated the same. Moreover, > it is built using maven, with maven plugin and just

Re: Re[2]: NUMA aware allocator, PR review request

2021-12-06 Thread Ivan Daschinsky
You are not wrong, it is built from source, every night. And every TC run. I don't understand why numa allocator cannot be treated the same. Moreover, it is built using maven, with maven plugin and just needs gcc and libnuma-dev. All of theese are already on TC agents and build are ready. I didn't

Re: Re[2]: NUMA aware allocator, PR review request

2021-12-06 Thread Ilya Kasnacheev
Hello! Maybe I am wrong, but ODBC installer is built from source and may be improved from release to release. Regards, -- Ilya Kasnacheev пн, 6 дек. 2021 г. в 20:41, Ivan Daschinsky : > Only one reason -- nowadays amost all hardware platforms uses NUMA > > Another reason -- there is no any re

Re: Re[2]: NUMA aware allocator, PR review request

2021-12-06 Thread Ivan Daschinsky
Only one reason -- nowadays amost all hardware platforms uses NUMA Another reason -- there is no any release process of extensions. BTW, apache ignite release is shipped with odbc binary installer for windows. And nobody complains about it. But may be listen to others? пн, 6 дек. 2021 г., 19:4

Re: Re[2]: NUMA aware allocator, PR review request

2021-12-06 Thread Anton Vinogradov
Any reason to release the same cpp sources for each release? Any reason to increase the requirements amount for each new release? Any reason to increase release complexity and duration? All answers are "definitely no" What we should do is to release cpp part once and use it as a dependency. Extens

Re[2]: NUMA aware allocator, PR review request

2021-12-06 Thread Zhenya Stanilovsky
+1 with Ivan, let`s store it in core product just because it looks like  inalienable functionality and release cycle of extensions a little bit different.   >Anton, I disagree. > >1. This should be released with main distro. >2. This should not be abandoned. >3. There is not any release proces

Re: NUMA aware allocator, PR review request

2021-12-06 Thread Ivan Daschinsky
Anton, I disagree. 1. This should be released with main distro. 2. This should not be abandoned. 3. There is not any release process in ignite-extensions. 4. Everything is working now and working good. So lets do not do this :) пн, 6 дек. 2021 г. в 14:49, Anton Vinogradov : > Let's move all GC

Re: NUMA aware allocator, PR review request

2021-12-06 Thread Anton Vinogradov
Let's move all GCC-related parts to ignite-extensions, release, and use them as a maven dependency. On Fri, Dec 3, 2021 at 1:08 PM Ivan Daschinsky wrote: > Ok, TC suite is ready [1]. > If there is no objections, I will merge it soon. > > Possible concerns -- now it is required to install build-

Re: NUMA aware allocator, PR review request

2021-12-03 Thread Ivan Daschinsky
Ok, TC suite is ready [1]. If there is no objections, I will merge it soon. Possible concerns -- now it is required to install build-essentials and libnuma-dev in order to build ignite on 64 bit linux. I suppose that this is not a big deal, but maybe someone will contradict? [1] -- https://ci.ig

Re: NUMA aware allocator, PR review request

2021-12-02 Thread Ivan Daschinsky
>> Our runs show about 7-10 speedup, Sorry, typo 7-10% speedup чт, 2 дек. 2021 г. в 12:01, Ivan Daschinsky : > Andrey, thanks! > > This allocator can be tested on every NUMA system. > Our runs show about 7-10 speedup, if we use allocattor with interleaved > strategy + -XX:+UseNUMA. > But unfortu

Re: NUMA aware allocator, PR review request

2021-12-02 Thread Ivan Daschinsky
Andrey, thanks! This allocator can be tested on every NUMA system. Our runs show about 7-10 speedup, if we use allocattor with interleaved strategy + -XX:+UseNUMA. But unfortunately our yardstick benches doesn't use offheap a lot, usually above one Gb. We trying to do more benches with real data a

Re: NUMA aware allocator, PR review request

2021-12-02 Thread Andrey Mashenkov
Ivan, Great job. PR looks good. This allocator in interleaved mode and passing `-XX:+UseNUMA` flag to jvm > show promising results on yardstick benches. Technically, G1 is not a numa > aware collector for java versions less than 14, but allocation of heap in > interleaved mode shows good results

Re: NUMA aware allocator, PR review request

2021-12-01 Thread Ivan Daschinsky
Semyon D. and Maks T. -- thanks a lot for review. BTW, Igniters, I will appreciate all opinions and feedback. пн, 29 нояб. 2021 г. в 10:13, Ivan Daschinsky : > Hi, igniters! > > There is not a big secret that nowadays NUMA is quite common in > multiprocessor systems. > And this memory architectu

NUMA aware allocator, PR review request

2021-11-28 Thread Ivan Daschinsky
Hi, igniters! There is not a big secret that nowadays NUMA is quite common in multiprocessor systems. And this memory architecture should be treated in specific ways. Support for NUMA is present in many commercial and open-source products. I've implemented a NUMA aware allocator for Apache Ignit

Re: One more round for PR review

2018-05-15 Thread Dmitry Pavlov
Hi Alexey, thank you for review. Hi Igor, could you please merge this PR? вт, 15 мая 2018 г. в 11:18, Alexey Kuznetsov : > Looks good for me now. > > On Tue, May 15, 2018 at 12:38 AM, Dmitry Pavlov > wrote: > > > Hi Alexey K, > > > > could you please check that all proposals were applied? > > >

Re: One more round for PR review

2018-05-15 Thread Alexey Kuznetsov
Looks good for me now. On Tue, May 15, 2018 at 12:38 AM, Dmitry Pavlov wrote: > Hi Alexey K, > > could you please check that all proposals were applied? > > Sincerely, > Dmitriy Pavlov > > чт, 10 мая 2018 г. в 22:59, Igor Rudyak : > > > Hi guys, > > > > I addressed all the comments and need one

Re: One more round for PR review

2018-05-14 Thread Dmitry Pavlov
Hi Alexey K, could you please check that all proposals were applied? Sincerely, Dmitriy Pavlov чт, 10 мая 2018 г. в 22:59, Igor Rudyak : > Hi guys, > > I addressed all the comments and need one more round of review for the PR: > https://github.com/apache/ignite/pull/3940 > > Igor >

One more round for PR review

2018-05-10 Thread Igor Rudyak
Hi guys, I addressed all the comments and need one more round of review for the PR: https://github.com/apache/ignite/pull/3940 Igor

PR review

2016-11-06 Thread Amir Akhmedov
Hi Igniters, I raised PR for IGNITE-4176. Can you please review it and let me know? https://issues.apache.org/jira/browse/IGNITE-4176 -- Sincerely Yours Amir Akhmedov

Re: PR review

2016-02-24 Thread Alexey Goncharuk
Thanks, Semyon! I've addressed the comments.

Re: PR review

2016-02-23 Thread Semyon Boikov
Hi Alexey, I reviewed your changes, added comments in tickets. Semyon On Wed, Feb 24, 2016 at 3:47 AM, Alexey Goncharuk < alexey.goncha...@gmail.com> wrote: > Folks, > > Can I get a review of pull requests 508 [1] and 509 [2] for Jira tickets > 2707 [3] and 2709 [4] respectively? The changes a

PR review

2016-02-23 Thread Alexey Goncharuk
Folks, Can I get a review of pull requests 508 [1] and 509 [2] for Jira tickets 2707 [3] and 2709 [4] respectively? The changes are quite small, TC looks ok. Thanks, AG [1] https://github.com/apache/ignite/pull/508 [2] https://github.com/apache/ignite/pull/509 [3] https://issues.apache.org/jira