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 Thu, 10 Jan 2019 13:45:04 GMT
Dmitrii,

See my comments at issue.
Next time, please leave review request inside the issue.

On Wed, Jan 9, 2019 at 6:39 PM Dmitrii Ryabov <somefireone@gmail.com> wrote:

> Hi,
>
> Anton, can you review ticket for examples [1]?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-6826
>
> пн, 24 дек. 2018 г., 15:23 Anton Vinogradov av@apache.org:
>
> > 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