ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov ...@apache.org>
Subject Re: Set 'TcpDiscoveryVmIpFinder' as default IP finder for tests instead of 'TcpDiscoveryMulticastIpFinder'
Date Mon, 24 Dec 2018 12:23:48 GMT
Folks,

The important thing here is that 99% of "final static" ip-finders were
removed. (~800 pcs.)
Non-static, mostly, kept as is since removal may or cause a test failure.
(~130 pcs.)

In case someone interested in the continuation of cleanup, please feel free
to create an issue and provide PR, I'm ready to review it.

On Mon, Dec 24, 2018 at 3:04 PM Vyacheslav Daradur <daradurvs@gmail.com>
wrote:

> The second step [2] "removing related boilerplate in tests" - has been
> done. It is expected that a bit memory will be released at tests TC
> agents, which could not be cleaned by GC because of static final
> fields.
>
> Guys, please, do not generate a new one )
>
> [1] https://issues.apache.org/jira/browse/IGNITE-10715
>
> On Tue, Dec 18, 2018 at 6:29 PM Anton Vinogradov <av@apache.org> wrote:
> >
> > Folks,
> >
> > Now I see 5-10% speedup at all TC suites right after the merge.
> > Great fix!
> >
> >
> > On Mon, Dec 17, 2018 at 2:39 PM Vyacheslav Daradur <daradurvs@gmail.com>
> > wrote:
> >
> > > Andrey Mashenkov, at first sight, I have not seen any problems with
> > > .NET platform.
> > >
> > > I believe we need carefully configure ports in platform's examples,
> > > additional actions should not be required.
> > >
> > > On Mon, Dec 17, 2018 at 2:35 PM Vyacheslav Daradur <
> daradurvs@gmail.com>
> > > wrote:
> > > >
> > > > The task [1] is done.
> > > >
> > > > 'TcpDiscoveryVmIpFinder' is default IP finder in tests now.
> > > >
> > > > The IP finder is initialized on per tests class level to avoid hidden
> > > > affecting among tests, it means the test methods in the common test
> > > > class will use the same IP finder.
> > > >
> > > > You don't need to set up 'TcpDiscoveryVmIpFinder' in your tests
> > > > explicitly anymore, also task [2] has been filled to remove related
> > > > boilerplate if nobody minds.
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-10555
> > > > [2] https://issues.apache.org/jira/browse/IGNITE-10715
> > > >
> > > >
> > > > On Wed, Dec 5, 2018 at 7:17 PM Dmitriy Pavlov <dpavlov@apache.org>
> > > wrote:
> > > > >
> > > > > ++1
> > > > >
> > > > > ср, 5 дек. 2018 г. в 18:38, Denis Mekhanikov <
> dmekhanikov@gmail.com>:
> > > > >
> > > > > > Andrey,
> > > > > >
> > > > > > Multi-JVM tests may also use a static IP finder, but it should
> use
> > > some
> > > > > > specific port range instead of being shared.
> > > > > > Something like 127.0.0.1:48500..48509 would do.
> > > > > >
> > > > > > Denis
> > > > > >
> > > > > > ср, 5 дек. 2018 г. в 18:34, Vyacheslav Daradur <
> daradurvs@gmail.com
> > > >:
> > > > > >
> > > > > > > I filled a task [1].
> > > > > > >
> > > > > > > >> Slava, do you think Platforms tests can be fixed
as well or
> one
> > > more
> > > > > > > ticket
> > > > > > > should be created?
> > > > > > >
> > > > > > > I'll try to fix them within one ticket, it should be
> investigated
> > > a bit
> > > > > > > deeper.
> > > > > > >
> > > > > > > I'll inform about the task's progress in this thread later.
> > > > > > >
> > > > > > > Thanks!
> > > > > > >
> > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10555
> > > > > > > On Wed, Dec 5, 2018 at 6:28 PM Andrey Mashenkov
> > > > > > > <andrey.mashenkov@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Slava,
> > > > > > > > +1 for your proposal.
> > > > > > > > Is there any ticket for this?
> > > > > > > >
> > > > > > > > Denis,
> > > > > > > > I've just read in nabble thread you suggest to allow
> multicast
> > > finder
> > > > > > for
> > > > > > > > multiJVM tests
> > > > > > > > and I'd think we shouldn't use multicast in test at
all
> (excepts
> > > > > > > multicast
> > > > > > > > Ip finder self tests of course),
> > > > > > > > but e.g. add an assertion to force user to create
ipfinder
> > > properly.
> > > > > > > >
> > > > > > > >
> > > > > > > > Also, we have a ticket for similar issue in 'examples'
> module.
> > > > > > > > Seems, there are some issues with Platforms module
> integration.
> > > > > > > > Slava, do you think Platforms tests can be fixed as
well or
> one
> > > more
> > > > > > > ticket
> > > > > > > > should be created?
> > > > > > > >
> > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-6826
> > > > > > > >
> > > > > > > > On Wed, Dec 5, 2018 at 5:55 PM Denis Mekhanikov <
> > > dmekhanikov@gmail.com
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Slava,
> > > > > > > > >
> > > > > > > > > These are exactly my thoughts, so I fully support
you here.
> > > > > > > > > I already wrote about it:
> > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > >
> http://apache-ignite-developers.2346864.n4.nabble.com/IP-finder-in-tests-td33322.html
> > > > > > > > > But I kind of abandoned this activity. Feel free
to take
> over
> > > it.
> > > > > > > > >
> > > > > > > > > Denis
> > > > > > > > >
> > > > > > > > > ср, 5 дек. 2018 г. в 17:22, Vladimir Ozerov
<
> > > vozerov@gridgain.com>:
> > > > > > > > >
> > > > > > > > > > Huge +1
> > > > > > > > > >
> > > > > > > > > > On Wed, Dec 5, 2018 at 5:09 PM Vyacheslav
Daradur <
> > > > > > > daradurvs@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Igniters,
> > > > > > > > > > >
> > > > > > > > > > > I've found that the project's test
framework uses
> > > > > > > > > > > 'TcpDiscoveryMulticastIpFinder' as
default IP finder
> for
> > > tests
> > > > > > and
> > > > > > > > > > > there are a lot of tests written by
Ignite's experts
> that
> > > > > > override
> > > > > > > it
> > > > > > > > > > > to 'TcpDiscoveryVmIpFinder'.
> > > > > > > > > > >
> > > > > > > > > > > Most of our tests starting Ignite nodes
in the same
> JVM,
> > > that
> > > > > > > allows
> > > > > > > > > > > us using shared 'TcpDiscoveryVmIpFinder'.
> > > > > > > > > > >
> > > > > > > > > > > I think that using of 'TcpDiscoveryMulticastIpFinder'
> may
> > > be
> > > > > > useful
> > > > > > > > > > > only in platforms tests, BTW multi-JVM
tests use the
> tuned
> > > > > > > > > > > 'TcpDiscoveryVmIpFinder'.
> > > > > > > > > > >
> > > > > > > > > > > I see the following main advantages
of using
> > > > > > > 'TcpDiscoveryVmIpFinder':
> > > > > > > > > > > * reducing possible conflicts in the
development
> > > environment,
> > > > > > when
> > > > > > > > > > > nodes from different clusters may find
each other;
> > > > > > > > > > > * speedup of nodes initial discovery,
especially on
> > > Windows;
> > > > > > > > > > > * avoiding of overwriting 'getConfiguration'
and
> copypasta
> > > only
> > > > > > to
> > > > > > > set
> > > > > > > > > > > up static IP finder in tests;
> > > > > > > > > > >
> > > > > > > > > > > So, I'd suggest changing the default
IP finder in
> tests to
> > > > > > > > > > > 'TcpDiscoveryVmIpFinder' as the first
step and remove
> > > related
> > > > > > > > > > > boilerplate as the second step.
> > > > > > > > > > >
> > > > > > > > > > > What do you think?
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Best Regards, Vyacheslav D.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Best regards,
> > > > > > > > Andrey V. Mashenkov
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best Regards, Vyacheslav D.
> > > > > > >
> > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards, Vyacheslav D.
> > >
> > >
> > >
> > > --
> > > Best Regards, Vyacheslav D.
> > >
>
>
>
> --
> Best Regards, Vyacheslav D.
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message