You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Denis Mekhanikov <dm...@gmail.com> on 2018/08/01 10:22:48 UTC
IP finder in tests
Igniters,
Almost every test in Ignite project has the following pattern: shared
*TcpDiscoveryVmIpFinder
*is defined as a field of a test class, which is then used in discovery
configuration in *getConfiguration(...)* method. There are more than 700
test classes with this setup.
But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by
default. I don't think, that it should be used in tests at all, since
external nodes may accidentally affect test results.
The only case, where it makes sense is multi JVM tests. Shared static IP
finder is not applicable there, since nodes are run in different JVMs and
cannot share the same IP finder object.
I would like to change the default IP finder to a shared
*TcpDiscoveryVmIpFinder.
*In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we could
fall back to multicast.
What do you think?
Denis
Re: IP finder in tests
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
I think it's a sensible decision that is long overdue.
Regards,
--
Ilya Kasnacheev
2018-08-01 13:22 GMT+03:00 Denis Mekhanikov <dm...@gmail.com>:
> Igniters,
>
> Almost every test in Ignite project has the following pattern: shared
> *TcpDiscoveryVmIpFinder
> *is defined as a field of a test class, which is then used in discovery
> configuration in *getConfiguration(...)* method. There are more than 700
> test classes with this setup.
>
> But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by
> default. I don't think, that it should be used in tests at all, since
> external nodes may accidentally affect test results.
>
> The only case, where it makes sense is multi JVM tests. Shared static IP
> finder is not applicable there, since nodes are run in different JVMs and
> cannot share the same IP finder object.
>
> I would like to change the default IP finder to a shared
> *TcpDiscoveryVmIpFinder.
> *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we could
> fall back to multicast.
>
> What do you think?
>
> Denis
>
Re: IP finder in tests
Posted by Yakov Zhdanov <yz...@apache.org>.
Well, I tend to agree. Can you try applying it on some suite and share the
results in terms of run time decrease and decreasing number of random
failures due to improper tests stop or cleanup?
--Yakov
2018-08-02 16:37 GMT+03:00 Denis Mekhanikov <dm...@gmail.com>:
> Yakov,
>
> Almost every test in the project uses the Vm IP finder anyway.
> It has become a convention to use it in all tests.
> So, I'm trying to reduce the amount of copy-pasted code and improve test's
> isolation.
>
> Also tests may be run outside TeamCity during development process.
> Multicast Ip Finder makes developers disconnect from their networks to
> guarantee, that no other nodes will get in the way.
> So, I don't see any advantages of multicast IP finder over Vm in context of
> tests.
>
> Denis
>
> чт, 2 авг. 2018 г. в 1:46, Pavel Kovalenko <jo...@gmail.com>:
>
> > Hi Yakov,
> >
> > Currently TC agents defended by Docker virtual network, that's why we
> don't
> > see intersection between several clusters, but in case of any step aside
> > (running several suites on one agent, running several tests on one
> machine
> > and so on) we will have problems and return back to this conversation.
> > I'm voting for simplifying and speeding up testing process. It will also
> > reduce the number of copy-paste in ton of tests, where Vm Ip Finder is
> used
> > explicitly. As developer I'm confusing when I see in a test VmIpFinder
> and
> > in other test Multicast without any reason or comment.
> > If you care about test coverage of MulticastIpFinder you can pick several
> > suites where number of starting/stopping is most frequent and leave
> > multicast there, but in general it's not necessary to have it everywhere.
> >
> > 2018-08-02 0:54 GMT+03:00 Yakov Zhdanov <yz...@gridgain.com>:
> >
> > > It should be true, otherwise we would have nodes from all agents
> > > intersecting. No?
> > >
> > > And multicast IP finder is the defailt one, so I would not reduce its
> > test
> > > volume.
> > >
> > > Yakov Zhdanov
> > > www.gridgain.com
> > >
> > > 2018-08-02 0:32 GMT+03:00 Dmitriy Pavlov <dp...@gmail.com>:
> > >
> > > > Hi Yakov,
> > > >
> > > > Regarding Each TC agent use own multicast: I'm not sure it is true,
> TC
> > > > admins tried to do so, but not succeded.
> > > >
> > > > One more reason is speed of tests run. Why do we need to scan
> something
> > > if
> > > > we always will connect localhost. TC tests do not use multicast in
> > almost
> > > > every test.
> > > >
> > > > Sincerely,
> > > > Dmitriy Pavlov
> > > >
> > > > чт, 2 авг. 2018 г. в 0:27, Yakov Zhdanov <yz...@apache.org>:
> > > >
> > > > > I disagree. Probably, no change required. Each TC agent use own
> > > multicast
> > > > > group so nodes do not intersect. If any of the test does not
> properly
> > > > clean
> > > > > up and leaves nodes running this dhould be flagged as test fail
> which
> > > is
> > > > > the case.
> > > > >
> > > > > Please provide strong reasons to start with this.
> > > > >
> > > > > --Yakov
> > > > >
> > > >
> > >
> >
>
Re: IP finder in tests
Posted by Denis Mekhanikov <dm...@gmail.com>.
Yakov,
Almost every test in the project uses the Vm IP finder anyway.
It has become a convention to use it in all tests.
So, I'm trying to reduce the amount of copy-pasted code and improve test's
isolation.
Also tests may be run outside TeamCity during development process.
Multicast Ip Finder makes developers disconnect from their networks to
guarantee, that no other nodes will get in the way.
So, I don't see any advantages of multicast IP finder over Vm in context of
tests.
Denis
чт, 2 авг. 2018 г. в 1:46, Pavel Kovalenko <jo...@gmail.com>:
> Hi Yakov,
>
> Currently TC agents defended by Docker virtual network, that's why we don't
> see intersection between several clusters, but in case of any step aside
> (running several suites on one agent, running several tests on one machine
> and so on) we will have problems and return back to this conversation.
> I'm voting for simplifying and speeding up testing process. It will also
> reduce the number of copy-paste in ton of tests, where Vm Ip Finder is used
> explicitly. As developer I'm confusing when I see in a test VmIpFinder and
> in other test Multicast without any reason or comment.
> If you care about test coverage of MulticastIpFinder you can pick several
> suites where number of starting/stopping is most frequent and leave
> multicast there, but in general it's not necessary to have it everywhere.
>
> 2018-08-02 0:54 GMT+03:00 Yakov Zhdanov <yz...@gridgain.com>:
>
> > It should be true, otherwise we would have nodes from all agents
> > intersecting. No?
> >
> > And multicast IP finder is the defailt one, so I would not reduce its
> test
> > volume.
> >
> > Yakov Zhdanov
> > www.gridgain.com
> >
> > 2018-08-02 0:32 GMT+03:00 Dmitriy Pavlov <dp...@gmail.com>:
> >
> > > Hi Yakov,
> > >
> > > Regarding Each TC agent use own multicast: I'm not sure it is true, TC
> > > admins tried to do so, but not succeded.
> > >
> > > One more reason is speed of tests run. Why do we need to scan something
> > if
> > > we always will connect localhost. TC tests do not use multicast in
> almost
> > > every test.
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > > чт, 2 авг. 2018 г. в 0:27, Yakov Zhdanov <yz...@apache.org>:
> > >
> > > > I disagree. Probably, no change required. Each TC agent use own
> > multicast
> > > > group so nodes do not intersect. If any of the test does not properly
> > > clean
> > > > up and leaves nodes running this dhould be flagged as test fail which
> > is
> > > > the case.
> > > >
> > > > Please provide strong reasons to start with this.
> > > >
> > > > --Yakov
> > > >
> > >
> >
>
Re: IP finder in tests
Posted by Pavel Kovalenko <jo...@gmail.com>.
Hi Yakov,
Currently TC agents defended by Docker virtual network, that's why we don't
see intersection between several clusters, but in case of any step aside
(running several suites on one agent, running several tests on one machine
and so on) we will have problems and return back to this conversation.
I'm voting for simplifying and speeding up testing process. It will also
reduce the number of copy-paste in ton of tests, where Vm Ip Finder is used
explicitly. As developer I'm confusing when I see in a test VmIpFinder and
in other test Multicast without any reason or comment.
If you care about test coverage of MulticastIpFinder you can pick several
suites where number of starting/stopping is most frequent and leave
multicast there, but in general it's not necessary to have it everywhere.
2018-08-02 0:54 GMT+03:00 Yakov Zhdanov <yz...@gridgain.com>:
> It should be true, otherwise we would have nodes from all agents
> intersecting. No?
>
> And multicast IP finder is the defailt one, so I would not reduce its test
> volume.
>
> Yakov Zhdanov
> www.gridgain.com
>
> 2018-08-02 0:32 GMT+03:00 Dmitriy Pavlov <dp...@gmail.com>:
>
> > Hi Yakov,
> >
> > Regarding Each TC agent use own multicast: I'm not sure it is true, TC
> > admins tried to do so, but not succeded.
> >
> > One more reason is speed of tests run. Why do we need to scan something
> if
> > we always will connect localhost. TC tests do not use multicast in almost
> > every test.
> >
> > Sincerely,
> > Dmitriy Pavlov
> >
> > чт, 2 авг. 2018 г. в 0:27, Yakov Zhdanov <yz...@apache.org>:
> >
> > > I disagree. Probably, no change required. Each TC agent use own
> multicast
> > > group so nodes do not intersect. If any of the test does not properly
> > clean
> > > up and leaves nodes running this dhould be flagged as test fail which
> is
> > > the case.
> > >
> > > Please provide strong reasons to start with this.
> > >
> > > --Yakov
> > >
> >
>
Re: IP finder in tests
Posted by Yakov Zhdanov <yz...@gridgain.com>.
It should be true, otherwise we would have nodes from all agents
intersecting. No?
And multicast IP finder is the defailt one, so I would not reduce its test
volume.
Yakov Zhdanov
www.gridgain.com
2018-08-02 0:32 GMT+03:00 Dmitriy Pavlov <dp...@gmail.com>:
> Hi Yakov,
>
> Regarding Each TC agent use own multicast: I'm not sure it is true, TC
> admins tried to do so, but not succeded.
>
> One more reason is speed of tests run. Why do we need to scan something if
> we always will connect localhost. TC tests do not use multicast in almost
> every test.
>
> Sincerely,
> Dmitriy Pavlov
>
> чт, 2 авг. 2018 г. в 0:27, Yakov Zhdanov <yz...@apache.org>:
>
> > I disagree. Probably, no change required. Each TC agent use own multicast
> > group so nodes do not intersect. If any of the test does not properly
> clean
> > up and leaves nodes running this dhould be flagged as test fail which is
> > the case.
> >
> > Please provide strong reasons to start with this.
> >
> > --Yakov
> >
>
Re: IP finder in tests
Posted by Dmitriy Pavlov <dp...@gmail.com>.
Hi Yakov,
Regarding Each TC agent use own multicast: I'm not sure it is true, TC
admins tried to do so, but not succeded.
One more reason is speed of tests run. Why do we need to scan something if
we always will connect localhost. TC tests do not use multicast in almost
every test.
Sincerely,
Dmitriy Pavlov
чт, 2 авг. 2018 г. в 0:27, Yakov Zhdanov <yz...@apache.org>:
> I disagree. Probably, no change required. Each TC agent use own multicast
> group so nodes do not intersect. If any of the test does not properly clean
> up and leaves nodes running this dhould be flagged as test fail which is
> the case.
>
> Please provide strong reasons to start with this.
>
> --Yakov
>
Re: IP finder in tests
Posted by Yakov Zhdanov <yz...@apache.org>.
I disagree. Probably, no change required. Each TC agent use own multicast
group so nodes do not intersect. If any of the test does not properly clean
up and leaves nodes running this dhould be flagged as test fail which is
the case.
Please provide strong reasons to start with this.
--Yakov
Re: IP finder in tests
Posted by Dmitriy Pavlov <dp...@gmail.com>.
Hi Dmitriy,
I guess this ticket relates to examples, but new proposal is related only
to test.
Community didn't make an agreement about examples change, so I suggest to
create new ticket (and probably link both tickets) and PR.
Sincerely,
Dmitriy Pavlov
ср, 1 авг. 2018 г. в 20:30, Dmitrii Ryabov <so...@gmail.com>:
> We have ticket [1] for this change.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-6826
>
> 2018-08-01 15:58 GMT+03:00 Alexey Zinoviev <za...@gmail.com>:
>
> > Great, sometimes I made it manually in local tests. It will be great to
> > change the situation.
> >
> > 2018-08-01 18:22 GMT+06:00 Stanislav Lukyanov <st...@gmail.com>:
> >
> > > +1.
> > >
> > > I don’t see why do we need to fallback to multicast for multi-JVM –
> let’s
> > > just set 127.0.0.1:47500..47509 by default,
> > > it’ll be enough for most (if not all) tests.
> > >
> > > Stan
> > >
> > > From: Dmitriy Pavlov
> > > Sent: 1 августа 2018 г. 14:21
> > > To: dev@ignite.apache.org
> > > Subject: Re: IP finder in tests
> > >
> > > Hi Denis,
> > >
> > > Thank you for bringing the question here.
> > >
> > > I totally support this change.
> > >
> > > Sincerely,
> > > Dmitriy Pavlov
> > >
> > > ср, 1 авг. 2018 г. в 13:23, Denis Mekhanikov <dm...@gmail.com>:
> > >
> > > > Igniters,
> > > >
> > > > Almost every test in Ignite project has the following pattern: shared
> > > > *TcpDiscoveryVmIpFinder
> > > > *is defined as a field of a test class, which is then used in
> discovery
> > > > configuration in *getConfiguration(...)* method. There are more than
> > 700
> > > > test classes with this setup.
> > > >
> > > > But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests
> by
> > > > default. I don't think, that it should be used in tests at all, since
> > > > external nodes may accidentally affect test results.
> > > >
> > > > The only case, where it makes sense is multi JVM tests. Shared static
> > IP
> > > > finder is not applicable there, since nodes are run in different JVMs
> > and
> > > > cannot share the same IP finder object.
> > > >
> > > > I would like to change the default IP finder to a shared
> > > > *TcpDiscoveryVmIpFinder.
> > > > *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we
> > could
> > > > fall back to multicast.
> > > >
> > > > What do you think?
> > > >
> > > > Denis
> > > >
> > >
> > >
> >
>
Re: IP finder in tests
Posted by Dmitrii Ryabov <so...@gmail.com>.
We have ticket [1] for this change.
[1] https://issues.apache.org/jira/browse/IGNITE-6826
2018-08-01 15:58 GMT+03:00 Alexey Zinoviev <za...@gmail.com>:
> Great, sometimes I made it manually in local tests. It will be great to
> change the situation.
>
> 2018-08-01 18:22 GMT+06:00 Stanislav Lukyanov <st...@gmail.com>:
>
> > +1.
> >
> > I don’t see why do we need to fallback to multicast for multi-JVM – let’s
> > just set 127.0.0.1:47500..47509 by default,
> > it’ll be enough for most (if not all) tests.
> >
> > Stan
> >
> > From: Dmitriy Pavlov
> > Sent: 1 августа 2018 г. 14:21
> > To: dev@ignite.apache.org
> > Subject: Re: IP finder in tests
> >
> > Hi Denis,
> >
> > Thank you for bringing the question here.
> >
> > I totally support this change.
> >
> > Sincerely,
> > Dmitriy Pavlov
> >
> > ср, 1 авг. 2018 г. в 13:23, Denis Mekhanikov <dm...@gmail.com>:
> >
> > > Igniters,
> > >
> > > Almost every test in Ignite project has the following pattern: shared
> > > *TcpDiscoveryVmIpFinder
> > > *is defined as a field of a test class, which is then used in discovery
> > > configuration in *getConfiguration(...)* method. There are more than
> 700
> > > test classes with this setup.
> > >
> > > But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by
> > > default. I don't think, that it should be used in tests at all, since
> > > external nodes may accidentally affect test results.
> > >
> > > The only case, where it makes sense is multi JVM tests. Shared static
> IP
> > > finder is not applicable there, since nodes are run in different JVMs
> and
> > > cannot share the same IP finder object.
> > >
> > > I would like to change the default IP finder to a shared
> > > *TcpDiscoveryVmIpFinder.
> > > *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we
> could
> > > fall back to multicast.
> > >
> > > What do you think?
> > >
> > > Denis
> > >
> >
> >
>
Re: IP finder in tests
Posted by Alexey Zinoviev <za...@gmail.com>.
Great, sometimes I made it manually in local tests. It will be great to
change the situation.
2018-08-01 18:22 GMT+06:00 Stanislav Lukyanov <st...@gmail.com>:
> +1.
>
> I don’t see why do we need to fallback to multicast for multi-JVM – let’s
> just set 127.0.0.1:47500..47509 by default,
> it’ll be enough for most (if not all) tests.
>
> Stan
>
> From: Dmitriy Pavlov
> Sent: 1 августа 2018 г. 14:21
> To: dev@ignite.apache.org
> Subject: Re: IP finder in tests
>
> Hi Denis,
>
> Thank you for bringing the question here.
>
> I totally support this change.
>
> Sincerely,
> Dmitriy Pavlov
>
> ср, 1 авг. 2018 г. в 13:23, Denis Mekhanikov <dm...@gmail.com>:
>
> > Igniters,
> >
> > Almost every test in Ignite project has the following pattern: shared
> > *TcpDiscoveryVmIpFinder
> > *is defined as a field of a test class, which is then used in discovery
> > configuration in *getConfiguration(...)* method. There are more than 700
> > test classes with this setup.
> >
> > But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by
> > default. I don't think, that it should be used in tests at all, since
> > external nodes may accidentally affect test results.
> >
> > The only case, where it makes sense is multi JVM tests. Shared static IP
> > finder is not applicable there, since nodes are run in different JVMs and
> > cannot share the same IP finder object.
> >
> > I would like to change the default IP finder to a shared
> > *TcpDiscoveryVmIpFinder.
> > *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we could
> > fall back to multicast.
> >
> > What do you think?
> >
> > Denis
> >
>
>
RE: IP finder in tests
Posted by Stanislav Lukyanov <st...@gmail.com>.
+1.
I don’t see why do we need to fallback to multicast for multi-JVM – let’s just set 127.0.0.1:47500..47509 by default,
it’ll be enough for most (if not all) tests.
Stan
From: Dmitriy Pavlov
Sent: 1 августа 2018 г. 14:21
To: dev@ignite.apache.org
Subject: Re: IP finder in tests
Hi Denis,
Thank you for bringing the question here.
I totally support this change.
Sincerely,
Dmitriy Pavlov
ср, 1 авг. 2018 г. в 13:23, Denis Mekhanikov <dm...@gmail.com>:
> Igniters,
>
> Almost every test in Ignite project has the following pattern: shared
> *TcpDiscoveryVmIpFinder
> *is defined as a field of a test class, which is then used in discovery
> configuration in *getConfiguration(...)* method. There are more than 700
> test classes with this setup.
>
> But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by
> default. I don't think, that it should be used in tests at all, since
> external nodes may accidentally affect test results.
>
> The only case, where it makes sense is multi JVM tests. Shared static IP
> finder is not applicable there, since nodes are run in different JVMs and
> cannot share the same IP finder object.
>
> I would like to change the default IP finder to a shared
> *TcpDiscoveryVmIpFinder.
> *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we could
> fall back to multicast.
>
> What do you think?
>
> Denis
>
Re: IP finder in tests
Posted by Pavel Kovalenko <jo...@gmail.com>.
Hi Denis,
I also definitely support this change. As an engineer who tightly working
with MakeTeamCityGreenAgain activity, I notice several test suites hanging
related with MulticastIpFinder infinite loops on datagram receiving. Here
is last recorded fail -
https://ci.ignite.apache.org/viewLog.html?buildId=1544659&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_BinaryObjectsSimpleMapperBasic
.
Changing default ipFinder in tests to VM will help with TeamCity stability.
2018-08-01 14:20 GMT+03:00 Dmitriy Pavlov <dp...@gmail.com>:
> Hi Denis,
>
> Thank you for bringing the question here.
>
> I totally support this change.
>
> Sincerely,
> Dmitriy Pavlov
>
> ср, 1 авг. 2018 г. в 13:23, Denis Mekhanikov <dm...@gmail.com>:
>
> > Igniters,
> >
> > Almost every test in Ignite project has the following pattern: shared
> > *TcpDiscoveryVmIpFinder
> > *is defined as a field of a test class, which is then used in discovery
> > configuration in *getConfiguration(...)* method. There are more than 700
> > test classes with this setup.
> >
> > But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by
> > default. I don't think, that it should be used in tests at all, since
> > external nodes may accidentally affect test results.
> >
> > The only case, where it makes sense is multi JVM tests. Shared static IP
> > finder is not applicable there, since nodes are run in different JVMs and
> > cannot share the same IP finder object.
> >
> > I would like to change the default IP finder to a shared
> > *TcpDiscoveryVmIpFinder.
> > *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we could
> > fall back to multicast.
> >
> > What do you think?
> >
> > Denis
> >
>
Re: IP finder in tests
Posted by Dmitriy Pavlov <dp...@gmail.com>.
Hi Denis,
Thank you for bringing the question here.
I totally support this change.
Sincerely,
Dmitriy Pavlov
ср, 1 авг. 2018 г. в 13:23, Denis Mekhanikov <dm...@gmail.com>:
> Igniters,
>
> Almost every test in Ignite project has the following pattern: shared
> *TcpDiscoveryVmIpFinder
> *is defined as a field of a test class, which is then used in discovery
> configuration in *getConfiguration(...)* method. There are more than 700
> test classes with this setup.
>
> But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by
> default. I don't think, that it should be used in tests at all, since
> external nodes may accidentally affect test results.
>
> The only case, where it makes sense is multi JVM tests. Shared static IP
> finder is not applicable there, since nodes are run in different JVMs and
> cannot share the same IP finder object.
>
> I would like to change the default IP finder to a shared
> *TcpDiscoveryVmIpFinder.
> *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we could
> fall back to multicast.
>
> What do you think?
>
> Denis
>