Re: [DISCUSS] Missed (non-suited) tests
Yes, they can be performed as parallel, as doesn't depend on each other. There is an overhead for running git + mvn test twice, but it is a cost for the flexibility. On Wed, Mar 3, 2021 at 8:55 PM Max Timonin wrote: > I mean that any TC job with tests depends on both [Build], [Sanity > Checks]. No tests run if any of those jobs failed. > > [Build] prepares ignite.zip for distribution between TC agents (mvn > install). > [Sanity Checks] checks that code is correct in terms of our static checks > (mvn test). > > Indeed it can be run as a single job, but in favor of flexibility in > configuration (enable / disable checks) it is OK to separate it in 2 steps. > > Do you have some objections to do it that way? > > On Wed, Mar 3, 2021 at 8:45 PM Maxim Muzafarov wrote: > >> Maxim, >> >> Can you clarify what means '[Sanity Checks] runs in parallel with >> [Build]'? AFAIK the checks need the build results to run themselves. >> >> On Wed, 3 Mar 2021 at 18:48, Max Timonin wrote: >> > >> > Discussed with Petr privately. Proposal is: >> > >> > 1. The [Build] job runs without any checks. >> > 2. There will be a new job [Sanity Checks], that runs all checks - >> > checkstyle, licenses, javadoc, check-suites. >> > 3. [Sanity Checks] runs in parallel with [Build]. >> > 4. All TC jobs with tests depend on a result of the [Sanity Checks] >> job. If >> > the check job fails then a test job won't be started. >> > 5. Users can disable the [Sanity Checks] job with a selector on the >> > Parameters tab of custom TC build. >> > >> > If no one has objections I will create a JIRA ticket for that. >> > >> > >> > On Wed, Mar 3, 2021 at 5:11 PM Max Timonin >> wrote: >> > >> > > Hi Petr! My proposal is: >> > > >> > > 1. Create a parameter in [Build] TC suite - MAVEN_CHECKS, default >> value is >> > > "-Plicenses,checkstyle,check-licenses,check-test-suites". >> > > 2. Use it in a command along with MAVEN_MODULES_STRING. >> > > -U -Pall-java,all-scala,scala,lgpl,examples %MAVEN_CHECKS% >> > > %MAVEN_MODULES_STRING% >> > > >> > > 3. Provide a global param for test suites "reverse.dep.MAVEN_CHECKS" >> that >> > > is possible to override in a custom build. If I understand it >> correctly is >> > > possible to do by editing the job [1]. >> > > 4. This param should be represented to a user as a selector with 2 >> > > options: >> > > - default (see point 1.) >> > > - "-DskipTests=true" - that ignores all checks, skip tests and just >> build >> > > a .zip of Ignite. >> > > >> > > Could you please review this solution? Is it OK for you? >> > > >> > > [1] >> > > >> https://ci.ignite.apache.org/admin/editBuildParams.html?id=template:IgniteTests24Java8_RunTestSuitesJava >> > > >> > > On Thu, Feb 25, 2021 at 1:47 PM Petr Ivanov >> wrote: >> > > >> > >> If profile can handle this — its ok. >> > >> >> > >> For choosing build type — we can introduce select, that will choose >> > >> between -p and -DskipTests=true (defaulting to profile). >> > >> Thus [Build] will pass either way. >> > >> >> > >> >> > >> Regards, >> > >> Petr Ivanov >> > >> Head of IT >> > >> IT & Development Solutions | GRIDGAIN SYSTEMS >> > >> +7 (911) 945-00-59 >> > >> >> > >> > On 25 Feb 2021, at 13:23, Max Timonin >> wrote: >> > >> > >> > >> > Yes, it's correct that "mvn install" runs also the "mvn test" >> command, >> > >> and >> > >> > this is OK as the check-test-suites profile handles all tests >> > >> > without running them. If the skipTests flag is triggered then this >> > >> check is >> > >> > useless. It will take only about 2 min to run "mvn test" with this >> > >> profile. >> > >> > Travis does that as one of steps. >> > >> > >> > >> > So, there are no issues with tests. Should I provide more info how >> this >> > >> > check works? >> > >> > >> > >> > Also, discussed with Anton Vinogradov, Alex Plekhanov. There can >> be an >> > >> > issu
Re: [DISCUSS] Missed (non-suited) tests
I mean that any TC job with tests depends on both [Build], [Sanity Checks]. No tests run if any of those jobs failed. [Build] prepares ignite.zip for distribution between TC agents (mvn install). [Sanity Checks] checks that code is correct in terms of our static checks (mvn test). Indeed it can be run as a single job, but in favor of flexibility in configuration (enable / disable checks) it is OK to separate it in 2 steps. Do you have some objections to do it that way? On Wed, Mar 3, 2021 at 8:45 PM Maxim Muzafarov wrote: > Maxim, > > Can you clarify what means '[Sanity Checks] runs in parallel with > [Build]'? AFAIK the checks need the build results to run themselves. > > On Wed, 3 Mar 2021 at 18:48, Max Timonin wrote: > > > > Discussed with Petr privately. Proposal is: > > > > 1. The [Build] job runs without any checks. > > 2. There will be a new job [Sanity Checks], that runs all checks - > > checkstyle, licenses, javadoc, check-suites. > > 3. [Sanity Checks] runs in parallel with [Build]. > > 4. All TC jobs with tests depend on a result of the [Sanity Checks] job. > If > > the check job fails then a test job won't be started. > > 5. Users can disable the [Sanity Checks] job with a selector on the > > Parameters tab of custom TC build. > > > > If no one has objections I will create a JIRA ticket for that. > > > > > > On Wed, Mar 3, 2021 at 5:11 PM Max Timonin > wrote: > > > > > Hi Petr! My proposal is: > > > > > > 1. Create a parameter in [Build] TC suite - MAVEN_CHECKS, default > value is > > > "-Plicenses,checkstyle,check-licenses,check-test-suites". > > > 2. Use it in a command along with MAVEN_MODULES_STRING. > > > -U -Pall-java,all-scala,scala,lgpl,examples %MAVEN_CHECKS% > > > %MAVEN_MODULES_STRING% > > > > > > 3. Provide a global param for test suites "reverse.dep.MAVEN_CHECKS" > that > > > is possible to override in a custom build. If I understand it > correctly is > > > possible to do by editing the job [1]. > > > 4. This param should be represented to a user as a selector with 2 > > > options: > > > - default (see point 1.) > > > - "-DskipTests=true" - that ignores all checks, skip tests and just > build > > > a .zip of Ignite. > > > > > > Could you please review this solution? Is it OK for you? > > > > > > [1] > > > > https://ci.ignite.apache.org/admin/editBuildParams.html?id=template:IgniteTests24Java8_RunTestSuitesJava > > > > > > On Thu, Feb 25, 2021 at 1:47 PM Petr Ivanov > wrote: > > > > > >> If profile can handle this — its ok. > > >> > > >> For choosing build type — we can introduce select, that will choose > > >> between -p and -DskipTests=true (defaulting to profile). > > >> Thus [Build] will pass either way. > > >> > > >> > > >> Regards, > > >> Petr Ivanov > > >> Head of IT > > >> IT & Development Solutions | GRIDGAIN SYSTEMS > > >> +7 (911) 945-00-59 > > >> > > >> > On 25 Feb 2021, at 13:23, Max Timonin > wrote: > > >> > > > >> > Yes, it's correct that "mvn install" runs also the "mvn test" > command, > > >> and > > >> > this is OK as the check-test-suites profile handles all tests > > >> > without running them. If the skipTests flag is triggered then this > > >> check is > > >> > useless. It will take only about 2 min to run "mvn test" with this > > >> profile. > > >> > Travis does that as one of steps. > > >> > > > >> > So, there are no issues with tests. Should I provide more info how > this > > >> > check works? > > >> > > > >> > Also, discussed with Anton Vinogradov, Alex Plekhanov. There can be > an > > >> > issue, that sometimes it's required to run custom test suites to > debug > > >> > flaky tests. Sequence of steps is the following: > > >> > 1. Find a test suite with flaky tests (that reproducible only on an > > >> > TeamCity agent); > > >> > 2. Comment some tests in the suite to isolate; > > >> > 3. Push it, and run related TC suite; > > >> > 4. TC suite depends on [Build] job, run the job - it will fail on > the > > >> check > > >> > "check-test-suites". > > >> > > > >> > So it is needed
Re: [DISCUSS] Missed (non-suited) tests
Discussed with Petr privately. Proposal is: 1. The [Build] job runs without any checks. 2. There will be a new job [Sanity Checks], that runs all checks - checkstyle, licenses, javadoc, check-suites. 3. [Sanity Checks] runs in parallel with [Build]. 4. All TC jobs with tests depend on a result of the [Sanity Checks] job. If the check job fails then a test job won't be started. 5. Users can disable the [Sanity Checks] job with a selector on the Parameters tab of custom TC build. If no one has objections I will create a JIRA ticket for that. On Wed, Mar 3, 2021 at 5:11 PM Max Timonin wrote: > Hi Petr! My proposal is: > > 1. Create a parameter in [Build] TC suite - MAVEN_CHECKS, default value is > "-Plicenses,checkstyle,check-licenses,check-test-suites". > 2. Use it in a command along with MAVEN_MODULES_STRING. > -U -Pall-java,all-scala,scala,lgpl,examples %MAVEN_CHECKS% > %MAVEN_MODULES_STRING% > > 3. Provide a global param for test suites "reverse.dep.MAVEN_CHECKS" that > is possible to override in a custom build. If I understand it correctly is > possible to do by editing the job [1]. > 4. This param should be represented to a user as a selector with 2 > options: > - default (see point 1.) > - "-DskipTests=true" - that ignores all checks, skip tests and just build > a .zip of Ignite. > > Could you please review this solution? Is it OK for you? > > [1] > https://ci.ignite.apache.org/admin/editBuildParams.html?id=template:IgniteTests24Java8_RunTestSuitesJava > > On Thu, Feb 25, 2021 at 1:47 PM Petr Ivanov wrote: > >> If profile can handle this — its ok. >> >> For choosing build type — we can introduce select, that will choose >> between -p and -DskipTests=true (defaulting to profile). >> Thus [Build] will pass either way. >> >> >> Regards, >> Petr Ivanov >> Head of IT >> IT & Development Solutions | GRIDGAIN SYSTEMS >> +7 (911) 945-00-59 >> >> > On 25 Feb 2021, at 13:23, Max Timonin wrote: >> > >> > Yes, it's correct that "mvn install" runs also the "mvn test" command, >> and >> > this is OK as the check-test-suites profile handles all tests >> > without running them. If the skipTests flag is triggered then this >> check is >> > useless. It will take only about 2 min to run "mvn test" with this >> profile. >> > Travis does that as one of steps. >> > >> > So, there are no issues with tests. Should I provide more info how this >> > check works? >> > >> > Also, discussed with Anton Vinogradov, Alex Plekhanov. There can be an >> > issue, that sometimes it's required to run custom test suites to debug >> > flaky tests. Sequence of steps is the following: >> > 1. Find a test suite with flaky tests (that reproducible only on an >> > TeamCity agent); >> > 2. Comment some tests in the suite to isolate; >> > 3. Push it, and run related TC suite; >> > 4. TC suite depends on [Build] job, run the job - it will fail on the >> check >> > "check-test-suites". >> > >> > So it is needed to provide a configuration to disable this check such >> runs. >> > I'll have a look on next week how to implement this. >> > >> > On Thu, Feb 25, 2021 at 11:02 AM Petr Ivanov >> wrote: >> > >> >> I am telling that INSTALL goal for maven will trigger TEST goal for the >> >> whole project and it cannot be prevented until the flag is specified >> either >> >> as command line parameter, or in profile somehow to be inherited by >> other >> >> modules. >> >> Thats why I am suggesting this as separate suite. >> >> >> >> >> >> Regards, >> >> *Petr Ivanov* >> >> Head of IT >> >> IT & Development Solutions | >> >> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 >> >> >> >> On 25 Feb 2021, at 10:44, Max Timonin wrote: >> >> >> >> Hi, Petr! >> >> >> >> Profile "check-test-suites" handles all tests in another way, it just >> >> verifies that all tests are suited. No tests run then. >> >> As I understand the [BUILD] job goal is preparing a .zip archive to >> >> distribute it for other jobs. I think it is a valid place to put sanity >> >> checks. If a check fails then no archive is prepared. WDYT? >> >> >> >> Also I see that there is a flag -Dmaven.javadoc.skip=true. I'd propose >> to >> >> change it to the profile "skip-docs", that wa
Re: [DISCUSS] Missed (non-suited) tests
Hi Petr! My proposal is: 1. Create a parameter in [Build] TC suite - MAVEN_CHECKS, default value is "-Plicenses,checkstyle,check-licenses,check-test-suites". 2. Use it in a command along with MAVEN_MODULES_STRING. -U -Pall-java,all-scala,scala,lgpl,examples %MAVEN_CHECKS% %MAVEN_MODULES_STRING% 3. Provide a global param for test suites "reverse.dep.MAVEN_CHECKS" that is possible to override in a custom build. If I understand it correctly is possible to do by editing the job [1]. 4. This param should be represented to a user as a selector with 2 options: - default (see point 1.) - "-DskipTests=true" - that ignores all checks, skip tests and just build a .zip of Ignite. Could you please review this solution? Is it OK for you? [1] https://ci.ignite.apache.org/admin/editBuildParams.html?id=template:IgniteTests24Java8_RunTestSuitesJava On Thu, Feb 25, 2021 at 1:47 PM Petr Ivanov wrote: > If profile can handle this — its ok. > > For choosing build type — we can introduce select, that will choose > between -p and -DskipTests=true (defaulting to profile). > Thus [Build] will pass either way. > > > Regards, > Petr Ivanov > Head of IT > IT & Development Solutions | GRIDGAIN SYSTEMS > +7 (911) 945-00-59 > > > On 25 Feb 2021, at 13:23, Max Timonin wrote: > > > > Yes, it's correct that "mvn install" runs also the "mvn test" command, > and > > this is OK as the check-test-suites profile handles all tests > > without running them. If the skipTests flag is triggered then this check > is > > useless. It will take only about 2 min to run "mvn test" with this > profile. > > Travis does that as one of steps. > > > > So, there are no issues with tests. Should I provide more info how this > > check works? > > > > Also, discussed with Anton Vinogradov, Alex Plekhanov. There can be an > > issue, that sometimes it's required to run custom test suites to debug > > flaky tests. Sequence of steps is the following: > > 1. Find a test suite with flaky tests (that reproducible only on an > > TeamCity agent); > > 2. Comment some tests in the suite to isolate; > > 3. Push it, and run related TC suite; > > 4. TC suite depends on [Build] job, run the job - it will fail on the > check > > "check-test-suites". > > > > So it is needed to provide a configuration to disable this check such > runs. > > I'll have a look on next week how to implement this. > > > > On Thu, Feb 25, 2021 at 11:02 AM Petr Ivanov > wrote: > > > >> I am telling that INSTALL goal for maven will trigger TEST goal for the > >> whole project and it cannot be prevented until the flag is specified > either > >> as command line parameter, or in profile somehow to be inherited by > other > >> modules. > >> Thats why I am suggesting this as separate suite. > >> > >> > >> Regards, > >> *Petr Ivanov* > >> Head of IT > >> IT & Development Solutions | > >> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 > >> > >> On 25 Feb 2021, at 10:44, Max Timonin wrote: > >> > >> Hi, Petr! > >> > >> Profile "check-test-suites" handles all tests in another way, it just > >> verifies that all tests are suited. No tests run then. > >> As I understand the [BUILD] job goal is preparing a .zip archive to > >> distribute it for other jobs. I think it is a valid place to put sanity > >> checks. If a check fails then no archive is prepared. WDYT? > >> > >> Also I see that there is a flag -Dmaven.javadoc.skip=true. I'd propose > to > >> change it to the profile "skip-docs", that was introduced in ticket [1] > >> IGNITE-13623. As the setting "maven.javadoc.skip" does not > >> affect scaladocs. > >> > >> [1] https://issues.apache.org/jira/browse/IGNITE-13623 > >> > >> On Thu, Feb 25, 2021 at 7:34 AM Petr Ivanov > wrote: > >> > >>> Won't the absence of -DskipTests flag trigger ALL the tests for all > >>> modules? > >>> This flag was added intentionally. > >>> > >>> Instead, I'd put Non-Suited tests into some kind of sanity check, group > >>> all sanity checks in single Run All, and make tests depend on it's > >>> successful pass. > >>> > >>> > >>> Regards, > >>> *Petr Ivanov* > >>> Head of IT > >>> IT & Development Solutions | > >>> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 > >>> > >>> On 24 Feb 2
Re: [DISCCUSS] Ignite 2.9.2. Index compatibility issue 2.x -> 2.9
Ok, I see, thanks for the answer! On Fri, Feb 26, 2021 at 2:13 PM Ilya Kasnacheev wrote: > Hello! > > I don't think we should fix it in 2.9.2 if the fix will already be > available in 2.10. > > Anyone who sees this bug is welcome to upgrade to 2.10. > > Regards, > -- > Ilya Kasnacheev > > > пт, 26 февр. 2021 г. в 11:32, Max Timonin : > > > Hi, Igniters! > > > > There was a compatibility issue [1] with upgrading an old version of > Ignite > > to 2.9. Inlined JavaObjects just are skipped and then it leads to > undefined > > behavior: > > 1. In best case it just fails with BPlusTreeCorruptedException; > > 2. But in some cases Ignite may *silently* make a wrong decision while > > comparing, then results of queries will be invalid. > > > > Workaround is to rebuild indexes. > > > > For the new release 2.10 this bug was fixed today (commits are [2], [3]). > > > > WDYT, is it worth to backport those commits to the previous Ignite > version? > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-14206 > > [2] > > > > > https://github.com/apache/ignite/commit/851f650ba03e0b6c081cfe23f411fd2bf6be0228 > > [3] > > > > > https://github.com/apache/ignite/commit/93b74922bd04b164301d7bc5a2788b9693d4a8a4 > > >
[DISCCUSS] Ignite 2.9.2. Index compatibility issue 2.x -> 2.9
Hi, Igniters! There was a compatibility issue [1] with upgrading an old version of Ignite to 2.9. Inlined JavaObjects just are skipped and then it leads to undefined behavior: 1. In best case it just fails with BPlusTreeCorruptedException; 2. But in some cases Ignite may *silently* make a wrong decision while comparing, then results of queries will be invalid. Workaround is to rebuild indexes. For the new release 2.10 this bug was fixed today (commits are [2], [3]). WDYT, is it worth to backport those commits to the previous Ignite version? [1] https://issues.apache.org/jira/browse/IGNITE-14206 [2] https://github.com/apache/ignite/commit/851f650ba03e0b6c081cfe23f411fd2bf6be0228 [3] https://github.com/apache/ignite/commit/93b74922bd04b164301d7bc5a2788b9693d4a8a4
Re: Re[2]: [DISCUSSION] Apache Ignite Release 2.10 (time, scope, manager)
Hi, Maxim! The bug IGNITE-14206 [1] fixed. There are 2 commits to cherry-pick 1. https://github.com/apache/ignite/commit/851f650ba03e0b6c081cfe23f411fd2bf6be0228 2. https://github.com/apache/ignite/commit/93b74922bd04b164301d7bc5a2788b9693d4a8a4 https://issues.apache.org/jira/browse/IGNITE-14206 On Wed, Feb 24, 2021 at 5:47 PM Maxim Muzafarov wrote: > Anton, > > Your improvement is very important, for sure, but it's almost 2 months > have passed since the initiation of the release branch. I think we > should prepare the changes as fast as possible for now and initiate > with the next one release. In general, it would be nice to have a > release plan for the year ahead so each developer will understand on > which release his changes can get into. > > That's why I'm going to prepare an RC-build on Monday 1-st March. > > On Wed, 24 Feb 2021 at 11:13, Anton Vinogradov wrote: > > > > Maxim, > > > > https://issues.apache.org/jira/browse/IGNITE-13873 > > Is ready for review, is it possible to wait for it? > > > > On Wed, Feb 24, 2021 at 12:01 AM Maxim Muzafarov > wrote: > > > > > Folks, > > > > > > > > > I'd like to cherry-pick to the release branch the following fixes: > > > > > > Fix security context for JDBC bulk-load operations > > > https://issues.apache.org/jira/browse/IGNITE-13472 > > > > > > Fixed an issue that caused a deadlock when user cache was created in > > > parallel with TTL worker was in progress. > > > https://issues.apache.org/jira/browse/IGNITE-14078 > > > > > > On Thu, 18 Feb 2021 at 20:26, Maxim Muzafarov > wrote: > > > > > > > > Maxim, Igor, > > > > > > > > Thanks for sharing details. > > > > Let's wait for these fixes. > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-14206 > > > > [2] https://issues.apache.org/jira/browse/IGNITE-14204 > > > > > > > > On Thu, 18 Feb 2021 at 18:35, Igor Sapego > wrote: > > > > > > > > > > Maxim, > > > > > > > > > > I believe I could fix the ticket [1] by the end of the next week. > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-14204 > > > > > > > > > > Best Regards, > > > > > Igor > > > > > > > > > > > > > > > On Thu, Feb 18, 2021 at 6:30 PM Max Timonin < > timonin.ma...@gmail.com> > > > wrote: > > > > > > > > > > > Hi! I've today found an issue [1], there is wrong handling of > > > inlined POJO. > > > > > > This bug appeared in 2.9.0 and makes it impossible to work with > > > > > > multi-column indexes created on old AI versions that contain > inlined > > > POJO > > > > > > keys in the middle. > > > > > > > > > > > > I'm working on the fix and asked Konstantin Orlov to review it. I > > > think > > > > > > that it will take only a couple of days to fix this issue. From > my > > > side, it > > > > > > looks like a release blocker, but we've been living with that > since > > > 2.9.0. > > > > > > WDYT if the release can wait for the fix? > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-14206 > > > > > > > > > > > > On Thu, Feb 18, 2021 at 5:38 PM Maxim Muzafarov < > mmu...@apache.org> > > > wrote: > > > > > > > > > > > > > Folks, > > > > > > > > > > > > > > > > > > > > > Do we have any estimations of how long does it take to fix the > > > issue > > > > > > > [1] in C++ thin client? The bug must be fixed for sure, > however, I > > > > > > > tend to continue with the release (if fixing the bug require a > few > > > > > > > weeks) and prepare a batch of fixes for the 2.10.1. > > > > > > > > > > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-14204 > > > > > > > > > > > > > > On Thu, 18 Feb 2021 at 13:22, Ilya Kasnacheev < > > > ilya.kasnach...@gmail.com > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > Hello! > > > > > > &g
Re: [DISCUSS] Missed (non-suited) tests
Yes, it's correct that "mvn install" runs also the "mvn test" command, and this is OK as the check-test-suites profile handles all tests without running them. If the skipTests flag is triggered then this check is useless. It will take only about 2 min to run "mvn test" with this profile. Travis does that as one of steps. So, there are no issues with tests. Should I provide more info how this check works? Also, discussed with Anton Vinogradov, Alex Plekhanov. There can be an issue, that sometimes it's required to run custom test suites to debug flaky tests. Sequence of steps is the following: 1. Find a test suite with flaky tests (that reproducible only on an TeamCity agent); 2. Comment some tests in the suite to isolate; 3. Push it, and run related TC suite; 4. TC suite depends on [Build] job, run the job - it will fail on the check "check-test-suites". So it is needed to provide a configuration to disable this check such runs. I'll have a look on next week how to implement this. On Thu, Feb 25, 2021 at 11:02 AM Petr Ivanov wrote: > I am telling that INSTALL goal for maven will trigger TEST goal for the > whole project and it cannot be prevented until the flag is specified either > as command line parameter, or in profile somehow to be inherited by other > modules. > Thats why I am suggesting this as separate suite. > > > Regards, > *Petr Ivanov* > Head of IT > IT & Development Solutions | > *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 > > On 25 Feb 2021, at 10:44, Max Timonin wrote: > > Hi, Petr! > > Profile "check-test-suites" handles all tests in another way, it just > verifies that all tests are suited. No tests run then. > As I understand the [BUILD] job goal is preparing a .zip archive to > distribute it for other jobs. I think it is a valid place to put sanity > checks. If a check fails then no archive is prepared. WDYT? > > Also I see that there is a flag -Dmaven.javadoc.skip=true. I'd propose to > change it to the profile "skip-docs", that was introduced in ticket [1] > IGNITE-13623. As the setting "maven.javadoc.skip" does not > affect scaladocs. > > [1] https://issues.apache.org/jira/browse/IGNITE-13623 > > On Thu, Feb 25, 2021 at 7:34 AM Petr Ivanov wrote: > >> Won't the absence of -DskipTests flag trigger ALL the tests for all >> modules? >> This flag was added intentionally. >> >> Instead, I'd put Non-Suited tests into some kind of sanity check, group >> all sanity checks in single Run All, and make tests depend on it's >> successful pass. >> >> >> Regards, >> *Petr Ivanov* >> Head of IT >> IT & Development Solutions | >> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 >> >> On 24 Feb 2021, at 19:58, Max Timonin wrote: >> >> Hi, all! >> >> What do you think if we add the check in the TC [Build] job. Currently >> [Build] runs also check for licences, checkstyle [1]: >> >> mvn *install* -Pall-java,all-scala,scala,*licenses*,lgpl,examples, >> *checkstyle* -DskipTests -Dmaven.javadoc.skip=true >> %MAVEN_MODULES_STRING%. >> >> So let's add the check too to block other jobs. As if there missed tests >> then TC run may be invalid - missed tests may be broken and then the MTCGA >> visa too. To made this we should change command line parameters: >> 1. Add profile check-test-suites; >> 2. Remove -Dskiptests flag. >> >> -Pall-java,all-scala,scala,licenses,lgpl,examples,checkstyle, >> *check-test-suites *-DskipTests -Dmaven.javadoc.skip=true >> %MAVEN_MODULES_STRING% >> >> WDYT? >> >> [1] >> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_BuildApacheIgnite=buildTypeSettings_IgniteTests24Java8=%3Cdefault >> >> On Tue, Feb 9, 2021 at 4:48 PM Ilya Kasnacheev >> wrote: >> >>> Hello again! >>> >>> Of course it's 20 minutes, not 20 seconds. >>> >>> Regards, >>> -- >>> Ilya Kasnacheev >>> >>> >>> вт, 9 февр. 2021 г. в 16:45, Ilya Kasnacheev >> >: >>> >>> > Hello! >>> > >>> > Java part kicks in if the target not found in pom.xml. Ideally we >>> should >>> > skip this build if target check-test-suites is not in pom.xml >>> > >>> > I have changed its timeout to 20 second which should terminate its >>> > progression on older builds. Maybe that would be sufficient for now. >>> > >>> > Regards, >>> > -- >>> > Ilya Kasnacheev >>> > >>> > >>> > вт, 9 февр. 2021 г. в
Re: [DISCUSS] Missed (non-suited) tests
Hi, Petr! Profile "check-test-suites" handles all tests in another way, it just verifies that all tests are suited. No tests run then. As I understand the [BUILD] job goal is preparing a .zip archive to distribute it for other jobs. I think it is a valid place to put sanity checks. If a check fails then no archive is prepared. WDYT? Also I see that there is a flag -Dmaven.javadoc.skip=true. I'd propose to change it to the profile "skip-docs", that was introduced in ticket [1] IGNITE-13623. As the setting "maven.javadoc.skip" does not affect scaladocs. [1] https://issues.apache.org/jira/browse/IGNITE-13623 On Thu, Feb 25, 2021 at 7:34 AM Petr Ivanov wrote: > Won't the absence of -DskipTests flag trigger ALL the tests for all > modules? > This flag was added intentionally. > > Instead, I'd put Non-Suited tests into some kind of sanity check, group > all sanity checks in single Run All, and make tests depend on it's > successful pass. > > > Regards, > *Petr Ivanov* > Head of IT > IT & Development Solutions | > *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 > > On 24 Feb 2021, at 19:58, Max Timonin wrote: > > Hi, all! > > What do you think if we add the check in the TC [Build] job. Currently > [Build] runs also check for licences, checkstyle [1]: > > mvn *install* -Pall-java,all-scala,scala,*licenses*,lgpl,examples, > *checkstyle* -DskipTests -Dmaven.javadoc.skip=true %MAVEN_MODULES_STRING%. > > So let's add the check too to block other jobs. As if there missed tests > then TC run may be invalid - missed tests may be broken and then the MTCGA > visa too. To made this we should change command line parameters: > 1. Add profile check-test-suites; > 2. Remove -Dskiptests flag. > > -Pall-java,all-scala,scala,licenses,lgpl,examples,checkstyle, > *check-test-suites *-DskipTests -Dmaven.javadoc.skip=true > %MAVEN_MODULES_STRING% > > WDYT? > > [1] > https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_BuildApacheIgnite=buildTypeSettings_IgniteTests24Java8=%3Cdefault > > On Tue, Feb 9, 2021 at 4:48 PM Ilya Kasnacheev > wrote: > >> Hello again! >> >> Of course it's 20 minutes, not 20 seconds. >> >> Regards, >> -- >> Ilya Kasnacheev >> >> >> вт, 9 февр. 2021 г. в 16:45, Ilya Kasnacheev : >> >> > Hello! >> > >> > Java part kicks in if the target not found in pom.xml. Ideally we should >> > skip this build if target check-test-suites is not in pom.xml >> > >> > I have changed its timeout to 20 second which should terminate its >> > progression on older builds. Maybe that would be sufficient for now. >> > >> > Regards, >> > -- >> > Ilya Kasnacheev >> > >> > >> > вт, 9 февр. 2021 г. в 14:09, Petr Ivanov : >> > >> >> As much as I understood — we execute internal class as plugin, where >> all >> >> the magic is done. >> >> Seems pretty solid in Maven part. Java part, unfortunately, cannot >> check. >> >> >> >> >> >> >> >> Regards, >> >> *Petr Ivanov* >> >> Head of IT >> >> IT & Development Solutions | >> >> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 >> >> >> >> On 9 Feb 2021, at 12:32, Ilya Kasnacheev >> >> wrote: >> >> >> >> Hello Peter, >> >> >> >> Thanks for chiming in. The code is under >> >> https://github.com/apache/ignite/pull/8367 >> >> >> >> Regards, >> >> -- >> >> Ilya Kasnacheev >> >> >> >> >> >> вт, 9 февр. 2021 г. в 12:20, Petr Ivanov : >> >> >> >>> Hi, Ilya. >> >>> >> >>> >> >>> I've added Inspections to Run All. >> >>> Checkstyle is currently integrated to Build and can be deleted. >> >>> >> >>> >> >>> Where can I find the code for check-test-suites profile? >> >>> >> >>> >> >>> Regards, >> >>> *Petr Ivanov* >> >>> Head of IT >> >>> IT & Development Solutions | >> >>> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 >> >>> >> >>> On 9 Feb 2021, at 12:16, Ilya Kasnacheev >> >>> wrote: >> >>> >> >>> Hello! >> >>> >> >>> I have found one issue where it would cause tests to be run if the >> >>> change is not present in the target branch. >> >>> >> >>>
Re: [DISCUSS] Missed (non-suited) tests
Hi, all! What do you think if we add the check in the TC [Build] job. Currently [Build] runs also check for licences, checkstyle [1]: mvn *install* -Pall-java,all-scala,scala,*licenses*,lgpl,examples, *checkstyle* -DskipTests -Dmaven.javadoc.skip=true %MAVEN_MODULES_STRING%. So let's add the check too to block other jobs. As if there missed tests then TC run may be invalid - missed tests may be broken and then the MTCGA visa too. To made this we should change command line parameters: 1. Add profile check-test-suites; 2. Remove -Dskiptests flag. -Pall-java,all-scala,scala,licenses,lgpl,examples,checkstyle, *check-test-suites *-DskipTests -Dmaven.javadoc.skip=true %MAVEN_MODULES_STRING% WDYT? [1] https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_BuildApacheIgnite=buildTypeSettings_IgniteTests24Java8=%3Cdefault On Tue, Feb 9, 2021 at 4:48 PM Ilya Kasnacheev wrote: > Hello again! > > Of course it's 20 minutes, not 20 seconds. > > Regards, > -- > Ilya Kasnacheev > > > вт, 9 февр. 2021 г. в 16:45, Ilya Kasnacheev : > > > Hello! > > > > Java part kicks in if the target not found in pom.xml. Ideally we should > > skip this build if target check-test-suites is not in pom.xml > > > > I have changed its timeout to 20 second which should terminate its > > progression on older builds. Maybe that would be sufficient for now. > > > > Regards, > > -- > > Ilya Kasnacheev > > > > > > вт, 9 февр. 2021 г. в 14:09, Petr Ivanov : > > > >> As much as I understood — we execute internal class as plugin, where all > >> the magic is done. > >> Seems pretty solid in Maven part. Java part, unfortunately, cannot > check. > >> > >> > >> > >> Regards, > >> *Petr Ivanov* > >> Head of IT > >> IT & Development Solutions | > >> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 > >> > >> On 9 Feb 2021, at 12:32, Ilya Kasnacheev > >> wrote: > >> > >> Hello Peter, > >> > >> Thanks for chiming in. The code is under > >> https://github.com/apache/ignite/pull/8367 > >> > >> Regards, > >> -- > >> Ilya Kasnacheev > >> > >> > >> вт, 9 февр. 2021 г. в 12:20, Petr Ivanov : > >> > >>> Hi, Ilya. > >>> > >>> > >>> I've added Inspections to Run All. > >>> Checkstyle is currently integrated to Build and can be deleted. > >>> > >>> > >>> Where can I find the code for check-test-suites profile? > >>> > >>> > >>> Regards, > >>> *Petr Ivanov* > >>> Head of IT > >>> IT & Development Solutions | > >>> *GRIDGAIN SYSTEMS*+7 (911) 945-00-59 > >>> > >>> On 9 Feb 2021, at 12:16, Ilya Kasnacheev > >>> wrote: > >>> > >>> Hello! > >>> > >>> I have found one issue where it would cause tests to be run if the > >>> change is not present in the target branch. > >>> > >>> This includes e.g. 2.10 nightlies. > >>> > >>> What can we do to avoid that? Is specifying -DskipTests sufficient? Any > >>> chance that it will break the missed tests check? > >>> > >>> Regards, > >>> -- > >>> Ilya Kasnacheev > >>> > >>> > >>> пн, 8 февр. 2021 г. в 14:13, Ilya Kasnacheev < > ilya.kasnach...@gmail.com > >>> >: > >>> > >>>> Hello! > >>>> > >>>> I have created a TC suite: > >>>> > >>>> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_MissingTests?mode=builds > >>>> > >>>> + Peter Ivanov > >>>> > >>>> Can you please check if everything is alright? > >>>> > >>>> BTW, it seems that Inspections [Core] is only in Run All Basic (but > not > >>>> in Run All), and Check Code Style is not triggered by either one. Is > it > >>>> correct? > >>>> > >>>> Regards, > >>>> -- > >>>> Ilya Kasnacheev > >>>> > >>>> > >>>> пн, 8 февр. 2021 г. в 10:22, Max Timonin : > >>>> > >>>>> Hi! > >>>>> > >>>>> Yes, now it's a part of the Travis check, and there is already a > first > >>>>> successful build [1]. But I think it's also required to run
Re: TeamCity agent aitc-lin07_04 fails to run Build
Looks like somebody fixed that. An hour ago Build succeeded at last [1]. It was the first successful job for a long time. Thanks! removed directory '/opt/buildagent/.m2/repository/org/apache/ignite' Process exited with code 0 [1] https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite/5890154 On Wed, Feb 24, 2021 at 10:10 AM Max Timonin wrote: > Hi! > > My "Build" runs on TC fail with on 1 (Step: Cleanup local maven > repository (Command Line)) > > Error is related to AI 3.0.0: > > + rm -rfv /opt/buildagent/.m2/repository/org/apache/ignite > > rm: cannot remove > '/opt/buildagent/.m2/repository/org/apache/ignite/ignite-configuration/ > *3.0.0-SNAPSHOT*/_remote.repositories': *Permission denied* > > As I can see this error is only on aitc-lin07_04 agent. The first time it > appeared 16th of February [1] > > [1] > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite/5876992 > > How can we fix this? >
TeamCity agent aitc-lin07_04 fails to run Build
Hi! My "Build" runs on TC fail with on 1 (Step: Cleanup local maven repository (Command Line)) Error is related to AI 3.0.0: + rm -rfv /opt/buildagent/.m2/repository/org/apache/ignite rm: cannot remove '/opt/buildagent/.m2/repository/org/apache/ignite/ignite-configuration/ *3.0.0-SNAPSHOT*/_remote.repositories': *Permission denied* As I can see this error is only on aitc-lin07_04 agent. The first time it appeared 16th of February [1] [1] https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite/5876992 How can we fix this?
Re: Re[2]: [DISCUSSION] Apache Ignite Release 2.10 (time, scope, manager)
Hi! I've today found an issue [1], there is wrong handling of inlined POJO. This bug appeared in 2.9.0 and makes it impossible to work with multi-column indexes created on old AI versions that contain inlined POJO keys in the middle. I'm working on the fix and asked Konstantin Orlov to review it. I think that it will take only a couple of days to fix this issue. From my side, it looks like a release blocker, but we've been living with that since 2.9.0. WDYT if the release can wait for the fix? [1] https://issues.apache.org/jira/browse/IGNITE-14206 On Thu, Feb 18, 2021 at 5:38 PM Maxim Muzafarov wrote: > Folks, > > > Do we have any estimations of how long does it take to fix the issue > [1] in C++ thin client? The bug must be fixed for sure, however, I > tend to continue with the release (if fixing the bug require a few > weeks) and prepare a batch of fixes for the 2.10.1. > > > [1] https://issues.apache.org/jira/browse/IGNITE-14204 > > On Thu, 18 Feb 2021 at 13:22, Ilya Kasnacheev > wrote: > > > > Hello! > > > > There was a ticket filed about the new feature, transactions in C++ thin > > client, making this feature unusable if there's more than one connection > > endpoint: > > https://issues.apache.org/jira/browse/IGNITE-14204 > > > > I don't think this is a genuine blocker for 2.10, since there is a > > workaround (use just one endpoint), nevertheless it is a critical crasher > > bug, so I wonder if Igor could take a look at it promptly, include the > fix > > in 2.10. > > > > Regards, > > -- > > Ilya Kasnacheev > > > > > > чт, 18 февр. 2021 г. в 09:03, Zhenya Stanilovsky > > >: > > > > > > > > > > > I fill the ticket with drop problem, plz take a look [1] > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-14205 > > > > > > >Ilya, > > > > > > > >Thanks! > > > >I've added this step to the Release Process wiki page also [1]. > > > > > > > >[1] > > > > https://cwiki.apache.org/confluence/display/IGNITE/Release+Process#ReleaseProcess-P1.2ImplementationandScopeFinalization > > > > > > > >On Wed, 17 Feb 2021 at 18:05, Ilya Kasnacheev < > ilya.kasnach...@gmail.com > > > > wrote: > > > >> > > > >> Hello! > > > >> > > > >> I have added ignite-2.10 to Nightly build triggering. I hope we will > > > have a > > > >> 2.10 nightly tomorrow. Shame that I didn't do it earlier. > > > >> > > > > https://ci.ignite.apache.org/buildConfiguration/Releases_NightlyRelease_RunApacheIgniteNightlyRelease?branch=ignite-2.10=overview=builds > > > >> > > > >> Regards, > > > >> -- > > > >> Ilya Kasnacheev > > > >> > > > >> > > > >> ср, 17 февр. 2021 г. в 17:37, Maxim Muzafarov < mmu...@apache.org > >: > > > >> > > > >> > Folks, > > > >> > > > > >> > There is a possible issue with the performance for 2.9.1 vs 2.10. > I'm > > > >> > trying to reproduce on a stress-testing environment. > > > >> > [1] > > > >> > > > > > https://cwiki.apache.org/confluence/download/attachments/165224767/atomicPutRandomValueFullSync.jpg?api=v2 > > > >> > > > > >> > On Fri, 12 Feb 2021 at 20:46, Maxim Muzafarov < mmu...@apache.org > > > > > wrote: > > > >> > > > > > >> > > Folks, > > > >> > > > > > >> > > I'm going to cherry-pick these issues to the release branch, any > > > >> > objections? > > > >> > > > > > >> > > > > > >> > > Checkpointer thread holds write lock too long > > > >> > > https://issues.apache.org/jira/browse/IGNITE-14140 > > > >> > > > > > >> > > Incorrect initialize checkpoint-runner-cpu thread pool > > > >> > > https://issues.apache.org/jira/browse/IGNITE-14139 > > > >> > > > > > >> > > On Wed, 10 Feb 2021 at 21:59, Maxim Muzafarov < > mmu...@apache.org > > > > wrote: > > > >> > > > > > > >> > > > Folks, > > > >> > > > > > > >> > > > Do we need any other critical issues from the master branch > that > > > need > > > >> > > > to be cherry-picked picked from the master branch? I've > marked the > > > >> > > > latest select issues with patching version 2.10. > > > >> > > > > > > >> > > > - benchmarks completed (I'll do another one prior to > preparing rc) > > > >> > > > - the release notes merged > > > >> > > > - cherry-picked issue (IGNITE-14073 Fixed transactions > failover) > > > >> > > > - most of the documentation pages also merged > > > >> > > > > > > >> > > > Hopefully, by Friday the 12th everything will be ready for the > > > >> > > > preparation of a release candidate. > > > >> > > > > > > >> > > > On Tue, 9 Feb 2021 at 05:09, Никита Сафонов < > > > vlasovpavel2...@gmail.com > > > > >> > wrote: > > > >> > > > > > > > >> > > > > Hi everyone, > > > >> > > > > > > > >> > > > > Below are two lists of items representing all the remaining > (and > > > >> > completed) > > > >> > > > > documentation tasks for the Ignite 2.10 release. > > > >> > > > > > > > >> > > > > The "*Improvements*" part includes PRs on reworked > > > documentation. > > > >> > > > > The "*Finished*" part includes PRs on newly added > documentation. > > > >> > > > > > > > >> > > > > *Improvements:* > > > >> > > > > > > > >> > > > > Documentation for
Re: [DISCUSS] Missed (non-suited) tests
Hi! Yes, now it's a part of the Travis check, and there is already a first successful build [1]. But I think it's also required to run the check on TC too, along with jobs for checking licenses, code style, and core inspections. [1] https://travis-ci.com/github/apache/ignite/builds/216363067 On Fri, Feb 5, 2021 at 7:13 PM Ilya Kasnacheev wrote: > Hello! > > I have merged it to master! > > I wonder what happens next. It will run as a part of travis check? Do we > also need to add it as a TC suite? > > Regards, > -- > Ilya Kasnacheev > > > ср, 3 февр. 2021 г. в 18:50, Ilya Kasnacheev : > > > Hello! > > > > Code mostly looks good, I have added a minor request. I will check how it > > works and then we may commit. > > > > + zaleslaw@gmail.com > > > > Can you please check whether the new ML suites make sense? > > math/distances/DistancesTestSuite.java > > naivebayes/NaiveBayesTestSuite.java > > > > Would we need to add them to some TC runs? > > > > Regards, > > -- > > Ilya Kasnacheev > > > > > > пн, 25 янв. 2021 г. в 22:07, Max Timonin : > > > >> Hi, Ilya! > >> > >> I made a fix to the check. Now it aggregates info about tests and suites > >> from all modules and then validates it. Could you please review the PR > >> [1]? > >> > >> I tried to move some tests between modules, but unfortunately it still > >> looks like spaghetti. So I reverted all changes to testsuites (new and > >> splitted suites) and reworked the check. > >> > >> [1] https://github.com/apache/ignite/pull/8367 > >> > >> On Mon, Dec 28, 2020 at 2:03 PM Ilya Kasnacheev < > >> ilya.kasnach...@gmail.com> > >> wrote: > >> > >> > Hello! > >> > > >> > You could try to move these tests as .java files between modules in a > >> > separate commit. I think I could review it. > >> > > >> > Regards, > >> > -- > >> > Ilya Kasnacheev > >> > > >> > > >> > пт, 25 дек. 2020 г. в 17:19, Max Timonin : > >> > > >> > > Hi! > >> > > > >> > > Ilya thanks for the reply! I agree that it's a valid case when a > test > >> is > >> > > part of multiple suites in different modules. But it is definitely a > >> bug > >> > > that the test is written in a module where it can't be run at all > and > >> > aimed > >> > > to run within different modules (core tests in core that require > h2). > >> I > >> > > propose to fix this issue. > >> > > > >> > > I'm going to check all such tests and move them to the right module. > >> As I > >> > > can see there are about 100 such test classes, but I hope that most > of > >> > them > >> > > follow only a few patterns. > >> > > > >> > > On Fri, Dec 25, 2020 at 2:58 PM Ivan Daschinsky < > ivanda...@gmail.com> > >> > > wrote: > >> > > > >> > > > Hi! > >> > > > >> I'm not sure that we should assume every test is only run from > >> one > >> > > test > >> > > > suite. One test may be run from different test suites located in > >> > > different > >> > > > modules. > >> > > > Agree. For example, if we introduce this limitation, zk suites > will > >> be > >> > > > broken. > >> > > > > >> > > > > >> > > > пт, 25 дек. 2020 г. в 14:48, Ilya Kasnacheev < > >> > ilya.kasnach...@gmail.com > >> > > >: > >> > > > > >> > > > > Hello! > >> > > > > > >> > > > > Sorry for the long wait. > >> > > > > > >> > > > > I'm not sure that we should assume every test is only run from > one > >> > test > >> > > > > suite. One test may be run from different test suites located in > >> > > > different > >> > > > > modules. > >> > > > > > >> > > > > I wonder if we can drop this requirement, check all the modules > >> > > > > transitively for used/unused tests. > >> > > > > > >> > > > > Regards, > >> > > > > -- > >> > > > > Ilya K
Re:
Closed. I've recreated a topic, as forget about the topic title there. On Tue, Feb 2, 2021 at 6:17 PM Max Timonin wrote: > Hi, Igniters! > > Last week I investigated a bug [1]. It's about an incorrect result for > non-colocated joins. For such joins it's required to set up the > "distributedJoin" flag, or try to make joined tables colocated. It is > covered in docs [2]. But it's not obvious and some users don't read that or > forget about that. In result there are wrong results for some queries that > are pretty hard to debug. > > There is a ticket [3] with a comment, where it's suggested to add a check > for such joins. I tried to implement it and found a place where it's > possible to put this check. But there is an open question what this check > should do. Currently I see 2 ways for that: > 1. Forbid non-colocated joins that aren't marked with the distributedJoin > flag, and throw an exception. > 2. Check every query for such joins and implicitly setup a distributedJoin > flag for them. > > Both solutions may break compatibility, but is this compatibility OK? > > Igniters, what do you think? > > [1] https://issues.apache.org/jira/browse/IGNITE-12847 > [2] > https://ignite.apache.org/docs/latest/SQL/distributed-joins#distributed-joins > [3] https://issues.apache.org/jira/browse/IGNITE-13019 >
[DISCUSSION] Fail on non-colocated join
Hi, Igniters! Last week I investigated a bug [1]. It's about an incorrect result for non-colocated joins. For such joins it's required to set up the "distributedJoin" flag, or try to make joined tables colocated. It is covered in docs [2]. But it's not obvious and some users don't read that or forget about that. In result there are wrong results for some queries that are pretty hard to debug. There is a ticket [3] with a comment, where it's suggested to add a check for such joins. I tried to implement it and found a place where it's possible to put this check. But there is an open question what this check should do. Currently I see 2 ways for that: 1. Forbid non-colocated joins that aren't marked with the distributedJoin flag, and throw an exception. 2. Check every query for such joins and implicitly setup a distributedJoin flag for them. Both solutions may break compatibility, but is this compatibility OK? Igniters, what do you think? [1] https://issues.apache.org/jira/browse/IGNITE-12847 [2] https://ignite.apache.org/docs/latest/SQL/distributed-joins#distributed-joins [3] https://issues.apache.org/jira/browse/IGNITE-13019
Re: [DISCUSS] Missed (non-suited) tests
Hi, Ilya! I made a fix to the check. Now it aggregates info about tests and suites from all modules and then validates it. Could you please review the PR [1]? I tried to move some tests between modules, but unfortunately it still looks like spaghetti. So I reverted all changes to testsuites (new and splitted suites) and reworked the check. [1] https://github.com/apache/ignite/pull/8367 On Mon, Dec 28, 2020 at 2:03 PM Ilya Kasnacheev wrote: > Hello! > > You could try to move these tests as .java files between modules in a > separate commit. I think I could review it. > > Regards, > -- > Ilya Kasnacheev > > > пт, 25 дек. 2020 г. в 17:19, Max Timonin : > > > Hi! > > > > Ilya thanks for the reply! I agree that it's a valid case when a test is > > part of multiple suites in different modules. But it is definitely a bug > > that the test is written in a module where it can't be run at all and > aimed > > to run within different modules (core tests in core that require h2). I > > propose to fix this issue. > > > > I'm going to check all such tests and move them to the right module. As I > > can see there are about 100 such test classes, but I hope that most of > them > > follow only a few patterns. > > > > On Fri, Dec 25, 2020 at 2:58 PM Ivan Daschinsky > > wrote: > > > > > Hi! > > > >> I'm not sure that we should assume every test is only run from one > > test > > > suite. One test may be run from different test suites located in > > different > > > modules. > > > Agree. For example, if we introduce this limitation, zk suites will be > > > broken. > > > > > > > > > пт, 25 дек. 2020 г. в 14:48, Ilya Kasnacheev < > ilya.kasnach...@gmail.com > > >: > > > > > > > Hello! > > > > > > > > Sorry for the long wait. > > > > > > > > I'm not sure that we should assume every test is only run from one > test > > > > suite. One test may be run from different test suites located in > > > different > > > > modules. > > > > > > > > I wonder if we can drop this requirement, check all the modules > > > > transitively for used/unused tests. > > > > > > > > Regards, > > > > -- > > > > Ilya Kasnacheev > > > > > > > > > > > > ср, 2 дек. 2020 г. в 18:23, Max Timonin : > > > > > > > > > Hi, > > > > > > > > > > I don't think so. It looks like a bug that tests fail if one runs > > them > > > > > within their module (actually, what is the goal of this test?). The > > > check > > > > > showed us this problem, there is no need to fix the check. > > > > > > > > > > Currently I see two ways: > > > > > 1. Find the right module for every misplaced test. There are 104 > > tests, > > > > > maybe just move them all to the target module? If TeamCity runs > them > > > > within > > > > > the indexing module only is there a reason to have a test in the > core > > > > > module at all? > > > > > 2. Back to my previous proposal - create fake suites within a > module, > > > > then > > > > > replace test classes in a target suite with the single class of the > > > fake > > > > > suite. > > > > > > > > > > > > > > > > > > > > On Wed, Dec 2, 2020 at 5:38 PM Ilya Kasnacheev < > > > > ilya.kasnach...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hello! > > > > > > > > > > > > I think this means that we should abandon the plan of moving > tests > > > > > between > > > > > > suites, and that your tool has to understand the dependency graph > > > > > > between modules' tests when assessing what's included and what's > > not. > > > > > > > > > > > > Regards, > > > > > > -- > > > > > > Ilya Kasnacheev > > > > > > > > > > > > > > > > > > ср, 2 дек. 2020 г. в 15:56, Max Timonin >: > > > > > > > > > > > > > Hi, Ilya! > > > > > > > > > > > > > > I've checked testsuites. There is an issue. For example > > > > > > > *IgniteBinaryCacheQuery
Re: [DISCUSS] Missed (non-suited) tests
Hi! Ilya thanks for the reply! I agree that it's a valid case when a test is part of multiple suites in different modules. But it is definitely a bug that the test is written in a module where it can't be run at all and aimed to run within different modules (core tests in core that require h2). I propose to fix this issue. I'm going to check all such tests and move them to the right module. As I can see there are about 100 such test classes, but I hope that most of them follow only a few patterns. On Fri, Dec 25, 2020 at 2:58 PM Ivan Daschinsky wrote: > Hi! > >> I'm not sure that we should assume every test is only run from one test > suite. One test may be run from different test suites located in different > modules. > Agree. For example, if we introduce this limitation, zk suites will be > broken. > > > пт, 25 дек. 2020 г. в 14:48, Ilya Kasnacheev : > > > Hello! > > > > Sorry for the long wait. > > > > I'm not sure that we should assume every test is only run from one test > > suite. One test may be run from different test suites located in > different > > modules. > > > > I wonder if we can drop this requirement, check all the modules > > transitively for used/unused tests. > > > > Regards, > > -- > > Ilya Kasnacheev > > > > > > ср, 2 дек. 2020 г. в 18:23, Max Timonin : > > > > > Hi, > > > > > > I don't think so. It looks like a bug that tests fail if one runs them > > > within their module (actually, what is the goal of this test?). The > check > > > showed us this problem, there is no need to fix the check. > > > > > > Currently I see two ways: > > > 1. Find the right module for every misplaced test. There are 104 tests, > > > maybe just move them all to the target module? If TeamCity runs them > > within > > > the indexing module only is there a reason to have a test in the core > > > module at all? > > > 2. Back to my previous proposal - create fake suites within a module, > > then > > > replace test classes in a target suite with the single class of the > fake > > > suite. > > > > > > > > > > > > On Wed, Dec 2, 2020 at 5:38 PM Ilya Kasnacheev < > > ilya.kasnach...@gmail.com> > > > wrote: > > > > > > > Hello! > > > > > > > > I think this means that we should abandon the plan of moving tests > > > between > > > > suites, and that your tool has to understand the dependency graph > > > > between modules' tests when assessing what's included and what's not. > > > > > > > > Regards, > > > > -- > > > > Ilya Kasnacheev > > > > > > > > > > > > ср, 2 дек. 2020 г. в 15:56, Max Timonin : > > > > > > > > > Hi, Ilya! > > > > > > > > > > I've checked testsuites. There is an issue. For example > > > > > *IgniteBinaryCacheQueryTestSuite* suite is now in 2 modules: > > > ignite-core, > > > > > ignite-indexing. On TeamCity it runs by "Query 1" suite. Simplified > > > maven > > > > > command for the suite is > > > > > > > > > > mvn -DtestIgniteBinaryCacheQueryTestSuite -am -pl :ignite-indexing > > > > > surefire:test > > > > > > > > > > Sequence of actions is: > > > > > 1. Find modules dependencies (*-am* flag): ignite-tools, > ignite-core; > > > > > 2. Run the test command for every module. In this step the maven > > tries > > > to > > > > > find the specified test for every module. This is good news, so we > > > don't > > > > > need to create new TeamCity suites for such splitted suites. > > > > > > > > > > But the run performs within the current module classpath, so for > the > > > core > > > > > module the test suite fails with error "Add module > 'ignite-indexing' > > to > > > > the > > > > > classpath of all Ignite nodes". Maven can't resolve it. > > > > > > > > > > The only way to work with it is to specify additional classpath > > > elements > > > > > for tests with setting > > > > *-Dmaven.test.additionalClasspath=/path/to/m2/jar*. > > > > > I did it by filling MAVEN_OPTS with the setting. Please check the > job > > > > > parameters [1]. After that the core module part ran successfully. > It > > > > m
Re: [DISCUSS] Missed (non-suited) tests
Hi, I don't think so. It looks like a bug that tests fail if one runs them within their module (actually, what is the goal of this test?). The check showed us this problem, there is no need to fix the check. Currently I see two ways: 1. Find the right module for every misplaced test. There are 104 tests, maybe just move them all to the target module? If TeamCity runs them within the indexing module only is there a reason to have a test in the core module at all? 2. Back to my previous proposal - create fake suites within a module, then replace test classes in a target suite with the single class of the fake suite. On Wed, Dec 2, 2020 at 5:38 PM Ilya Kasnacheev wrote: > Hello! > > I think this means that we should abandon the plan of moving tests between > suites, and that your tool has to understand the dependency graph > between modules' tests when assessing what's included and what's not. > > Regards, > -- > Ilya Kasnacheev > > > ср, 2 дек. 2020 г. в 15:56, Max Timonin : > > > Hi, Ilya! > > > > I've checked testsuites. There is an issue. For example > > *IgniteBinaryCacheQueryTestSuite* suite is now in 2 modules: ignite-core, > > ignite-indexing. On TeamCity it runs by "Query 1" suite. Simplified maven > > command for the suite is > > > > mvn -DtestIgniteBinaryCacheQueryTestSuite -am -pl :ignite-indexing > > surefire:test > > > > Sequence of actions is: > > 1. Find modules dependencies (*-am* flag): ignite-tools, ignite-core; > > 2. Run the test command for every module. In this step the maven tries to > > find the specified test for every module. This is good news, so we don't > > need to create new TeamCity suites for such splitted suites. > > > > But the run performs within the current module classpath, so for the core > > module the test suite fails with error "Add module 'ignite-indexing' to > the > > classpath of all Ignite nodes". Maven can't resolve it. > > > > The only way to work with it is to specify additional classpath elements > > for tests with setting > *-Dmaven.test.additionalClasspath=/path/to/m2/jar*. > > I did it by filling MAVEN_OPTS with the setting. Please check the job > > parameters [1]. After that the core module part ran successfully. It > means > > for every TC suite that runs such splitted suite we need to set the > > setting. What do you think, is it a valid way to handle the issue? If > there > > are no objections, I will check other such suites. > > > > Also to mention there, the work directory contains a *repository/* folder > > with all required .jars. But usage of this path in the setting didn't > help. > > I'm not sure, but I think it's an issue due to usage of Classworlds. So, > > using dependency from .m2 is the only way. > > > > [1] > > > > > https://ci.ignite.apache.org/viewLog.html?buildId=5770727=IgniteTests24Java8_Queries1=buildParameters > > > > > > > > On Fri, Nov 27, 2020 at 3:55 PM Max Timonin > > wrote: > > > > > Sure, I'll do that. > > > > > > On Fri, Nov 27, 2020 at 2:00 PM Ilya Kasnacheev < > > ilya.kasnach...@gmail.com> > > > wrote: > > > > > >> Hello! > > >> > > >> You can override these values (module, suites) values when running a > > suite > > >> on TC. Can you please run these ones which need to be changed > > individually > > >> on TC, make sure they run without errors and contain all the needed > > tests, > > >> and link to these runs in the ticket? Then I can modify the suites to > > fit > > >> those. > > >> > > >> I'm not sure that class shadowing will work as we want it to work, > e.g., > > >> we > > >> now have two IgniteCacheQuerySelfTestSuite6 with the same FQDN, I'm > not > > >> sure if maven/TC is going to pick both or just one. > > >> Maybe they should go to a different package, e.g., testsuites/core for > > >> every suite already present in indexing/spring/etc. Maybe you can > rename > > >> them just now? This will mean a lot less of work reconfiguring suites. > > >> In TC configurations, suite names are simple class names, not FQ, so > no > > >> changes may be needed at all. > > >> > > >> Regards, > > >> -- > > >> Ilya Kasnacheev > > >> > > >> > > >> пт, 27 нояб. 2020 г. в 13:03, Max Timonin : > > >> > > >> > Hi, sorry for the misleading. I mean "adding ignite-core module > > >
Re: [DISCUSS] Missed (non-suited) tests
Hi, Ilya! I've checked testsuites. There is an issue. For example *IgniteBinaryCacheQueryTestSuite* suite is now in 2 modules: ignite-core, ignite-indexing. On TeamCity it runs by "Query 1" suite. Simplified maven command for the suite is mvn -DtestIgniteBinaryCacheQueryTestSuite -am -pl :ignite-indexing surefire:test Sequence of actions is: 1. Find modules dependencies (*-am* flag): ignite-tools, ignite-core; 2. Run the test command for every module. In this step the maven tries to find the specified test for every module. This is good news, so we don't need to create new TeamCity suites for such splitted suites. But the run performs within the current module classpath, so for the core module the test suite fails with error "Add module 'ignite-indexing' to the classpath of all Ignite nodes". Maven can't resolve it. The only way to work with it is to specify additional classpath elements for tests with setting *-Dmaven.test.additionalClasspath=/path/to/m2/jar*. I did it by filling MAVEN_OPTS with the setting. Please check the job parameters [1]. After that the core module part ran successfully. It means for every TC suite that runs such splitted suite we need to set the setting. What do you think, is it a valid way to handle the issue? If there are no objections, I will check other such suites. Also to mention there, the work directory contains a *repository/* folder with all required .jars. But usage of this path in the setting didn't help. I'm not sure, but I think it's an issue due to usage of Classworlds. So, using dependency from .m2 is the only way. [1] https://ci.ignite.apache.org/viewLog.html?buildId=5770727=IgniteTests24Java8_Queries1=buildParameters On Fri, Nov 27, 2020 at 3:55 PM Max Timonin wrote: > Sure, I'll do that. > > On Fri, Nov 27, 2020 at 2:00 PM Ilya Kasnacheev > wrote: > >> Hello! >> >> You can override these values (module, suites) values when running a suite >> on TC. Can you please run these ones which need to be changed individually >> on TC, make sure they run without errors and contain all the needed tests, >> and link to these runs in the ticket? Then I can modify the suites to fit >> those. >> >> I'm not sure that class shadowing will work as we want it to work, e.g., >> we >> now have two IgniteCacheQuerySelfTestSuite6 with the same FQDN, I'm not >> sure if maven/TC is going to pick both or just one. >> Maybe they should go to a different package, e.g., testsuites/core for >> every suite already present in indexing/spring/etc. Maybe you can rename >> them just now? This will mean a lot less of work reconfiguring suites. >> In TC configurations, suite names are simple class names, not FQ, so no >> changes may be needed at all. >> >> Regards, >> -- >> Ilya Kasnacheev >> >> >> пт, 27 нояб. 2020 г. в 13:03, Max Timonin : >> >> > Hi, sorry for the misleading. I mean "adding ignite-core module >> *suites* to >> > the TeamCity Queries* suite" >> > >> > On Fri, Nov 27, 2020 at 12:44 PM Ilya Kasnacheev < >> > ilya.kasnach...@gmail.com> >> > wrote: >> > >> > > Hello! >> > > >> > > What do you mean by "adding ignite-core to suite"? ignite-core is a >> top >> > > dependency and its tests are also included in all other modules' tests >> > > classpath since it provides GridAbstractTest. >> > > >> > > Regards, >> > > -- >> > > Ilya Kasnacheev >> > > >> > > >> > > пт, 27 нояб. 2020 г. в 01:24, Max Timonin : >> > > >> > > > Hi, Ilya! >> > > > >> > > > So, I've updated PR, fixed comments and removed Core* prefixes. >> MTCGA >> > > shows >> > > > no blockers, but it was 2 weeks ago, so I've started it again. >> > > > >> > > > If PR is OK then there are some suites that should be updated on TC. >> > > Could >> > > > you please tell me how we can do it? >> > > > >> > > > 1. Add ignite-cassandra-serializers suite: >> > > > >> > > >1. org.apache.ignite.tests.SerializerSuite >> > > > >> > > > 2. Add ignite-core to Queries* TC suite: >> > > > >> > > >1. org.apache.ignite.client.IgniteClientTestSuite >> > > >2. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite >> > > >3. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite2 >> > > >4. org.apache.ignite.suites.IgniteCacheQuerySel
Re: [DISCUSS] Missed (non-suited) tests
Sure, I'll do that. On Fri, Nov 27, 2020 at 2:00 PM Ilya Kasnacheev wrote: > Hello! > > You can override these values (module, suites) values when running a suite > on TC. Can you please run these ones which need to be changed individually > on TC, make sure they run without errors and contain all the needed tests, > and link to these runs in the ticket? Then I can modify the suites to fit > those. > > I'm not sure that class shadowing will work as we want it to work, e.g., we > now have two IgniteCacheQuerySelfTestSuite6 with the same FQDN, I'm not > sure if maven/TC is going to pick both or just one. > Maybe they should go to a different package, e.g., testsuites/core for > every suite already present in indexing/spring/etc. Maybe you can rename > them just now? This will mean a lot less of work reconfiguring suites. > In TC configurations, suite names are simple class names, not FQ, so no > changes may be needed at all. > > Regards, > -- > Ilya Kasnacheev > > > пт, 27 нояб. 2020 г. в 13:03, Max Timonin : > > > Hi, sorry for the misleading. I mean "adding ignite-core module *suites* > to > > the TeamCity Queries* suite" > > > > On Fri, Nov 27, 2020 at 12:44 PM Ilya Kasnacheev < > > ilya.kasnach...@gmail.com> > > wrote: > > > > > Hello! > > > > > > What do you mean by "adding ignite-core to suite"? ignite-core is a top > > > dependency and its tests are also included in all other modules' tests > > > classpath since it provides GridAbstractTest. > > > > > > Regards, > > > -- > > > Ilya Kasnacheev > > > > > > > > > пт, 27 нояб. 2020 г. в 01:24, Max Timonin : > > > > > > > Hi, Ilya! > > > > > > > > So, I've updated PR, fixed comments and removed Core* prefixes. MTCGA > > > shows > > > > no blockers, but it was 2 weeks ago, so I've started it again. > > > > > > > > If PR is OK then there are some suites that should be updated on TC. > > > Could > > > > you please tell me how we can do it? > > > > > > > > 1. Add ignite-cassandra-serializers suite: > > > > > > > >1. org.apache.ignite.tests.SerializerSuite > > > > > > > > 2. Add ignite-core to Queries* TC suite: > > > > > > > >1. org.apache.ignite.client.IgniteClientTestSuite > > > >2. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite > > > >3. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite2 > > > >4. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite3 > > > >5. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite4 > > > >6. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite5 > > > >7. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite6 > > > >8. org.apache.ignite.suites.IgnitePdsWithIndexingCoreTestSuite > > > >9. org.apache.ignite.suites.IgniteCacheMvccSqlTestSuite > > > > > > > > 3. Remove ignite-indexing from TC suites: > > > > > > > >1. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite3 > > > >2. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite4 > > > >3. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite5 > > > > > > > > 4. Add ignite-core to Spring* TC suite: > > > > > > > >1. org.apache.ignite.testsuites.IgniteSpringTestSuite > > > > > > > > 5. Add ignite-core suite (depends on uri-deployment module): > > > > > > > >1. org.apache.ignite.testsuites.IgniteUriDeploymentTestSuite > > > > > > > > 6. Add ignite-core suite to Zookeeper TC suite: > > > > > > > >1. org.apache.ignite.testsuites.ZookeeperDiscoverySpiTestSuite3 > > > > > > > > 7. Remove ignite-zookeeper test suite: > > > > > > > >1. org.apache.ignite.testsuites.ZookeeperDiscoverySpiTestSuite3 > > > > > > > > 8. Add ignite-ml test suites: > > > > > > > >1. org.apache.ignite.ml.math.distances.DistancesTestSuite > > > >2. org.apache.ignite.ml.naivebayes.NaiveBayesTestSuite > > > > > > > > > > > > On Wed, Nov 25, 2020 at 4:26 PM Ilya Kasnacheev < > > > ilya.kasnach...@gmail.com > > > > > > > > > wrote: > > > > > > > > > Hello! > > > > > > > > > > Yes, we have such tests whic
Re: [DISCUSS] Missed (non-suited) tests
Hi, sorry for the misleading. I mean "adding ignite-core module *suites* to the TeamCity Queries* suite" On Fri, Nov 27, 2020 at 12:44 PM Ilya Kasnacheev wrote: > Hello! > > What do you mean by "adding ignite-core to suite"? ignite-core is a top > dependency and its tests are also included in all other modules' tests > classpath since it provides GridAbstractTest. > > Regards, > -- > Ilya Kasnacheev > > > пт, 27 нояб. 2020 г. в 01:24, Max Timonin : > > > Hi, Ilya! > > > > So, I've updated PR, fixed comments and removed Core* prefixes. MTCGA > shows > > no blockers, but it was 2 weeks ago, so I've started it again. > > > > If PR is OK then there are some suites that should be updated on TC. > Could > > you please tell me how we can do it? > > > > 1. Add ignite-cassandra-serializers suite: > > > >1. org.apache.ignite.tests.SerializerSuite > > > > 2. Add ignite-core to Queries* TC suite: > > > >1. org.apache.ignite.client.IgniteClientTestSuite > >2. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite > >3. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite2 > >4. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite3 > >5. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite4 > >6. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite5 > >7. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite6 > >8. org.apache.ignite.suites.IgnitePdsWithIndexingCoreTestSuite > >9. org.apache.ignite.suites.IgniteCacheMvccSqlTestSuite > > > > 3. Remove ignite-indexing from TC suites: > > > >1. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite3 > >2. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite4 > >3. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite5 > > > > 4. Add ignite-core to Spring* TC suite: > > > >1. org.apache.ignite.testsuites.IgniteSpringTestSuite > > > > 5. Add ignite-core suite (depends on uri-deployment module): > > > >1. org.apache.ignite.testsuites.IgniteUriDeploymentTestSuite > > > > 6. Add ignite-core suite to Zookeeper TC suite: > > > >1. org.apache.ignite.testsuites.ZookeeperDiscoverySpiTestSuite3 > > > > 7. Remove ignite-zookeeper test suite: > > > >1. org.apache.ignite.testsuites.ZookeeperDiscoverySpiTestSuite3 > > > > 8. Add ignite-ml test suites: > > > >1. org.apache.ignite.ml.math.distances.DistancesTestSuite > >2. org.apache.ignite.ml.naivebayes.NaiveBayesTestSuite > > > > > > On Wed, Nov 25, 2020 at 4:26 PM Ilya Kasnacheev < > ilya.kasnach...@gmail.com > > > > > wrote: > > > > > Hello! > > > > > > Yes, we have such tests which depend on ignite-indexing or > ignite-spring. > > > They just need to be included in Spring* or Queries* test suite. Then > > they > > > will be executed on TC in the correct context. You can also run these > > tests > > > from IDEA by specifying other module as classpath. No need to move the > > > classes around. > > > > > > I will check the PR. > > > > > > Regards, > > > -- > > > Ilya Kasnacheev > > > > > > > > > ср, 25 нояб. 2020 г. в 00:22, Max Timonin : > > > > > > > Ilya, Anton, Ivan, hi! > > > > > > > > I fix some comments you leave in the PR. Also I checked some test > > suites > > > > and found that some tests are written in the core module but depend > on > > > the > > > > indexing module (or other modules). Some of such test classes contain > > > tests > > > > that are related to the core functionality, but some to indexing. I'm > > not > > > > sure if it is correct to move a whole suite with all tests from the > > > > indexing module to the core, as it will hide some core tests from the > > > core > > > > module. > > > > > > > > I believe that the correct solution is to investigate every such test > > and > > > > move it to the right module. But I think this work will take a lot of > > > time > > > > and should be performed in a separate ticket, I will do it in the > > > > background. > > > > > > > > I think currently we should proceed with a way I introduced in PR: > > > > 1. Create fake suites for all such tests (written in core, suited in > > > other > > > > modules: indexing/spring/zook
Re: [DISCUSS] Missed (non-suited) tests
Hi, Ilya! So, I've updated PR, fixed comments and removed Core* prefixes. MTCGA shows no blockers, but it was 2 weeks ago, so I've started it again. If PR is OK then there are some suites that should be updated on TC. Could you please tell me how we can do it? 1. Add ignite-cassandra-serializers suite: 1. org.apache.ignite.tests.SerializerSuite 2. Add ignite-core to Queries* TC suite: 1. org.apache.ignite.client.IgniteClientTestSuite 2. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite 3. org.apache.ignite.suites.IgniteBinaryCacheQueryTestSuite2 4. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite3 5. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite4 6. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite5 7. org.apache.ignite.suites.IgniteCacheQuerySelfTestSuite6 8. org.apache.ignite.suites.IgnitePdsWithIndexingCoreTestSuite 9. org.apache.ignite.suites.IgniteCacheMvccSqlTestSuite 3. Remove ignite-indexing from TC suites: 1. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite3 2. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite4 3. org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite5 4. Add ignite-core to Spring* TC suite: 1. org.apache.ignite.testsuites.IgniteSpringTestSuite 5. Add ignite-core suite (depends on uri-deployment module): 1. org.apache.ignite.testsuites.IgniteUriDeploymentTestSuite 6. Add ignite-core suite to Zookeeper TC suite: 1. org.apache.ignite.testsuites.ZookeeperDiscoverySpiTestSuite3 7. Remove ignite-zookeeper test suite: 1. org.apache.ignite.testsuites.ZookeeperDiscoverySpiTestSuite3 8. Add ignite-ml test suites: 1. org.apache.ignite.ml.math.distances.DistancesTestSuite 2. org.apache.ignite.ml.naivebayes.NaiveBayesTestSuite On Wed, Nov 25, 2020 at 4:26 PM Ilya Kasnacheev wrote: > Hello! > > Yes, we have such tests which depend on ignite-indexing or ignite-spring. > They just need to be included in Spring* or Queries* test suite. Then they > will be executed on TC in the correct context. You can also run these tests > from IDEA by specifying other module as classpath. No need to move the > classes around. > > I will check the PR. > > Regards, > -- > Ilya Kasnacheev > > > ср, 25 нояб. 2020 г. в 00:22, Max Timonin : > > > Ilya, Anton, Ivan, hi! > > > > I fix some comments you leave in the PR. Also I checked some test suites > > and found that some tests are written in the core module but depend on > the > > indexing module (or other modules). Some of such test classes contain > tests > > that are related to the core functionality, but some to indexing. I'm not > > sure if it is correct to move a whole suite with all tests from the > > indexing module to the core, as it will hide some core tests from the > core > > module. > > > > I believe that the correct solution is to investigate every such test and > > move it to the right module. But I think this work will take a lot of > time > > and should be performed in a separate ticket, I will do it in the > > background. > > > > I think currently we should proceed with a way I introduced in PR: > > 1. Create fake suites for all such tests (written in core, suited in > other > > modules: indexing/spring/zookeeper/etc) in the core module. I named such > > suites with prefix Core*. > > 2. Replace tests in modules with links to fake suites. > > 3. Create an umbrella Jira ticket to discover every fake suite and > replace > > it with a new one in the right module. > > 4. Merge this PR for introducing a new travis check to avoid losing > > new tests. > > > > WDYT? > > > > List of such mixed suites: > > > > 1. suite IgniteBinaryCacheQueryTestSuite > > > > test GridCacheQueryIndexingDisabledSelfTest > > test IgniteCacheBinaryObjectsScanSelfTest > > test IgniteCacheBinaryObjectsScanWithEventsSelfTest) > > > > > > 2. suite IgniteCacheQuerySelfTestSuite3 > > > > test GridCacheContinuousQueryNodesFilteringTest > > > > > > 3. suite IgniteCacheQuerySelfTestSuite5 > > > > test ContinuousQueryRemoteFilterMissingInClassPathSelfTest > > > > 4. suite IgniteCacheQuerySelfTestSuite6 > > > > test CacheContinuousQueryOperationP2PTest > > > > test CacheContinuousQueryFilterDeploymentFailedTest > > > > > > 5. all tests in suite IgnitePdsWithIndexingCoreTestSuite > > > > > > 6. and some others. > > > > On Wed, Nov 18, 2020 at 12:38 PM Max Timonin > > wrote: > > > > > Hi Ilya! Thank you for up the topic. I will come back with fixes and > > > comments in a couple of days. > > > > > &g
Re: [DISCUSS] Missed (non-suited) tests
Ilya, Anton, Ivan, hi! I fix some comments you leave in the PR. Also I checked some test suites and found that some tests are written in the core module but depend on the indexing module (or other modules). Some of such test classes contain tests that are related to the core functionality, but some to indexing. I'm not sure if it is correct to move a whole suite with all tests from the indexing module to the core, as it will hide some core tests from the core module. I believe that the correct solution is to investigate every such test and move it to the right module. But I think this work will take a lot of time and should be performed in a separate ticket, I will do it in the background. I think currently we should proceed with a way I introduced in PR: 1. Create fake suites for all such tests (written in core, suited in other modules: indexing/spring/zookeeper/etc) in the core module. I named such suites with prefix Core*. 2. Replace tests in modules with links to fake suites. 3. Create an umbrella Jira ticket to discover every fake suite and replace it with a new one in the right module. 4. Merge this PR for introducing a new travis check to avoid losing new tests. WDYT? List of such mixed suites: 1. suite IgniteBinaryCacheQueryTestSuite test GridCacheQueryIndexingDisabledSelfTest test IgniteCacheBinaryObjectsScanSelfTest test IgniteCacheBinaryObjectsScanWithEventsSelfTest) 2. suite IgniteCacheQuerySelfTestSuite3 test GridCacheContinuousQueryNodesFilteringTest 3. suite IgniteCacheQuerySelfTestSuite5 test ContinuousQueryRemoteFilterMissingInClassPathSelfTest 4. suite IgniteCacheQuerySelfTestSuite6 test CacheContinuousQueryOperationP2PTest test CacheContinuousQueryFilterDeploymentFailedTest 5. all tests in suite IgnitePdsWithIndexingCoreTestSuite 6. and some others. On Wed, Nov 18, 2020 at 12:38 PM Max Timonin wrote: > Hi Ilya! Thank you for up the topic. I will come back with fixes and > comments in a couple of days. > > On Tue, Nov 17, 2020 at 4:26 PM Ilya Kasnacheev > wrote: > >> Hello! >> >> I have left some comments and there's also more discussion there. Can you >> please look? >> >> Thanks, >> -- >> Ilya Kasnacheev >> >> >> вт, 3 нояб. 2020 г. в 00:03, Max Timonin : >> >> > Hi! >> > >> > I've updated PR: https://github.com/apache/ignite/pull/8367. Anton, >> Ivan, >> > Ivan could you please review it? >> > >> > Some moments to mention: >> > 1. I've added new suites: SerializerSuite >> (ignite-cassandra-serializers), >> > DistanceTestSuite, NaiveBayesTestSuite (ignite-ml). Should we configure >> a >> > TeamCity to run them? >> > >> > 2. Some tests marked as failed, I'll create corresponding tickets for >> them >> > after PR approved: >> > - IgnitePKIndexesMigrationToUnwrapPkTest >> > - P2PGridifySelfTest >> > - GridCacheMultithreadedFailoverAbstractTest >> > - WalCompactionAfterRestartTest >> > - GridTcpCommunicationSpiLogTest >> > - ComplexSecondaryKeyUnwrapSelfTest >> > - SqlTransactionsSelfTest >> > >> > 3. Add docs to DEVNOTES.txt >> > >> > On Mon, Nov 2, 2020 at 11:44 AM Anton Vinogradov wrote: >> > >> > > > As I understand we >> > > > can't just move suites between modules, as TeamCity may depend on >> the >> > > path >> > > > to them. >> > > See no problem to update TC as well. >> > > >> > > On Fri, Oct 30, 2020 at 4:32 PM Ivan Daschinsky >> > > wrote: >> > > >> > > > I suggests to mark these tests with @Ignore and file tickets to fix >> > them. >> > > > >> > > > пт, 30 окт. 2020 г. в 16:26, Ivan Daschinsky : >> > > > >> > > > > Hi >> > > > > >> > > > > WalCompactionAfterRestartTest -- yes we need it. This test failed >> > > because >> > > > > of race (test shold be rewritten a little bit) >> > > > > >> > > > > пт, 30 окт. 2020 г. в 16:15, Max Timonin > >: >> > > > > >> > > > >> Hi! >> > > > >> >> > > > >> Yes, you're correct. I've developed the check and started to >> clean >> > > tests >> > > > >> (move them to suites, mark some tests with Ignore, etc.). I >> finish >> > > work >> > > > on >> > > > >> the core module. I hope it was the biggest one, and others are >> less. >> > > If >> > > > >> so, >> > >
Re: [DISCUSS] Missed (non-suited) tests
Hi Ilya! Thank you for up the topic. I will come back with fixes and comments in a couple of days. On Tue, Nov 17, 2020 at 4:26 PM Ilya Kasnacheev wrote: > Hello! > > I have left some comments and there's also more discussion there. Can you > please look? > > Thanks, > -- > Ilya Kasnacheev > > > вт, 3 нояб. 2020 г. в 00:03, Max Timonin : > > > Hi! > > > > I've updated PR: https://github.com/apache/ignite/pull/8367. Anton, > Ivan, > > Ivan could you please review it? > > > > Some moments to mention: > > 1. I've added new suites: SerializerSuite (ignite-cassandra-serializers), > > DistanceTestSuite, NaiveBayesTestSuite (ignite-ml). Should we configure a > > TeamCity to run them? > > > > 2. Some tests marked as failed, I'll create corresponding tickets for > them > > after PR approved: > > - IgnitePKIndexesMigrationToUnwrapPkTest > > - P2PGridifySelfTest > > - GridCacheMultithreadedFailoverAbstractTest > > - WalCompactionAfterRestartTest > > - GridTcpCommunicationSpiLogTest > > - ComplexSecondaryKeyUnwrapSelfTest > > - SqlTransactionsSelfTest > > > > 3. Add docs to DEVNOTES.txt > > > > On Mon, Nov 2, 2020 at 11:44 AM Anton Vinogradov wrote: > > > > > > As I understand we > > > > can't just move suites between modules, as TeamCity may depend on the > > > path > > > > to them. > > > See no problem to update TC as well. > > > > > > On Fri, Oct 30, 2020 at 4:32 PM Ivan Daschinsky > > > wrote: > > > > > > > I suggests to mark these tests with @Ignore and file tickets to fix > > them. > > > > > > > > пт, 30 окт. 2020 г. в 16:26, Ivan Daschinsky : > > > > > > > > > Hi > > > > > > > > > > WalCompactionAfterRestartTest -- yes we need it. This test failed > > > because > > > > > of race (test shold be rewritten a little bit) > > > > > > > > > > пт, 30 окт. 2020 г. в 16:15, Max Timonin >: > > > > > > > > > >> Hi! > > > > >> > > > > >> Yes, you're correct. I've developed the check and started to clean > > > tests > > > > >> (move them to suites, mark some tests with Ignore, etc.). I finish > > > work > > > > on > > > > >> the core module. I hope it was the biggest one, and others are > less. > > > If > > > > >> so, > > > > >> I think I will finish the work on other modules in 1 or 2 weeks, > as > > I > > > do > > > > >> this activity in the background (~10% of my work time). Actually > > I've > > > > >> found > > > > >> 3 failed tests in the core module that aren't in any suite, so I > > need > > > > time > > > > >> to discover reason of failures and if we actually need those > tests: > > > > >> > > > > >> GridCacheMultithreadedFailoverAbstractTest > > > > >> WalCompactionAfterRestartTest > > > > >> GridTcpCommunicationSpiLogTest > > > > >> > > > > >> Also we should decide how to be with wrongly located es. As I > > > understand > > > > >> we > > > > >> can't just move suites between modules, as TeamCity may depend on > > the > > > > path > > > > >> to them. So, for such cases I've just created suites in the right > > > > module, > > > > >> and replaced the test list with the new class suite. It does not > > look > > > > >> pretty enough, but I think It's a path of least resistance. WDYT? > > > > >> > > > > >> BEFORE: > > > > >> Module A -> SuiteA -> testA1, testA2, testB1, testB2 > > > > >> Module B -> testB1, testB2 > > > > >> > > > > >> AFTER: > > > > >> Module A -> SuiteA, SuiteB > > > > >> Module B -> SuiteB -> testB1, testB2 > > > > >> > > > > >> On Fri, Oct 30, 2020 at 3:38 PM Anton Vinogradov > > > wrote: > > > > >> > > > > >> > Folks, > > > > >> > What's the current state of this thread? > > > > >> > AFAIU, we found unused and wrongly located tests and developed > > some > > > > >> > checker, could we split this to some PRs? >
Re: [DISCUSS] Missed (non-suited) tests
Hi! I've updated PR: https://github.com/apache/ignite/pull/8367. Anton, Ivan, Ivan could you please review it? Some moments to mention: 1. I've added new suites: SerializerSuite (ignite-cassandra-serializers), DistanceTestSuite, NaiveBayesTestSuite (ignite-ml). Should we configure a TeamCity to run them? 2. Some tests marked as failed, I'll create corresponding tickets for them after PR approved: - IgnitePKIndexesMigrationToUnwrapPkTest - P2PGridifySelfTest - GridCacheMultithreadedFailoverAbstractTest - WalCompactionAfterRestartTest - GridTcpCommunicationSpiLogTest - ComplexSecondaryKeyUnwrapSelfTest - SqlTransactionsSelfTest 3. Add docs to DEVNOTES.txt On Mon, Nov 2, 2020 at 11:44 AM Anton Vinogradov wrote: > > As I understand we > > can't just move suites between modules, as TeamCity may depend on the > path > > to them. > See no problem to update TC as well. > > On Fri, Oct 30, 2020 at 4:32 PM Ivan Daschinsky > wrote: > > > I suggests to mark these tests with @Ignore and file tickets to fix them. > > > > пт, 30 окт. 2020 г. в 16:26, Ivan Daschinsky : > > > > > Hi > > > > > > WalCompactionAfterRestartTest -- yes we need it. This test failed > because > > > of race (test shold be rewritten a little bit) > > > > > > пт, 30 окт. 2020 г. в 16:15, Max Timonin : > > > > > >> Hi! > > >> > > >> Yes, you're correct. I've developed the check and started to clean > tests > > >> (move them to suites, mark some tests with Ignore, etc.). I finish > work > > on > > >> the core module. I hope it was the biggest one, and others are less. > If > > >> so, > > >> I think I will finish the work on other modules in 1 or 2 weeks, as I > do > > >> this activity in the background (~10% of my work time). Actually I've > > >> found > > >> 3 failed tests in the core module that aren't in any suite, so I need > > time > > >> to discover reason of failures and if we actually need those tests: > > >> > > >> GridCacheMultithreadedFailoverAbstractTest > > >> WalCompactionAfterRestartTest > > >> GridTcpCommunicationSpiLogTest > > >> > > >> Also we should decide how to be with wrongly located es. As I > understand > > >> we > > >> can't just move suites between modules, as TeamCity may depend on the > > path > > >> to them. So, for such cases I've just created suites in the right > > module, > > >> and replaced the test list with the new class suite. It does not look > > >> pretty enough, but I think It's a path of least resistance. WDYT? > > >> > > >> BEFORE: > > >> Module A -> SuiteA -> testA1, testA2, testB1, testB2 > > >> Module B -> testB1, testB2 > > >> > > >> AFTER: > > >> Module A -> SuiteA, SuiteB > > >> Module B -> SuiteB -> testB1, testB2 > > >> > > >> On Fri, Oct 30, 2020 at 3:38 PM Anton Vinogradov > wrote: > > >> > > >> > Folks, > > >> > What's the current state of this thread? > > >> > AFAIU, we found unused and wrongly located tests and developed some > > >> > checker, could we split this to some PRs? > > >> > Let's merge tests usage fix and location fixes first, this will > > provide > > >> us > > >> > an ability to automate check using Travis. > > >> > > > >> > On Tue, Oct 20, 2020 at 12:06 PM Ivan Pavlukhin < > vololo...@gmail.com> > > >> > wrote: > > >> > > > >> > > Max, Ivan, > > >> > > > > >> > > Using explicit @Ignore and the automated check sounds good to me. > If > > >> > > nobody has arguments against it I think we should do it. > > >> > > > > >> > > 2020-10-19 19:30 GMT+03:00, Max Timonin >: > > >> > > > Hi Ivan, > > >> > > > > > >> > > > I've checked the ticket you provide. It contains subtasks to > > >> uncomment > > >> > or > > >> > > > to remove some unused tests. It definitely describes some cases > > I've > > >> > > found. > > >> > > > So what do you think if I uncomment them in suites, add @Ignore > > >> > > annotation > > >> > > > for those tests while the tickets are open? This will help to &g
Re: [DISCUSS] Missed (non-suited) tests
Hi! Yes, you're correct. I've developed the check and started to clean tests (move them to suites, mark some tests with Ignore, etc.). I finish work on the core module. I hope it was the biggest one, and others are less. If so, I think I will finish the work on other modules in 1 or 2 weeks, as I do this activity in the background (~10% of my work time). Actually I've found 3 failed tests in the core module that aren't in any suite, so I need time to discover reason of failures and if we actually need those tests: GridCacheMultithreadedFailoverAbstractTest WalCompactionAfterRestartTest GridTcpCommunicationSpiLogTest Also we should decide how to be with wrongly located es. As I understand we can't just move suites between modules, as TeamCity may depend on the path to them. So, for such cases I've just created suites in the right module, and replaced the test list with the new class suite. It does not look pretty enough, but I think It's a path of least resistance. WDYT? BEFORE: Module A -> SuiteA -> testA1, testA2, testB1, testB2 Module B -> testB1, testB2 AFTER: Module A -> SuiteA, SuiteB Module B -> SuiteB -> testB1, testB2 On Fri, Oct 30, 2020 at 3:38 PM Anton Vinogradov wrote: > Folks, > What's the current state of this thread? > AFAIU, we found unused and wrongly located tests and developed some > checker, could we split this to some PRs? > Let's merge tests usage fix and location fixes first, this will provide us > an ability to automate check using Travis. > > On Tue, Oct 20, 2020 at 12:06 PM Ivan Pavlukhin > wrote: > > > Max, Ivan, > > > > Using explicit @Ignore and the automated check sounds good to me. If > > nobody has arguments against it I think we should do it. > > > > 2020-10-19 19:30 GMT+03:00, Max Timonin : > > > Hi Ivan, > > > > > > I've checked the ticket you provide. It contains subtasks to uncomment > or > > > to remove some unused tests. It definitely describes some cases I've > > found. > > > So what do you think if I uncomment them in suites, add @Ignore > > annotation > > > for those tests while the tickets are open? This will help to find out > > > tests that were forgiven in a recent time. > > > > > > Also I believe that this check must be automated. I didn't find a way > how > > > uncomment / unused tests are found in the ticket. If there is no any - > I > > > propose mine PR for this purpose. > > > > > > > > > > > > On Mon, Oct 19, 2020 at 5:24 PM Ivan Daschinsky > > > wrote: > > > > > >> Ivan, as far as I understand, Max also created verification check for > > not > > >> included test and found a few tests, that have never been included in > > any > > >> testsuites. > > >> > > >> Also, I suppose, that even if we cannot run some tests, these tests > > >> should > > >> be ignored using annotation, but not commented. > > >> > > >> пн, 19 окт. 2020 г. в 16:33, Ivan Pavlukhin : > > >> > > >> > Hi Max, > > >> > > > >> > There is an existing effort about "abandoned" tests > > >> > https://issues.apache.org/jira/browse/IGNITE-9210 > > >> > > > >> > 2020-10-19 16:25 GMT+03:00, Max Timonin : > > >> > > Hi Igniters! > > >> > > > > >> > > I made a research into tests that aren't included in any test > suite. > > >> > > As > > >> > > TeamCity runs tests by suites so there could be tests that never > run > > >> > > on > > >> > TC. > > >> > > So I tried implementing a simple check for such tests and include > it > > >> > > in > > >> > > Ignite's travis config. > > >> > > > > >> > > The check runs while "mvn test" command and piggy-backs on the > maven > > >> > > surefire plugin. I replaced the junit provider with a custom one > > that > > >> > > checks if a class is a test or a suite (there are some Ignite > > >> > > specific > > >> > > stuff), marks tests that are in suites and raises an exception if > > >> > > there > > >> > are > > >> > > non-suited tests. It's implemented as a part of maven command so > it > > >> runs > > >> > > for every module separately. > > >> > > > > >> > > I've prepared draft PR with this check: > > >> > > https://github.com/apa
Re: [DISCUSS] Missed (non-suited) tests
Hi Ivan, I've checked the ticket you provide. It contains subtasks to uncomment or to remove some unused tests. It definitely describes some cases I've found. So what do you think if I uncomment them in suites, add @Ignore annotation for those tests while the tickets are open? This will help to find out tests that were forgiven in a recent time. Also I believe that this check must be automated. I didn't find a way how uncomment / unused tests are found in the ticket. If there is no any - I propose mine PR for this purpose. On Mon, Oct 19, 2020 at 5:24 PM Ivan Daschinsky wrote: > Ivan, as far as I understand, Max also created verification check for not > included test and found a few tests, that have never been included in any > testsuites. > > Also, I suppose, that even if we cannot run some tests, these tests should > be ignored using annotation, but not commented. > > пн, 19 окт. 2020 г. в 16:33, Ivan Pavlukhin : > > > Hi Max, > > > > There is an existing effort about "abandoned" tests > > https://issues.apache.org/jira/browse/IGNITE-9210 > > > > 2020-10-19 16:25 GMT+03:00, Max Timonin : > > > Hi Igniters! > > > > > > I made a research into tests that aren't included in any test suite. As > > > TeamCity runs tests by suites so there could be tests that never run on > > TC. > > > So I tried implementing a simple check for such tests and include it in > > > Ignite's travis config. > > > > > > The check runs while "mvn test" command and piggy-backs on the maven > > > surefire plugin. I replaced the junit provider with a custom one that > > > checks if a class is a test or a suite (there are some Ignite specific > > > stuff), marks tests that are in suites and raises an exception if there > > are > > > non-suited tests. It's implemented as a part of maven command so it > runs > > > for every module separately. > > > > > > I've prepared draft PR with this check: > > > https://github.com/apache/ignite/pull/8367 > > > Travis check report is here: > > > https://travis-ci.org/github/apache/ignite/jobs/737046387 > > > As It's a draft, so I skip some maven configuration steps for a while. > > Also > > > I run the check only for the core module. > > > > > > But I have some results that want to discuss before continue the work: > > > 1. Currently in the core module there are 53 tests that aren't part of > > any > > > test suite. I'm not sure about the reason for every test. So I just put > > > below a list of the tests and last contributor to a file that contains > a > > > test. > > > > > > 2. Some tests are located in the core module, but suites are in a > > > different, for example ignite-indexing suite > > > IgniteCacheQuerySelfTestSuite3 contains > > > only tests written in the core module, and none from the indexing > module. > > > Also there are suites in spring, uri-deploy, zookeeper modules. In my > PR > > > I've just copied the test suites to the core module. > > > > > > 3. Some test classes are named with the "Abstract" suffix but don't > have > > > the corresponding modifier (for example, IgniteTxTimeoutAbstractTest). > > So, > > > I add the modifier for every such file if it's not a part of any suite. > > > > > > What do you think about this check? If Ignite needs it, let's discuss > > next > > > things: > > > 1. Mark tests that should never be in any suite by some reason; > > > 2. Fix the missed tests; > > > 3. How to declare suites that contains tests from a different module; > > > 4. How to check if TC runs all suites. > > > > > > List of non-suited tests in the core module: > > > > > > maksim.stepac...@gmail.com: > > > GridTcpCommunicationSpiLogTest > > > > > > nizhi...@apache.org: > > > IgniteCacheClientMultiNodeUpdateTopologyLockTest > > > CacheClientsConcurrentStartTest > > > IgniteOutOfMemoryPropagationTest > > > GridCacheP2PUndeploySelfTest > > > GridCacheRebalancingOrderingTest > > > IgniteMassLoadSandboxTest > > > PageLockTrackerMXBeanImplTest > > > IgniteBinaryMetadataUpdateNodeRestartTest > > > CacheLockCandidatesThreadTest > > > GridMBeanBaselineTest > > > RendezvousAffinityFunctionSimpleBenchmark > > > > > > samvi...@yandex.ru: > > > IgnitePdsNoSpaceLe
[DISCUSS] Missed (non-suited) tests
Hi Igniters! I made a research into tests that aren't included in any test suite. As TeamCity runs tests by suites so there could be tests that never run on TC. So I tried implementing a simple check for such tests and include it in Ignite's travis config. The check runs while "mvn test" command and piggy-backs on the maven surefire plugin. I replaced the junit provider with a custom one that checks if a class is a test or a suite (there are some Ignite specific stuff), marks tests that are in suites and raises an exception if there are non-suited tests. It's implemented as a part of maven command so it runs for every module separately. I've prepared draft PR with this check: https://github.com/apache/ignite/pull/8367 Travis check report is here: https://travis-ci.org/github/apache/ignite/jobs/737046387 As It's a draft, so I skip some maven configuration steps for a while. Also I run the check only for the core module. But I have some results that want to discuss before continue the work: 1. Currently in the core module there are 53 tests that aren't part of any test suite. I'm not sure about the reason for every test. So I just put below a list of the tests and last contributor to a file that contains a test. 2. Some tests are located in the core module, but suites are in a different, for example ignite-indexing suite IgniteCacheQuerySelfTestSuite3 contains only tests written in the core module, and none from the indexing module. Also there are suites in spring, uri-deploy, zookeeper modules. In my PR I've just copied the test suites to the core module. 3. Some test classes are named with the "Abstract" suffix but don't have the corresponding modifier (for example, IgniteTxTimeoutAbstractTest). So, I add the modifier for every such file if it's not a part of any suite. What do you think about this check? If Ignite needs it, let's discuss next things: 1. Mark tests that should never be in any suite by some reason; 2. Fix the missed tests; 3. How to declare suites that contains tests from a different module; 4. How to check if TC runs all suites. List of non-suited tests in the core module: maksim.stepac...@gmail.com: GridTcpCommunicationSpiLogTest nizhi...@apache.org: IgniteCacheClientMultiNodeUpdateTopologyLockTest CacheClientsConcurrentStartTest IgniteOutOfMemoryPropagationTest GridCacheP2PUndeploySelfTest GridCacheRebalancingOrderingTest IgniteMassLoadSandboxTest PageLockTrackerMXBeanImplTest IgniteBinaryMetadataUpdateNodeRestartTest CacheLockCandidatesThreadTest GridMBeanBaselineTest RendezvousAffinityFunctionSimpleBenchmark samvi...@yandex.ru: IgnitePdsNoSpaceLeftOnDeviceTest maxmu...@gmail.com: GridCacheOnCopyFlagReplicatedSelfTest GridCacheOnCopyFlagLocalSelfTest GridCacheReplicatedAtomicReferenceMultiNodeTest GridCacheReplicatedMarshallerTxTest GridCacheReplicatedTxConcurrentGetTest GridCacheOnCopyFlagTxPartitionedSelfTest GridCacheReplicatedTxReadTest GridCachePartitionedAtomicReferenceMultiNodeTest GridCacheOnCopyFlagAtomicSelfTest mmu...@apache.org: GridActivateExtensionTest IgniteChangeGlobalStateCacheTest IgniteChangeGlobalStateTest IgniteChangeGlobalStateServiceTest IgniteChangeGlobalStateDataStructureTest oignate...@gridgain.com: CacheEntryProcessorCopySelfTest MemoryLeaksOnRestartNodeTest GridCacheAtomicPreloadSelfTest WalCompactionAfterRestartTest IgniteCacheConcurrentPutGetRemove GridIoManagerBenchmark0 nsamelc...@gmail.com: GridLongRunningInitNewCrdFutureDiagnosticsTest GridCacheMultithreadedFailoverAbstractTest alexey.goncha...@gmail.com: GridCacheBinaryObjectsAtomicOnheapSelfTest GridCacheBinaryObjectsAtomicNearDisabledOnheapSelfTest GridCacheBinaryObjectsPartitionedOnheapSelfTest GridCacheBinaryObjectsPartitionedNearDisabledOnheapSelfTest vladis...@gmail.com: IgnitePartitionedLockSelfTest alexandr.bel...@xored.com: IgniteStableBaselineCachePutAllFailoverTest IgniteStableBaselineCacheRemoveFailoverTest ilant...@gridgain.com: IgniteCacheAtomicOnheapExpiryPolicyTest IgniteCacheAtomicLocalOnheapExpiryPolicyTest GridCacheReplicatedOnheapFullApiSelfTest GridCacheBinaryObjectsLocalOnheapSelfTest oignate...@users.noreply.github.com: GridCacheTtlManagerEvictionSelfTest ira...@apache.org: CommonPoolStarvationCheckpointTest alievmi...@gmail.com: RemoveAllDeadlockTest schugu...@gridgain.com: FullyConnectedComponentSearcherTest sboi...@gridgain.com: IgniteDataStructuresNoClassOnServerTest timonin.ma...@gmail.com: ReliableChannelTest ThinClientPartitionAwarenessDiscoveryTest
Broken test in master: BasicIndexTest
Hi, Igniters! I was discovering how indexes work and found a failed test. BasicIndexTest#testInlineSizeChange is broken in master and it's not a flaky case [1]. But it has been failing since 25/09 only. I discovered that it happened after the IGNITE-13207 ticket merged (Checkpointer code refactoring) [2]. I'm not sure about the expected behaviour of the inline index and how checkpointer affects it. But let's fix it if it is a bug or completely remove this test. [1] https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8=6131871779633595667=%3Cdefault%3E=testDetails [2] https://issues.apache.org/jira/browse/IGNITE-13207
Re: Thin Client Kubernetes discovery
Hi, StatefulSet provides predictable naming, then it should be easy to configure a client with addresses ignite-1,ignite-2...ignite-N. So there is no need in custom discovery, IPs etc. I think it corresponds to k8s patterns, as some pods are different from others as they store specific partitions (read, have a state). There will be some maintenance by a user - the list of server namings should be provided too unless it's very simple (ignite-XX). As StatefulSet is required to enable persistence, I think it's not a big problem to configure it the same way. And it should work out of the box. I will investigate how much cost is to implement custom discovery for thin clients. And compare it with the StatefulSet solution. On Thu, Aug 13, 2020 at 12:52 PM Pavel Tupitsyn wrote: > Vladimir, > > I agree with you, StatefulSet is not related here. > > > it's not a strict rule to communicate only directly with a pod > > running a node with a primary partition > Yes, if a node with a primary partition is not known or can't be contacted, > we fail over to a default (random) node > (afaik this is how Java, C++ and .NET thin clients are implemented) > > On Thu, Aug 13, 2020 at 12:44 PM Vladimir Pligin > wrote: > > > Hi guys, > > > > Maybe I'm missing something but I don't undestand how StatefulSet relates > > to > > the described functionality. > > StatefulSet is more about persistence. Correct me if I'm wrong but my > > current understanding is that we don't need to have any explicit state > for > > a > > thin client connection. I'd like this thing to be simple: if I'm working > > with a pod and it fails then I just go to another one and try my request > > again. The corner case is the best-effort affinity. As far as I can it's > > not > > a strict rule to communicate only directly with a pod running a node > with a > > primary partition. It's ok to fail-over in this case and communicate with > > any pod. Am I right? > > > > > > > > -- > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ > > >
Re: Thin Client Kubernetes discovery
In case of an enabled partition-awareness feature or sessions I suppose that a state appears. And as we have a state we should use the stateful deployment of kubernetes with StatefulSet [1]. In that case addresses of nodes are predictable and aren't changed so one can configure a client with them. I think it's a better solution than implementing specific discovery by self. Point is to rely on k8s discovery mechanisms as much as possible otherwise why use it? What do you think? I will prepare a demo with an enabled partition-awareness feature and configured StatefulSet. If you see any issues with it please let me know. [1] https://apacheignite.readme.io/docs/stateful-deployment On Thu, Aug 13, 2020 at 11:25 AM Pavel Tupitsyn wrote: > Max, Denis, > > Partition Awareness [1] is our main reason to have a specialized k8s > discovery mechanism in Thin Clients. > > k8s ingress mechanism is fine for a single client connection, but in > Partition Aware mode > thin clients need to connect to every server node, and track nodes as they > enter or leave the topology. > > Denis is right, current tickets [2] [3] are about Thin Clients being > deployed in the same k8s cluster as server nodes. > Using k8s APIs we can get every pod address and establish connections. > > [1] > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-23%3A+Best+Effort+Affinity+for+thin+clients > [2] https://issues.apache.org/jira/browse/IGNITE-13011 > <https://issues.apache.org/jira/browse/IGNITE-13204> > [3] https://issues.apache.org/jira/browse/IGNITE-13204 > > On Thu, Aug 13, 2020 at 10:48 AM Denis Magda wrote: > > > We're discussing the case when the thin client and server nodes are a > part > > of a single Kubernetes cluster. > > > > Ignite thick client uses KubernetesIPFinder > > <https://apacheignite.readme.io/docs/kubernetes-ip-finder> to > > auto-discover > > other Ignite pods. The IP finder gets IP addresses of other Ignite pods > via > > a special Service instance, and then the thick client uses those > addresses > > to communicate to the rest of the cluster bypassing the Service. I > > believe the ultimate goal of the JIRA task is to support a similar > > capability for the thin client. > > > > However, you solution might be the way to go. Still, I wonder how the > thin > > client will handle cases when a server pod, the client was connected to, > > goes down, and the Service connects the client to another pod > (considering > > the session affinity option). It might break some internal state of the > > connection on the client-side and lead to unpredictable behavior. Also, > if > > the partition-awareness feature is enabled, then the thin client will > start > > bypassing the Service for most of the key-value requests connecting to > the > > nodes directly using their IP addresses. > > > > - > > Denis > > > > > > On Wed, Aug 12, 2020 at 11:07 PM Max Timonin > > wrote: > > > > > I'm not sure what you mean by inside/outside of kubernetes. Service > name > > is > > > visible within k8s environment. I've described a case with connection > > from > > > another pod that is part of k8s cluster. > > > > > > To provide connection outside of Kubernetes one should configure > Ingress > > > [1]. It will have a fixed address assigned by a cluster administrator. > > Any > > > request outside of k8s to the Ingress will be proxied inside k8s env to > > the > > > Service and then to a pod automatically without need to provide any IP > > > addresses in a client configuration. > > > > > > [1] https://kubernetes.io/docs/concepts/services-networking/ingress/ > > > > > > On Thu, Aug 13, 2020 at 1:39 AM Denis Magda wrote: > > > > > > > Max, > > > > > > > > That improvement automates the cluster discovery if both the cluster > > and > > > > thin clients are deployed *inside* of Kubernetes. That's my reading > of > > > the > > > > ticket's description. While you're referring to the case when the > > client > > > is > > > > deployed outside of a K8S environment. > > > > > > > > @Vladimir Pligin , could you please join the > > > thread. > > > > I've seen you joined the discussion in JIRA. > > > > > > > > - > > > > Denis > > > > > > > > > > > > On Wed, Aug 12, 2020 at 12:06 PM Max Timonin < > timonin.ma...@gmail.com> > > > > wrote: > > > > > > >
Re: Thin Client Kubernetes discovery
I'm not sure what you mean by inside/outside of kubernetes. Service name is visible within k8s environment. I've described a case with connection from another pod that is part of k8s cluster. To provide connection outside of Kubernetes one should configure Ingress [1]. It will have a fixed address assigned by a cluster administrator. Any request outside of k8s to the Ingress will be proxied inside k8s env to the Service and then to a pod automatically without need to provide any IP addresses in a client configuration. [1] https://kubernetes.io/docs/concepts/services-networking/ingress/ On Thu, Aug 13, 2020 at 1:39 AM Denis Magda wrote: > Max, > > That improvement automates the cluster discovery if both the cluster and > thin clients are deployed *inside* of Kubernetes. That's my reading of the > ticket's description. While you're referring to the case when the client is > deployed outside of a K8S environment. > > @Vladimir Pligin , could you please join the thread. > I've seen you joined the discussion in JIRA. > > - > Denis > > > On Wed, Aug 12, 2020 at 12:06 PM Max Timonin > wrote: > > > Hi, Igniters > > > > I'm now discovering the issue > > https://issues.apache.org/jira/browse/IGNITE-13204 and wondering what > is a > > case that requires a client to be able to discover a cluster? > > > > I believe that discovery is a goal of kubernetes itself. We could assign > a > > DNS name for a Service [1] and then any request to the Service will be > > forwarded to any of Ignite pods behind it. So there is no need to extract > > pods IP to provide the connection. For example, I've configured a k8s > > service with name "ignite" and port mapping 10800:9888. Then just connect > > to it with sqlline by a string "ignite:9888". > > > > I think that it's OK that clients should be configured with those > settings > > as they aren't changed during cluster live. > > > > Also k8s provides SessionAffinity [2] to guarantee forwarding traffic > from > > one client to the same pod for every request. So there is no need to > > provide IP for a specific pod if a state is required. > > > > Maybe I'm missing something about this feature? Could somebody provide > more > > details for this task? > > > > Below is the example config and connection string: > > > > apiVersion: v1 > > kind: Service > > metadata: > > name: ignite > > spec: > > ports: > > - port: 9888 > > targetPort: 10800 > > selector: > > app: ignite > > > > ./bin/sqlline.sh --verbose=true -u jdbc:ignite:thin://ignite:9888 > > 0: jdbc:ignite:thin://ignite:9888> > > > > [1] https://kubernetes.io/docs/concepts/services-networking/service/ > > [2] > > > > > https://kubernetes.io/docs/concepts/services-networking/service/#proxy-mode-userspace > > >
Thin Client Kubernetes discovery
Hi, Igniters I'm now discovering the issue https://issues.apache.org/jira/browse/IGNITE-13204 and wondering what is a case that requires a client to be able to discover a cluster? I believe that discovery is a goal of kubernetes itself. We could assign a DNS name for a Service [1] and then any request to the Service will be forwarded to any of Ignite pods behind it. So there is no need to extract pods IP to provide the connection. For example, I've configured a k8s service with name "ignite" and port mapping 10800:9888. Then just connect to it with sqlline by a string "ignite:9888". I think that it's OK that clients should be configured with those settings as they aren't changed during cluster live. Also k8s provides SessionAffinity [2] to guarantee forwarding traffic from one client to the same pod for every request. So there is no need to provide IP for a specific pod if a state is required. Maybe I'm missing something about this feature? Could somebody provide more details for this task? Below is the example config and connection string: apiVersion: v1 kind: Service metadata: name: ignite spec: ports: - port: 9888 targetPort: 10800 selector: app: ignite ./bin/sqlline.sh --verbose=true -u jdbc:ignite:thin://ignite:9888 0: jdbc:ignite:thin://ignite:9888> [1] https://kubernetes.io/docs/concepts/services-networking/service/ [2] https://kubernetes.io/docs/concepts/services-networking/service/#proxy-mode-userspace
Re: Proposal of new event QUERY_EXECUTION_EVENT
Looks like EVT_CACHE_QUERY_EXECUTED just works for different use cases: 1. it relates to a specific cache (Event for SQL queries looks different as it could contain multiple caches or none of them); 2. Also the EVT_CACHE_QUERY_EXECUTED event fires multiple times for distributed queries, see GridMapQueryExecutor class (For SQL query it's required to fire once independently on how many nodes are affected). So there are different patterns for events. I think EVT_CACHE_QUERY_EXECUTED should not be deprecated or changed. > What happens in case the event listener fails? > Would the event notification be synchronous? It depends on how other events are implemented. As I see for the EVT_CACHE_QUERY_EXECUTED event - it's synchronous, and listener errors aren't handled. I think these questions are related to GridEventStorageManager as it just provides an API for recording events. Internal implementations (sync / async, error handling) is not related to an event, is it? > I have some doubts about provide text of a query even with hidden arguments, probably it should be configured due to it could lead to security leak Currently event EVT_CACHE_QUERY_EXECUTED provides a sql clause without limitations. If we're going to provide some restrictions it will require additional investigation. I see at least 2 configurations here: 1. Ignite can be configured to hide clase, params only or nothing for all listeners; 2. Only authorized listeners can subscribe to the event. Should we discuss this within this topic? On Mon, Jul 20, 2020 at 1:55 PM Юрий wrote: > In my opinion existing events EVT_CACHE_QUERY_EXECUTION_STARTED should be > deprecated and added two new for start and for end of queries which should > cover all SQL query types. > I have some doubts about provide text of a query even with hidden > arguments, probably it should be configured due to it could lead to > security leak > > пн, 20 июл. 2020 г. в 12:49, Stanislav Lukyanov : > > > Maksim, > > > > Can we change the EVT_CACHE_QUERY_EXECUTED to fire earlier? Or should > > there be an EVT_CACHE_QUERY_EXECUTION_STARTED for the query start, while > > the old event would continue to work for query finish? > > I think the new event needs to either reuse the old one, or be its mirror > > for the query start. It also means that we probably should resolve the > > issues you've listed. > > > > Would the event notification be synchronous? In which thread? > Asynchronous > > is generally preferred - would it work? > > > > What happens in case the event listener fails? > > > > Thanks, > > Stan > > > > > On 16 Jul 2020, at 18:49, Denis Magda wrote: > > > > > > Taras, Yury, Ivan, > > > > > > Could you please join this thread and share your thoughts? Do we > already > > > have any plans on tracking of the DDL and DML queries? > > > > > > - > > > Denis > > > > > > > > > On Wed, Jul 15, 2020 at 12:09 AM Max Timonin > > > wrote: > > > > > >> Hi Denis, thanks for the answer! > > >> > > >> We already checked EVT_CACHE_QUERY_EXECUTED and found that it works > > only in > > >> cases: > > >> 1. Scan queries and Select queries (common pattern is access to cache > > >> data); > > >> 2. This event triggers only if query execution succeeds, in case of > > failure > > >> while execution this event won't fire. > > >> > > >> Our additional requirements are to protocol queries: > > >> 1. that aren't cache related (for example, alter user); > > >> 2. that relate to multiple caches (while EVT_CACHE_QUERY_EXECUTED have > > >> field cacheName related to specific cache); > > >> 3. we need to protocol also DDL and DML queries. > > >> > > >> Regards, > > >> Maksim > > >> > > >> On Tue, Jul 14, 2020 at 10:20 PM Denis Magda > wrote: > > >> > > >>> Hi Max, > > >>> > > >>> Could you check if the EVT_CACHE_QUERY_EXECUTED event is what you're > > >>> looking for? > > >>> > > >>> > > >> > > > https://www.gridgain.com/docs/latest/developers-guide/events/events#cache-query-events > > >>> > > >>> - > > >>> Denis > > >>> > > >>> > > >>> On Fri, Jul 10, 2020 at 3:54 AM Max Timonin > > > >>> wrote: > > >>> > > >>>> Hi Igniters! > > >>>> > > >>>> We're going to protocol all input SQL queries from our use
Re: Proposal of new event QUERY_EXECUTION_EVENT
Hi Denis, thanks for the answer! We already checked EVT_CACHE_QUERY_EXECUTED and found that it works only in cases: 1. Scan queries and Select queries (common pattern is access to cache data); 2. This event triggers only if query execution succeeds, in case of failure while execution this event won't fire. Our additional requirements are to protocol queries: 1. that aren't cache related (for example, alter user); 2. that relate to multiple caches (while EVT_CACHE_QUERY_EXECUTED have field cacheName related to specific cache); 3. we need to protocol also DDL and DML queries. Regards, Maksim On Tue, Jul 14, 2020 at 10:20 PM Denis Magda wrote: > Hi Max, > > Could you check if the EVT_CACHE_QUERY_EXECUTED event is what you're > looking for? > > https://www.gridgain.com/docs/latest/developers-guide/events/events#cache-query-events > > - > Denis > > > On Fri, Jul 10, 2020 at 3:54 AM Max Timonin > wrote: > > > Hi Igniters! > > > > We're going to protocol all input SQL queries from our users. Currently > > there is no such mechanism in Ignite to use for it. So we're proposing to > > add a new event: QUERY_EXECUITION_EVENT. > > > > Requirements for the event: > > 1. If this event fires it means that a query is correct and will be > > executed (and failed only in exceptional cases); > > > > 2. Event fires for all query types; > > > > 3. Required fields are: > > - text of a query (with hidden arguments); > > - arguments of query; > > - query type; > > - node id. > > > > Looks that this event should go along with `runningQryMgr::register` in > > class `IgniteH2Indexing` as this method invoked for all input queries > too. > > > > What do you think? > > > > Regards, > > Maksim > > >
Proposal of new event QUERY_EXECUTION_EVENT
Hi Igniters! We're going to protocol all input SQL queries from our users. Currently there is no such mechanism in Ignite to use for it. So we're proposing to add a new event: QUERY_EXECUITION_EVENT. Requirements for the event: 1. If this event fires it means that a query is correct and will be executed (and failed only in exceptional cases); 2. Event fires for all query types; 3. Required fields are: - text of a query (with hidden arguments); - arguments of query; - query type; - node id. Looks that this event should go along with `runningQryMgr::register` in class `IgniteH2Indexing` as this method invoked for all input queries too. What do you think? Regards, Maksim
Event for failed queries
Hi Igniters, I'm looking for a technique that can provide SQL clauses for an external observer. There is an event 'CacheQueryExecutedEvent' (maps to EVT_CACHE_QUERY_EXECUTED) that contains the clause. But as I can see it's produced only for success queries and there is no any event if a query fails. I wonder: 1. Is there a reason why it is designed that only success queries produce the event? 2. Can I rely on the event to subscribe to all success queries for my task? 3. Is there a valid way to subscribe on failed queries? Thanks for any help, Maksim
Request for contributors permissions
Hi all, my name is Maksim Timonin, I'm going to contribute to Ignite. Could you please grant contributors permissions to *timonin.maksim *account? Regards, Maksim