Re: Code inspection

[Petr Ivanov]

Investigating problem, stand by.


> On 18 Dec 2018, at 19:41, Dmitriy Pavlov  wrote:
> 
> Both patches were applied. Maxim, thank you!
> 
> What about 1. An `Unexpected error during build messages processing in
> TeamCity`, what can we do as the next step to fix it?
> 
> Sincerely,
> Dmitriy Pavlov
> 
> пн, 17 дек. 2018 г. в 18:31, Andrey Mashenkov :
> 
>> Maxim,
>> 
>> Looks ok. Let's apply IGNITE-10682.
>> 
>> All,
>> 
>> Also, I'd like to publish idea.logs into artefacts by default.
>> This will give us more details for investigation in future if any failure
>> will occurs.
>> It will costs 1-10 kB.
>> 
>> On Mon, Dec 17, 2018 at 3:21 PM Maxim Muzafarov 
>> wrote:
>> 
>>> Dmitry,
>>> 
>>> It seems to me that we have two independent issues here.
>>> 1. An `Unexpected error during build messages processing in TeamCity`
>>> error message which is related to TC agent configuration. Suppose,
>>> server.log will provide us more details about it. I have to access
>>> there.
>>> 2. A new set of inspection rules was introduced in 2018+ IntelliJ IDEA
>>> and they should be disabled in our ignite_inspections_teamcity.xml
>>> configuration file. They are not fixed in the Apache Ignite project
>>> code yet. I've prepared the issue [1] for it. Please, take a look.
>>> 
>>> 
>>> Andrey,
>>> 
>>> I've fixed disabled plugins file according to your suggestions. The
>>> issue [2] is ready. I've re-run `Excluded [Inspections] Core Debug`
>>> suite and the log details show me that now only 3 plugins are enabled:
>>> IDEA CORE, Maven Integration, Properties Support. It seems to me that
>>> it's correct.
>>> 
>>> [1] https://issues.apache.org/jira/browse/IGNITE-10709
>>> [2] https://issues.apache.org/jira/browse/IGNITE-10682
>>> 
>>> On Sat, 15 Dec 2018 at 15:22, Dmitriy Pavlov  wrote:
 
 Folks,
 
 There is a strange error on TC
 
>>> 
>> https://ci.ignite.apache.org/viewLog.html?buildId=2556875&buildTypeId=IgniteTests24Java8_InspectionsCore
 
 It appeared after TC update to the latest version.
 
 Sincerely,
 Dmitry Pavlov
 
 пт, 14 дек. 2018 г. в 16:09, Andrey Mashenkov <
>>> andrey.mashen...@gmail.com>:
 
> Maxim,
> 
> PR is incomplete. Some plugins should be disabled with different
>>> id\name.
> Maven plugin shouldn't be disabled as Idea Inspector use it to use
>>> Ignite
> project pom file.
> 
> Please, find details in ticket.
> 
> 
> On Fri, Dec 14, 2018 at 12:00 PM Andrey Mashenkov <
> andrey.mashen...@gmail.com> wrote:
> 
>> Maxim,
>> 
>> Thanks, I'll check PR and let you know about results.
>> 
>> For now, Inspections task execution time looks much better (15-22
>>> min),
>> but fluctuation is still noticeable.
>> 
>> On Fri, Dec 14, 2018 at 11:13 AM Maxim Muzafarov <
>> maxmu...@gmail.com
 
>> wrote:
>> 
>>> Andrey,
>>> 
>>> Thanks! I've consulted with the IntelliJ IDEA source code and
>> found
>>> how this disabled plugins file should look like. I've created a
>> new
>>> issue [1] and prepared PR [2] with the set of disabled plugins
>>> (maybe
>>> not complete set). I don't have access to change corresponding
>>> `~Excluded [Inspections] Core Debug` test suite properties.
>>> Can we test this PR?
>>> 
>>> [1] https://issues.apache.org/jira/browse/IGNITE-10682
>>> [2] https://github.com/apache/ignite/pull/5666
>>> On Thu, 13 Dec 2018 at 17:35, Andrey Mashenkov
>>>  wrote:
 
 Maxim,
 
 Idea has a file in config directory
>> ./config/disabled_plugins.txt
>>> ,
> you
>>> can easily find it at you local machine.
 Teamcity Inspections runner has an option "Disabled plugins"
>> where
>>> disabled_plugins.txt file content can be set.
 
 So, looks like we can disable useless plugins.
 But I'm not expert and can't suggest changes we can safely
>> apply.
 
 On Thu, Dec 13, 2018 at 4:59 PM Maxim Muzafarov <
>>> maxmu...@gmail.com>
>>> wrote:
> 
> Andrey,
> 
> Thank you for solving this issue with GC pauses! I've checked
>> the
> given report. The inspections configuration is correct, but it
>>> seems
> to me that we have enabled by default rules of included plugins
>>> (for
> instance, KotlinInternalInJava in the report is enabled).
> 
> Can you share more details about `disable plugin` option you
>>> found?
> 
> I see that idea instance starts with the default
>>> -Didea.plugins.path
> system property, can we change it so the plugins will be not
>>> loaded
> by
> default?
> On Thu, 13 Dec 2018 at 15:45, Andrey Mashenkov
>  wrote:
>> 
>> Maxim,
>> 
>> It looks like we can't make logs more verbose due to possible
>>> bug,
>>> I've create a ticket in Jetbrains Jira [1].
>> We 

Re: Async debugging in IDEA

[Ivan Bessonov]

Hi guys,

First of all, considering cons: I couldn't find a way to see the whole
stacktrace from worker in "Frames" window. Maybe this'll be fixed in
future versions of IDEA. Workaround it to use "Threads" tab of the
debugger. It's not that comfortable but does its job pretty well.

> Are there any chances to keep variable's values at original stacktrace?

No, runtime overhead would be way too big. The only stored information
is pretty much the array of "StackTraceElement" objects.

> Will this work correctly in case of concurrent calls?
for example, when more that one stacktrace will cause same async op.

I believe that if you actually submit the same object twice as a listener
from two different places in code then debugger agent will process it
incorrectly (it'll show the latest submission point). Everything else should
work fine.
The best explanation of the internal mechanisms is the source code,
you can always use it as a reference [1].

[1]
https://github.com/JetBrains/intellij-community/blob/master/java/debugger/debugger-agent-storage/src/com/intellij/rt/debugger/agent/CaptureStorage.java

вт, 18 дек. 2018 г. в 20:13, Anton Vinogradov :

> Folks,
>
> That's extremely awesome!
> Now I see what stacktrace cause future creation and who will gain the
> result and how.
>
> Are there any chances to keep variable's values at original stacktrace?
>
> Will this work correctly in case of concurrent calls?
> for example, when more that one stacktrace will cause same async op.
> Will this feature guarantee that I see exactly that what caused this async,
> not one of possible?
>
> Cons.
> Is there any chances now to see the whole stacktrace starting from worker
> got a message from the queue?
>
> On Fri, Dec 14, 2018 at 6:53 PM Dmitriy Pavlov  wrote:
>
> > Looks cool, I will try
> >
> > пт, 14 дек. 2018 г. в 16:46, Павлухин Иван :
> >
> > > Hi Igniters,
> > >
> > > I would like to share with you that now it is possible to use IDEA
> > > async debugger for debugging Ignite futures. And we should thank Ivan
> > > Bessonov for that awesome contribution [1].
> > >
> > > Most of your might be already familiar with async debugger in IDEA.
> > > For the rest I prepared two screenshots [2], [3]. We can see regular
> > > stacktrace on breakpoint hit in future completion callback which is
> > > extended with an additional stacktrace showing a path where the
> > > callback was installed.
> > >
> > > Thank you Ivan!
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-10475
> > > [2]
> > >
> >
> https://gist.githubusercontent.com/pavlukhin/c8c7c6266eeab56048c31f5cdfb31d20/raw/bfe32b8c11a67ccc3bcbbb8b398e39a8a818cad6/adebug1.png
> > > [3]
> > >
> >
> https://gist.githubusercontent.com/pavlukhin/c8c7c6266eeab56048c31f5cdfb31d20/raw/bfe32b8c11a67ccc3bcbbb8b398e39a8a818cad6/adebug2.png
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
> > >
> >
>


-- 
Sincerely yours,
Ivan Bessonov

Re: Is it time to move forward to JUnit4 (5)?

[Павлухин Иван]

Hi Oleg,

I have not quite understood who is going to call (from standpoint of
test framework) beforeTestsStarted, beforeTest, afterTest,
afterTestsStarted methods?
вт, 18 дек. 2018 г. в 23:31, oignatenko :
>
> Hi Ivan,
>
> To answer your last question, yes, all the tests are to be marked with @Test
> annotations, and those that are meant to be ignored are going to be marked
> with @Ignore annotations. There is no exceptions to that, and these
> annotations will work just as well on tests using our home brewed
> beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
>
> For the sake of completeness, developers will also be able to use Before* /
> After* annotations on their own methods in tests. The only exception
> (clarified in respective javadocs) is that these annotations shouldn't be
> used on overrides of our home brewed methods - and these methods, in
> addition, will be recommended (not mandated) to avoid wia deprecation
> notices.
>
> -
>
> As for accessing tests which have problems running under junit4, the way how
> I search for these in current master is regex search in IDEA for
> "addTestSuite.*class", that is I search in testsuites for entries that are
> added using method "addTestSuite" with parameter class.
>
> Probably worth noting that some of the problems that were previously
> blocking addition of particular tests have been resolved in the course of
> working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One
> riddle that currently looks particularly difficult to crack is Teamcity
> failures in "Queries 1", I even haven't yet figured how to reproduce these
> locally.
>
> regards, Oleg
>
>
> Павлухин Иван wrote
> > Hi Oleg,
> >
> > Now concerns about junit3 elimination are clear for me. And I agree
> > that it is worth to do it. By the way is it possible to access tests
> > which have problems running under junit4? I would like to take a look.
> >
> > Also a clarifying bit regarding next migration steps is needed. Sorry
> > if it was described but I missed it. Currently we have tons of tests
> > which rely on our home brewed beforeTestsStarted, beforeTest,
> > afterTest, afterTestsStarted. Are you going to mark them all with
> > corresponding junit4 annotations?
> > пн, 17 дек. 2018 г. в 19:13, oignatenko <
>
> > oignatenko@
>
> > >:
> >>
> >> Hi Ivan,
> >>
> >> Per my cursory check of the code currently in master, we have 239 test
> >> classes that couldn't migrate (that's probably about 500 or something
> >> test
> >> cases). For comparison, over 1600 classes migrated without problems. This
> >> is
> >> to answer your questions about how many tests are affected by change and
> >> how many tests were not migrated yet.
> >>
> >> -
> >>
> >> I think we can say that only small percent of tests turned out sensitive
> >> to
> >> JUnit framework change.
> >>
> >> In the same time, due to very large overall amount of our tests we have
> >> quite a big number as an absolute value. I point this because if amount
> >> of
> >> troublesome tests was smaller (say, order of magnitude smaller) I would
> >> consider delaying migration until all tests reworked to blend totally
> >> seamlessly into new version JUnit. This is doable - as sort of "pilot
> >> exercise" me and Ed adjusted one test case this way - but the sheer
> >> amount
> >> of work on 200+ classes raises a question, whether it is worth it (I
> >> think
> >> it isn't).
> >>
> >> With regards to question 2, changes to TestCounters are farly trivial
> >> (and
> >> necessary) if you look at the code diff. Prior code counted amount of
> >> test
> >> cases in the class by JUnit 3 convention: it picked public void methods
> >> without parameters starting with "test". Changed code counts test cases
> >> as
> >> JUnit 4 does: by checking if method is annotated with @Test. Change is
> >> necessary because it will allow test developers fully use JUnit 4
> >> features
> >> by adding test cases that don't follow old naming requirement. I
> >> experimented with it a little and as far as I could see the old
> >> TestCounters
> >> indeed break when there are methods following new convention, it
> >> triggered
> >> afterTestsStopped too early.
> >>
> >> The answer to your question 3 lies in javadocs I added to runSerializer
> >> declaration, or, more precisely, in TestCounters javadoc referred from
> >> there. As you can see, current plan is to get rid of TestCounters fairly
> >> soon (per IGNITE-10179) and this will also get rid of runSerializer so
> >> this
> >> is merely a temporary band aid to be removed soon.
> >>
> >> For the sake of completeness, my initial plan was to thoroughly
> >> investigate
> >> and test whether change from JUnit 3 to JUnit 4 would keep accessing
> >> counters safe or not and only introduce runSerializer if it really does
> >> but
> >> after realising that this whole thing is likely to go away in a few days
> >> from now I decided that it isn't worth wasting time and just temporarily
> >> made i

[jira] [Created] (IGNITE-10737) Enable inspection Collections.sort(..) can be replaced with list.sort(..)

[Maxim Muzafarov (JIRA)]

Maxim Muzafarov created IGNITE-10737:


 Summary: Enable inspection Collections.sort(..) can be replaced 
with list.sort(..)
 Key: IGNITE-10737
 URL: https://issues.apache.org/jira/browse/IGNITE-10737
 Project: Ignite
  Issue Type: Task
Reporter: Maxim Muzafarov
Assignee: Maxim Muzafarov
 Fix For: 2.8


We need to enable inspection which reports calls to Collections.sort(list, 
comparator) which could be replaced with list.sort(comparator).

# Update ignite_inspections_teamcity.xml configuration file by enabling current 
inspection
# Fix all the code issues.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Critical worker threads liveness checking drawbacks

[Alexey Goncharuk]

Folks, why did not we include IGNITE-10003 to ignite-2.7 release scope?
This causes an Ignite node to be stopped by default when checkpoint read
lock acquire times out. I expect a lot of Ignite 2.7 users will be affected
by this mistake.

We should at least update the documentation and make users aware of a
workaround.

чт, 25 окт. 2018 г. в 16:35, Alexey Goncharuk :

> Andrey,
>
> I still see that checkpoint read lock acquisition raises a CRITICAL_ERROR,
> which by default will shut down local node. As far as I remember, we
> decided that by default thread timeout should not trigger node failure.
> Now, however, it does, because we ignore SYSTEM_WORKER_BLOCKED events in
> default configuration.
>
> Should we introduce another critical failure type
> CHECKPOINT_READ_LOCK_BLOCKED or use SYSTEM_WORKER_BLOCKED for checkpoint
> read lock acquire failure?
>
> --AG
>
> пт, 12 окт. 2018 г. в 8:29, Andrey Kuznetsov :
>
>> Igniters,
>>
>> Now I spot blocking / long-running code arising from
>> {{GridDhtPartitionsExchangeFuture#init}} calls in partition-exchanger
>> thread, see [1]. Ideally, all blocking operations along all possible code
>> paths should be guarded implicitly from critical failure detector to avoid
>> the thread from being considered blocked. There is a pull request [2] that
>> provides shallow solution. I didn't change code outside
>> {{GridDhtPartitionsExchangeFuture}}, otherwise it could be broken by any
>> upcoming change. Also, I didn't touch the code runnable by threads other
>> than partition-exchanger. So I have a number of guarded sections that are
>> wider than they could be, and this potentially hides issues from failure
>> detector. Does this PR make sense? Or maybe it's better to exclude
>> partition-exchanger from critical threads registry at all?
>>
>> [1] https://issues.apache.org/jira/browse/IGNITE-9710
>> [2] https://github.com/apache/ignite/pull/4962
>>
>>
>> пт, 28 сент. 2018 г. в 18:56, Maxim Muzafarov :
>>
>> > Andrey, Andrey
>> >
>> > > Thanks for being attentive! It's definitely a typo. Could you please
>> > create
>> > > an issue?
>> >
>> > I've created an issue [1] and prepared PR [2].
>> > Please, review this change.
>> >
>> > [1] https://issues.apache.org/jira/browse/IGNITE-9723
>> > [2] https://github.com/apache/ignite/pull/4862
>> >
>> > On Fri, 28 Sep 2018 at 16:58 Yakov Zhdanov  wrote:
>> >
>> > > Config option + mbean access. Does that make sense?
>> > >
>> > > Yakov
>> > >
>> > > On Fri, Sep 28, 2018, 17:17 Vladimir Ozerov 
>> > wrote:
>> > >
>> > > > Then it should be config option.
>> > > >
>> > > > пт, 28 сент. 2018 г. в 13:15, Andrey Gura :
>> > > >
>> > > > > Guys,
>> > > > >
>> > > > > why we need both config option and system property? I believe one
>> way
>> > > is
>> > > > > enough.
>> > > > > On Fri, Sep 28, 2018 at 12:38 PM Nikolay Izhikov <
>> > nizhi...@apache.org>
>> > > > > wrote:
>> > > > > >
>> > > > > > Ticket created -
>> https://issues.apache.org/jira/browse/IGNITE-9737
>> > > > > >
>> > > > > > Fixed version is 2.7.
>> > > > > >
>> > > > > > В Пт, 28/09/2018 в 11:41 +0300, Alexey Goncharuk пишет:
>> > > > > > > Nikolay, I agree, a user should be able to disable both thread
>> > > > liveness
>> > > > > > > check and checkpoint read lock timeout check from config and a
>> > > system
>> > > > > > > property.
>> > > > > > >
>> > > > > > > пт, 28 сент. 2018 г. в 11:30, Nikolay Izhikov <
>> > nizhi...@apache.org
>> > > >:
>> > > > > > >
>> > > > > > > > Hello, Igniters.
>> > > > > > > >
>> > > > > > > > I found that this feature can't be disabled from config.
>> > > > > > > > The only way to disable it is from JMX bean.
>> > > > > > > >
>> > > > > > > > I think it very dangerous: If we have some corner case or a
>> bug
>> > > in
>> > > > > this
>> > > > > > > > Watch Dog it can make Ignite unusable.
>> > > > > > > > I propose to implement possibility to disable this feature
>> > both -
>> > > > > from
>> > > > > > > > config and from JVM options.
>> > > > > > > >
>> > > > > > > > What do you think?
>> > > > > > > >
>> > > > > > > > В Чт, 27/09/2018 в 16:14 +0300, Andrey Kuznetsov пишет:
>> > > > > > > > > Maxim,
>> > > > > > > > >
>> > > > > > > > > Thanks for being attentive! It's definitely a typo. Could
>> you
>> > > > > please
>> > > > > > > >
>> > > > > > > > create
>> > > > > > > > > an issue?
>> > > > > > > > >
>> > > > > > > > > чт, 27 сент. 2018 г. в 16:00, Maxim Muzafarov <
>> > > > maxmu...@gmail.com
>> > > > > >:
>> > > > > > > > >
>> > > > > > > > > > Folks,
>> > > > > > > > > >
>> > > > > > > > > > I've found in `GridCachePartitionExchangeManager:2684`
>> [1]
>> > > > > (master
>> > > > > > > >
>> > > > > > > > branch)
>> > > > > > > > > > exchange future wrapped
>> > > > > > > > > > with double `blockingSectionEnd` method. Is it correct?
>> I
>> > > just
>> > > > > want to
>> > > > > > > > > > understand this change and
>> > > > > > > > > > how should I use this in the future.
>> > > > > > > > > >
>> > > > > > > > > > Should I file a ne

Re: Critical worker threads liveness checking drawbacks

[Nikolay Izhikov]

Hello, Alexey.

No, we don't include this ticket to 2.7.
Should we?

ср, 19 дек. 2018 г. в 12:55, Alexey Goncharuk :

> Folks, why did not we include IGNITE-10003 to ignite-2.7 release scope?
> This causes an Ignite node to be stopped by default when checkpoint read
> lock acquire times out. I expect a lot of Ignite 2.7 users will be affected
> by this mistake.
>
> We should at least update the documentation and make users aware of a
> workaround.
>
> чт, 25 окт. 2018 г. в 16:35, Alexey Goncharuk  >:
>
> > Andrey,
> >
> > I still see that checkpoint read lock acquisition raises a
> CRITICAL_ERROR,
> > which by default will shut down local node. As far as I remember, we
> > decided that by default thread timeout should not trigger node failure.
> > Now, however, it does, because we ignore SYSTEM_WORKER_BLOCKED events in
> > default configuration.
> >
> > Should we introduce another critical failure type
> > CHECKPOINT_READ_LOCK_BLOCKED or use SYSTEM_WORKER_BLOCKED for checkpoint
> > read lock acquire failure?
> >
> > --AG
> >
> > пт, 12 окт. 2018 г. в 8:29, Andrey Kuznetsov :
> >
> >> Igniters,
> >>
> >> Now I spot blocking / long-running code arising from
> >> {{GridDhtPartitionsExchangeFuture#init}} calls in partition-exchanger
> >> thread, see [1]. Ideally, all blocking operations along all possible
> code
> >> paths should be guarded implicitly from critical failure detector to
> avoid
> >> the thread from being considered blocked. There is a pull request [2]
> that
> >> provides shallow solution. I didn't change code outside
> >> {{GridDhtPartitionsExchangeFuture}}, otherwise it could be broken by any
> >> upcoming change. Also, I didn't touch the code runnable by threads other
> >> than partition-exchanger. So I have a number of guarded sections that
> are
> >> wider than they could be, and this potentially hides issues from failure
> >> detector. Does this PR make sense? Or maybe it's better to exclude
> >> partition-exchanger from critical threads registry at all?
> >>
> >> [1] https://issues.apache.org/jira/browse/IGNITE-9710
> >> [2] https://github.com/apache/ignite/pull/4962
> >>
> >>
> >> пт, 28 сент. 2018 г. в 18:56, Maxim Muzafarov :
> >>
> >> > Andrey, Andrey
> >> >
> >> > > Thanks for being attentive! It's definitely a typo. Could you please
> >> > create
> >> > > an issue?
> >> >
> >> > I've created an issue [1] and prepared PR [2].
> >> > Please, review this change.
> >> >
> >> > [1] https://issues.apache.org/jira/browse/IGNITE-9723
> >> > [2] https://github.com/apache/ignite/pull/4862
> >> >
> >> > On Fri, 28 Sep 2018 at 16:58 Yakov Zhdanov 
> wrote:
> >> >
> >> > > Config option + mbean access. Does that make sense?
> >> > >
> >> > > Yakov
> >> > >
> >> > > On Fri, Sep 28, 2018, 17:17 Vladimir Ozerov 
> >> > wrote:
> >> > >
> >> > > > Then it should be config option.
> >> > > >
> >> > > > пт, 28 сент. 2018 г. в 13:15, Andrey Gura :
> >> > > >
> >> > > > > Guys,
> >> > > > >
> >> > > > > why we need both config option and system property? I believe
> one
> >> way
> >> > > is
> >> > > > > enough.
> >> > > > > On Fri, Sep 28, 2018 at 12:38 PM Nikolay Izhikov <
> >> > nizhi...@apache.org>
> >> > > > > wrote:
> >> > > > > >
> >> > > > > > Ticket created -
> >> https://issues.apache.org/jira/browse/IGNITE-9737
> >> > > > > >
> >> > > > > > Fixed version is 2.7.
> >> > > > > >
> >> > > > > > В Пт, 28/09/2018 в 11:41 +0300, Alexey Goncharuk пишет:
> >> > > > > > > Nikolay, I agree, a user should be able to disable both
> thread
> >> > > > liveness
> >> > > > > > > check and checkpoint read lock timeout check from config
> and a
> >> > > system
> >> > > > > > > property.
> >> > > > > > >
> >> > > > > > > пт, 28 сент. 2018 г. в 11:30, Nikolay Izhikov <
> >> > nizhi...@apache.org
> >> > > >:
> >> > > > > > >
> >> > > > > > > > Hello, Igniters.
> >> > > > > > > >
> >> > > > > > > > I found that this feature can't be disabled from config.
> >> > > > > > > > The only way to disable it is from JMX bean.
> >> > > > > > > >
> >> > > > > > > > I think it very dangerous: If we have some corner case or
> a
> >> bug
> >> > > in
> >> > > > > this
> >> > > > > > > > Watch Dog it can make Ignite unusable.
> >> > > > > > > > I propose to implement possibility to disable this feature
> >> > both -
> >> > > > > from
> >> > > > > > > > config and from JVM options.
> >> > > > > > > >
> >> > > > > > > > What do you think?
> >> > > > > > > >
> >> > > > > > > > В Чт, 27/09/2018 в 16:14 +0300, Andrey Kuznetsov пишет:
> >> > > > > > > > > Maxim,
> >> > > > > > > > >
> >> > > > > > > > > Thanks for being attentive! It's definitely a typo.
> Could
> >> you
> >> > > > > please
> >> > > > > > > >
> >> > > > > > > > create
> >> > > > > > > > > an issue?
> >> > > > > > > > >
> >> > > > > > > > > чт, 27 сент. 2018 г. в 16:00, Maxim Muzafarov <
> >> > > > maxmu...@gmail.com
> >> > > > > >:
> >> > > > > > > > >
> >> > > > > > > > > > Folks,
> >> > > > > > > > > >
> >> > > > > > > > > > I've found in `GridCachePartitionExchangeMan

Re: Is it time to move forward to JUnit4 (5)?

[Dmitriy Pavlov]

Hi Igniters,

I've tried yesterday to find any of @Ignore d tests with a link to the
issue in its value on TeamCity, but I did not manage to.

Do you know if JUnit Ignored tests are completely invisible in the TC and
its REST? I've also tried to google it, but not found anything yet.

I remember that .NET Ignores are at least visible in the UI and in a REST
request (see, for example,
https://ci.ignite.apache.org/viewLog.html?buildId=2590524&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_PlatformNet&branch_IgniteTests24Java8=%3Cdefault%3E
).

Please advice me or share a link to a build with Java JUnit Ignores, which
is visible in TC.

Sincerely,
Dmitriy Pavlov

ср, 19 дек. 2018 г. в 11:59, Павлухин Иван :

> Hi Oleg,
>
> I have not quite understood who is going to call (from standpoint of
> test framework) beforeTestsStarted, beforeTest, afterTest,
> afterTestsStarted methods?
> вт, 18 дек. 2018 г. в 23:31, oignatenko :
> >
> > Hi Ivan,
> >
> > To answer your last question, yes, all the tests are to be marked with
> @Test
> > annotations, and those that are meant to be ignored are going to be
> marked
> > with @Ignore annotations. There is no exceptions to that, and these
> > annotations will work just as well on tests using our home brewed
> > beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
> >
> > For the sake of completeness, developers will also be able to use
> Before* /
> > After* annotations on their own methods in tests. The only exception
> > (clarified in respective javadocs) is that these annotations shouldn't be
> > used on overrides of our home brewed methods - and these methods, in
> > addition, will be recommended (not mandated) to avoid wia deprecation
> > notices.
> >
> > -
> >
> > As for accessing tests which have problems running under junit4, the way
> how
> > I search for these in current master is regex search in IDEA for
> > "addTestSuite.*class", that is I search in testsuites for entries that
> are
> > added using method "addTestSuite" with parameter class.
> >
> > Probably worth noting that some of the problems that were previously
> > blocking addition of particular tests have been resolved in the course of
> > working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615).
> One
> > riddle that currently looks particularly difficult to crack is Teamcity
> > failures in "Queries 1", I even haven't yet figured how to reproduce
> these
> > locally.
> >
> > regards, Oleg
> >
> >
> > Павлухин Иван wrote
> > > Hi Oleg,
> > >
> > > Now concerns about junit3 elimination are clear for me. And I agree
> > > that it is worth to do it. By the way is it possible to access tests
> > > which have problems running under junit4? I would like to take a look.
> > >
> > > Also a clarifying bit regarding next migration steps is needed. Sorry
> > > if it was described but I missed it. Currently we have tons of tests
> > > which rely on our home brewed beforeTestsStarted, beforeTest,
> > > afterTest, afterTestsStarted. Are you going to mark them all with
> > > corresponding junit4 annotations?
> > > пн, 17 дек. 2018 г. в 19:13, oignatenko <
> >
> > > oignatenko@
> >
> > > >:
> > >>
> > >> Hi Ivan,
> > >>
> > >> Per my cursory check of the code currently in master, we have 239 test
> > >> classes that couldn't migrate (that's probably about 500 or something
> > >> test
> > >> cases). For comparison, over 1600 classes migrated without problems.
> This
> > >> is
> > >> to answer your questions about how many tests are affected by change
> and
> > >> how many tests were not migrated yet.
> > >>
> > >> -
> > >>
> > >> I think we can say that only small percent of tests turned out
> sensitive
> > >> to
> > >> JUnit framework change.
> > >>
> > >> In the same time, due to very large overall amount of our tests we
> have
> > >> quite a big number as an absolute value. I point this because if
> amount
> > >> of
> > >> troublesome tests was smaller (say, order of magnitude smaller) I
> would
> > >> consider delaying migration until all tests reworked to blend totally
> > >> seamlessly into new version JUnit. This is doable - as sort of "pilot
> > >> exercise" me and Ed adjusted one test case this way - but the sheer
> > >> amount
> > >> of work on 200+ classes raises a question, whether it is worth it (I
> > >> think
> > >> it isn't).
> > >>
> > >> With regards to question 2, changes to TestCounters are farly trivial
> > >> (and
> > >> necessary) if you look at the code diff. Prior code counted amount of
> > >> test
> > >> cases in the class by JUnit 3 convention: it picked public void
> methods
> > >> without parameters starting with "test". Changed code counts test
> cases
> > >> as
> > >> JUnit 4 does: by checking if method is annotated with @Test. Change is
> > >> necessary because it will allow test developers fully use JUnit 4
> > >> features
> > >> by adding test cases that don't follow old naming requirement. I
> > >> experimented with it a little and as far as I could see

Re: Critical worker threads liveness checking drawbacks

[Dmitriy Pavlov]

Hi,

Sorry for being too formal here, but IGNITE-10003
 is in progress.

Also, I've tried to find anything related to it in the list. So according
to the list, no one was asking to include.

Sincerely,
Dmitriy Pavlov

ср, 19 дек. 2018 г. в 13:24, Nikolay Izhikov :

> Hello, Alexey.
>
> No, we don't include this ticket to 2.7.
> Should we?
>
> ср, 19 дек. 2018 г. в 12:55, Alexey Goncharuk  >:
>
> > Folks, why did not we include IGNITE-10003 to ignite-2.7 release scope?
> > This causes an Ignite node to be stopped by default when checkpoint read
> > lock acquire times out. I expect a lot of Ignite 2.7 users will be
> affected
> > by this mistake.
> >
> > We should at least update the documentation and make users aware of a
> > workaround.
> >
> > чт, 25 окт. 2018 г. в 16:35, Alexey Goncharuk <
> alexey.goncha...@gmail.com
> > >:
> >
> > > Andrey,
> > >
> > > I still see that checkpoint read lock acquisition raises a
> > CRITICAL_ERROR,
> > > which by default will shut down local node. As far as I remember, we
> > > decided that by default thread timeout should not trigger node failure.
> > > Now, however, it does, because we ignore SYSTEM_WORKER_BLOCKED events
> in
> > > default configuration.
> > >
> > > Should we introduce another critical failure type
> > > CHECKPOINT_READ_LOCK_BLOCKED or use SYSTEM_WORKER_BLOCKED for
> checkpoint
> > > read lock acquire failure?
> > >
> > > --AG
> > >
> > > пт, 12 окт. 2018 г. в 8:29, Andrey Kuznetsov :
> > >
> > >> Igniters,
> > >>
> > >> Now I spot blocking / long-running code arising from
> > >> {{GridDhtPartitionsExchangeFuture#init}} calls in partition-exchanger
> > >> thread, see [1]. Ideally, all blocking operations along all possible
> > code
> > >> paths should be guarded implicitly from critical failure detector to
> > avoid
> > >> the thread from being considered blocked. There is a pull request [2]
> > that
> > >> provides shallow solution. I didn't change code outside
> > >> {{GridDhtPartitionsExchangeFuture}}, otherwise it could be broken by
> any
> > >> upcoming change. Also, I didn't touch the code runnable by threads
> other
> > >> than partition-exchanger. So I have a number of guarded sections that
> > are
> > >> wider than they could be, and this potentially hides issues from
> failure
> > >> detector. Does this PR make sense? Or maybe it's better to exclude
> > >> partition-exchanger from critical threads registry at all?
> > >>
> > >> [1] https://issues.apache.org/jira/browse/IGNITE-9710
> > >> [2] https://github.com/apache/ignite/pull/4962
> > >>
> > >>
> > >> пт, 28 сент. 2018 г. в 18:56, Maxim Muzafarov :
> > >>
> > >> > Andrey, Andrey
> > >> >
> > >> > > Thanks for being attentive! It's definitely a typo. Could you
> please
> > >> > create
> > >> > > an issue?
> > >> >
> > >> > I've created an issue [1] and prepared PR [2].
> > >> > Please, review this change.
> > >> >
> > >> > [1] https://issues.apache.org/jira/browse/IGNITE-9723
> > >> > [2] https://github.com/apache/ignite/pull/4862
> > >> >
> > >> > On Fri, 28 Sep 2018 at 16:58 Yakov Zhdanov 
> > wrote:
> > >> >
> > >> > > Config option + mbean access. Does that make sense?
> > >> > >
> > >> > > Yakov
> > >> > >
> > >> > > On Fri, Sep 28, 2018, 17:17 Vladimir Ozerov  >
> > >> > wrote:
> > >> > >
> > >> > > > Then it should be config option.
> > >> > > >
> > >> > > > пт, 28 сент. 2018 г. в 13:15, Andrey Gura :
> > >> > > >
> > >> > > > > Guys,
> > >> > > > >
> > >> > > > > why we need both config option and system property? I believe
> > one
> > >> way
> > >> > > is
> > >> > > > > enough.
> > >> > > > > On Fri, Sep 28, 2018 at 12:38 PM Nikolay Izhikov <
> > >> > nizhi...@apache.org>
> > >> > > > > wrote:
> > >> > > > > >
> > >> > > > > > Ticket created -
> > >> https://issues.apache.org/jira/browse/IGNITE-9737
> > >> > > > > >
> > >> > > > > > Fixed version is 2.7.
> > >> > > > > >
> > >> > > > > > В Пт, 28/09/2018 в 11:41 +0300, Alexey Goncharuk пишет:
> > >> > > > > > > Nikolay, I agree, a user should be able to disable both
> > thread
> > >> > > > liveness
> > >> > > > > > > check and checkpoint read lock timeout check from config
> > and a
> > >> > > system
> > >> > > > > > > property.
> > >> > > > > > >
> > >> > > > > > > пт, 28 сент. 2018 г. в 11:30, Nikolay Izhikov <
> > >> > nizhi...@apache.org
> > >> > > >:
> > >> > > > > > >
> > >> > > > > > > > Hello, Igniters.
> > >> > > > > > > >
> > >> > > > > > > > I found that this feature can't be disabled from config.
> > >> > > > > > > > The only way to disable it is from JMX bean.
> > >> > > > > > > >
> > >> > > > > > > > I think it very dangerous: If we have some corner case
> or
> > a
> > >> bug
> > >> > > in
> > >> > > > > this
> > >> > > > > > > > Watch Dog it can make Ignite unusable.
> > >> > > > > > > > I propose to implement possibility to disable this
> feature
> > >> > both -
> > >> > > > > from
> > >> > > > > > > > config and from JVM options.
> > >> > > > > > > >
> > >> > > > 

[jira] [Created] (IGNITE-10738) MVCC: Enable page evictions for MVCC caches with in-memory mode

[Roman Kondakov (JIRA)]

Roman Kondakov created IGNITE-10738:
---

 Summary: MVCC: Enable page evictions for MVCC caches with 
in-memory mode
 Key: IGNITE-10738
 URL: https://issues.apache.org/jira/browse/IGNITE-10738
 Project: Ignite
  Issue Type: Improvement
  Components: mvcc
Reporter: Roman Kondakov


Currently MVCC caches are not allowed to be located in data regions with 
enabled page evictions in in-memory mode because unexpected data purges may 
lead to repeatable read semantics violations. See this 
[thread|http://apache-ignite-developers.2346864.n4.nabble.com/Page-eviction-for-in-memory-mode-with-enabled-MVCC-tt39460.html]
 on dev-list for details.

We need to think about a suitable solution to this problem and implement it.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (IGNITE-10739) get rid of using JUnit 3 API in IgniteConfigVariationsAbstractTest

[Oleg Ignatenko (JIRA)]

Oleg Ignatenko created IGNITE-10739:
---

 Summary: get rid of using JUnit 3 API in 
IgniteConfigVariationsAbstractTest
 Key: IGNITE-10739
 URL: https://issues.apache.org/jira/browse/IGNITE-10739
 Project: Ignite
  Issue Type: Task
Affects Versions: 2.8
Reporter: Oleg Ignatenko


{{IgniteConfigVariationsAbstractTest}} and part of test framework that uses 
this class very heavily rely on JUnit 3 specific features.

The task is to get rid of these and use newer versions instead - preferably 
JUnit 4, although JUnit 5 is also an option

This will allow consistent JUnit framework version across Ignite tests which in 
turn is expected to simplify maintenance and development of the tests 
(including ignoring tests, workiing on Teamcity related things etc).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Is it time to move forward to JUnit4 (5)?

[oignatenko]

Hi Ivan,

These methods are called from within GridAbstractTest in exactly same
sequence as they were called in the past from JUnit 3 TestCase class.

This is by the way the reason why these methods should not be annotated with
Before* / After* - because if someone does that then method will be called
twice, once from JUnit 4 framework and one more time from GridAbstractTest.

regards, Oleg


Павлухин Иван wrote
> Hi Oleg,
> 
> I have not quite understood who is going to call (from standpoint of
> test framework) beforeTestsStarted, beforeTest, afterTest,
> afterTestsStarted methods?
> вт, 18 дек. 2018 г. в 23:31, oignatenko <

> oignatenko@

> >:
>>
>> Hi Ivan,
>>
>> To answer your last question, yes, all the tests are to be marked with
>> @Test
>> annotations, and those that are meant to be ignored are going to be
>> marked
>> with @Ignore annotations. There is no exceptions to that, and these
>> annotations will work just as well on tests using our home brewed
>> beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
>>
>> For the sake of completeness, developers will also be able to use Before*
>> /
>> After* annotations on their own methods in tests. The only exception
>> (clarified in respective javadocs) is that these annotations shouldn't be
>> used on overrides of our home brewed methods - and these methods, in
>> addition, will be recommended (not mandated) to avoid wia deprecation
>> notices.
>>
>> -
>>
>> As for accessing tests which have problems running under junit4, the way
>> how
>> I search for these in current master is regex search in IDEA for
>> "addTestSuite.*class", that is I search in testsuites for entries that
>> are
>> added using method "addTestSuite" with parameter class.
>>
>> Probably worth noting that some of the problems that were previously
>> blocking addition of particular tests have been resolved in the course of
>> working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One
>> riddle that currently looks particularly difficult to crack is Teamcity
>> failures in "Queries 1", I even haven't yet figured how to reproduce
>> these
>> locally.
>>
>> regards, Oleg
>>
>>
>> Павлухин Иван wrote
>> > Hi Oleg,
>> >
>> > Now concerns about junit3 elimination are clear for me. And I agree
>> > that it is worth to do it. By the way is it possible to access tests
>> > which have problems running under junit4? I would like to take a look.
>> >
>> > Also a clarifying bit regarding next migration steps is needed. Sorry
>> > if it was described but I missed it. Currently we have tons of tests
>> > which rely on our home brewed beforeTestsStarted, beforeTest,
>> > afterTest, afterTestsStarted. Are you going to mark them all with
>> > corresponding junit4 annotations?
>> > пн, 17 дек. 2018 г. в 19:13, oignatenko <
>>
>> > oignatenko@
>>
>> > >:
>> >>
>> >> Hi Ivan,
>> >>
>> >> Per my cursory check of the code currently in master, we have 239 test
>> >> classes that couldn't migrate (that's probably about 500 or something
>> >> test
>> >> cases). For comparison, over 1600 classes migrated without problems.
>> This
>> >> is
>> >> to answer your questions about how many tests are affected by change
>> and
>> >> how many tests were not migrated yet.
>> >>
>> >> -
>> >>
>> >> I think we can say that only small percent of tests turned out
>> sensitive
>> >> to
>> >> JUnit framework change.
>> >>
>> >> In the same time, due to very large overall amount of our tests we
>> have
>> >> quite a big number as an absolute value. I point this because if
>> amount
>> >> of
>> >> troublesome tests was smaller (say, order of magnitude smaller) I
>> would
>> >> consider delaying migration until all tests reworked to blend totally
>> >> seamlessly into new version JUnit. This is doable - as sort of "pilot
>> >> exercise" me and Ed adjusted one test case this way - but the sheer
>> >> amount
>> >> of work on 200+ classes raises a question, whether it is worth it (I
>> >> think
>> >> it isn't).
>> >>
>> >> With regards to question 2, changes to TestCounters are farly trivial
>> >> (and
>> >> necessary) if you look at the code diff. Prior code counted amount of
>> >> test
>> >> cases in the class by JUnit 3 convention: it picked public void
>> methods
>> >> without parameters starting with "test". Changed code counts test
>> cases
>> >> as
>> >> JUnit 4 does: by checking if method is annotated with @Test. Change is
>> >> necessary because it will allow test developers fully use JUnit 4
>> >> features
>> >> by adding test cases that don't follow old naming requirement. I
>> >> experimented with it a little and as far as I could see the old
>> >> TestCounters
>> >> indeed break when there are methods following new convention, it
>> >> triggered
>> >> afterTestsStopped too early.
>> >>
>> >> The answer to your question 3 lies in javadocs I added to
>> runSerializer
>> >> declaration, or, more precisely, in TestCounters javadoc referred from
>> >> there. As you can see, current plan is t

Re: Is it time to move forward to JUnit4 (5)?

[Павлухин Иван]

Dmitriy,

Thank you for noticing! It seems we have a problem here. When junit4
test is called from junit3 suite (with help of JUnit4TestAdapter) such
tests is skipped silently. It seems that we cannot use @Ignore
everywhere yet.
ср, 19 дек. 2018 г. в 15:09, oignatenko :
>
> Hi Ivan,
>
> These methods are called from within GridAbstractTest in exactly same
> sequence as they were called in the past from JUnit 3 TestCase class.
>
> This is by the way the reason why these methods should not be annotated with
> Before* / After* - because if someone does that then method will be called
> twice, once from JUnit 4 framework and one more time from GridAbstractTest.
>
> regards, Oleg
>
>
> Павлухин Иван wrote
> > Hi Oleg,
> >
> > I have not quite understood who is going to call (from standpoint of
> > test framework) beforeTestsStarted, beforeTest, afterTest,
> > afterTestsStarted methods?
> > вт, 18 дек. 2018 г. в 23:31, oignatenko <
>
> > oignatenko@
>
> > >:
> >>
> >> Hi Ivan,
> >>
> >> To answer your last question, yes, all the tests are to be marked with
> >> @Test
> >> annotations, and those that are meant to be ignored are going to be
> >> marked
> >> with @Ignore annotations. There is no exceptions to that, and these
> >> annotations will work just as well on tests using our home brewed
> >> beforeTestsStarted, beforeTest, afterTest, afterTestsStarted.
> >>
> >> For the sake of completeness, developers will also be able to use Before*
> >> /
> >> After* annotations on their own methods in tests. The only exception
> >> (clarified in respective javadocs) is that these annotations shouldn't be
> >> used on overrides of our home brewed methods - and these methods, in
> >> addition, will be recommended (not mandated) to avoid wia deprecation
> >> notices.
> >>
> >> -
> >>
> >> As for accessing tests which have problems running under junit4, the way
> >> how
> >> I search for these in current master is regex search in IDEA for
> >> "addTestSuite.*class", that is I search in testsuites for entries that
> >> are
> >> added using method "addTestSuite" with parameter class.
> >>
> >> Probably worth noting that some of the problems that were previously
> >> blocking addition of particular tests have been resolved in the course of
> >> working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One
> >> riddle that currently looks particularly difficult to crack is Teamcity
> >> failures in "Queries 1", I even haven't yet figured how to reproduce
> >> these
> >> locally.
> >>
> >> regards, Oleg
> >>
> >>
> >> Павлухин Иван wrote
> >> > Hi Oleg,
> >> >
> >> > Now concerns about junit3 elimination are clear for me. And I agree
> >> > that it is worth to do it. By the way is it possible to access tests
> >> > which have problems running under junit4? I would like to take a look.
> >> >
> >> > Also a clarifying bit regarding next migration steps is needed. Sorry
> >> > if it was described but I missed it. Currently we have tons of tests
> >> > which rely on our home brewed beforeTestsStarted, beforeTest,
> >> > afterTest, afterTestsStarted. Are you going to mark them all with
> >> > corresponding junit4 annotations?
> >> > пн, 17 дек. 2018 г. в 19:13, oignatenko <
> >>
> >> > oignatenko@
> >>
> >> > >:
> >> >>
> >> >> Hi Ivan,
> >> >>
> >> >> Per my cursory check of the code currently in master, we have 239 test
> >> >> classes that couldn't migrate (that's probably about 500 or something
> >> >> test
> >> >> cases). For comparison, over 1600 classes migrated without problems.
> >> This
> >> >> is
> >> >> to answer your questions about how many tests are affected by change
> >> and
> >> >> how many tests were not migrated yet.
> >> >>
> >> >> -
> >> >>
> >> >> I think we can say that only small percent of tests turned out
> >> sensitive
> >> >> to
> >> >> JUnit framework change.
> >> >>
> >> >> In the same time, due to very large overall amount of our tests we
> >> have
> >> >> quite a big number as an absolute value. I point this because if
> >> amount
> >> >> of
> >> >> troublesome tests was smaller (say, order of magnitude smaller) I
> >> would
> >> >> consider delaying migration until all tests reworked to blend totally
> >> >> seamlessly into new version JUnit. This is doable - as sort of "pilot
> >> >> exercise" me and Ed adjusted one test case this way - but the sheer
> >> >> amount
> >> >> of work on 200+ classes raises a question, whether it is worth it (I
> >> >> think
> >> >> it isn't).
> >> >>
> >> >> With regards to question 2, changes to TestCounters are farly trivial
> >> >> (and
> >> >> necessary) if you look at the code diff. Prior code counted amount of
> >> >> test
> >> >> cases in the class by JUnit 3 convention: it picked public void
> >> methods
> >> >> without parameters starting with "test". Changed code counts test
> >> cases
> >> >> as
> >> >> JUnit 4 does: by checking if method is annotated with @Test. Change is
> >> >> necessary because it will allow test developers fully use 

Re: Suggestion to improve deadlock detection

[Павлухин Иван]

Igor,

I see your points. And I agree to start with "lightweight
implementation". Today we have 2PL and there is no activity on
implementing rollback to savepoint. And if we do it in the future we
will have to return to the subject of deadlock detection anyway.

I will proceed with "forward-only" approach.
вт, 18 дек. 2018 г. в 18:33, Seliverstov Igor :
>
> Ivan,
>
> I would prefer forward-only implementation even knowing it allows false
> positive check results.
>
> Why I think so:
>
> 1) From my experience, when we have any future is waiting for reply, we
> have to take a failover into consideration.
> Usually failover implementations are way more complex than an initial
> algorithm itself.
> Forward-only version doesn't need any failover implementation as it expects
> failed probes (the probes didn't face a deadlock ).
>
> 2) Simple lightweight feature implementation is a really good point to
> start - it may be extended if needed but really often such extending
> doesn't need at all.
>
> 3) Any implementation allow false positive result - we are working with
> distributed system, there are a bunch of processes happens at the same time
> and,
> for example, concurrently happened rollback on timeout may solve a deadlock
> but probe is finished at this time, so- there is a rollback on deadlock
> also as a result.
>
> 4) The described case (when false positive result is probable) isn't usual
> but, potentially, extremely rare, I don't think we should solve it since it
> doesn't break the grid.
>
> Regards,
> Igor
>
> вт, 18 дек. 2018 г. в 17:57, Павлухин Иван :
>
> > Hi folks,
> >
> > During implementation of edge-chasing deadlock detection algorithm in
> > scope of [1] it has been realized that basically we have 2 options for
> > "chasing" strategy. I will use terms Near when GridNearTxLocal is
> > assumed and Dht when GridDhtTxLocal (tx which updates primary
> > partition). So, here is 2 mentioned strategies:
> > 1. Send initial probe when Dht tx blocks waiting for another tx to
> > release a key. Initial probe is sent from waiting Dht tx to Near tx
> > holding a lock. If receiving Near tx is blocked as well then it relays
> > the probe to Dht tx it awaits response from. So, the probe is
> > traveling like Dht -> Near -> Dht -> Near -> ... Let's call the
> > approach "forward-only".
> > 2. Send probes only between Near transactions. This approach requires
> > additional request-response call which Near tx issues to Dht node to
> > check if tx sending a request is waiting for another tx. The call
> > returns identifier of a Near tx blocking tx sending a request (if
> > sending tx is in fact blocked). Then waiting Near tx relays a probe to
> > blocked Near tx. Let's call that approach "lock-checking".
> >
> > I would like to define several points to consider while comparing
> > approaches (please point out more if I miss something):
> > 1. Correctness. Especially regarding reporting false deadlocks.
> > 2. Extensibility. Rollback to savepoint and generalization for classic
> > transactions should be considered.
> > 3. Messaging overhead.
> > 4. Simplicity of implementation.
> >
> > You can find an implementation of "lock-checking" approach in PR
> > attached to the ticket [1]. Currently it works correctly only for
> > transactions obeying strict two-phase locking (2PL). It is fairly easy
> > to adopt PR to implement "forward-only" approach. Which will work
> > correct in conditions of 2PL. "lock-checking" algorithm uses 1.5 times
> > more messages than "forward-only". Implementation of "forward-only"
> > algorithm is simpler in a sense that it does not require introducing
> > lock-wait-check messages and future. But as for me it does not look as
> > big deal. I think that implementation efforts for both approaches are
> > almost equal so far.
> >
> > But corner stone here is an extensibility. Let's imagine that we are
> > going to implement rollback to savepoint. One suggested approach to
> > extend "forward-only" approach for that case is invalidating sent
> > probe. Basically, a tx initiated deadlock check assigns unique id to a
> > probe. And this probe id is invalidated when the tx wakes up from
> > wait. If that tx receives back a probe with invalidated id it simply
> > discards it. If id is not invalidated it means a deadlock. But false
> > deadlock still can be detected. I will provide an example when it does
> > not work correctly.
> >
> > Please note, that our transactions can request multiple locks
> > simultaneously (so-called AND model). Also rollback to savepoint
> > releases locks obtained after creating a savepoint. Let's use
> > following notation. "*" means savepoint. "^" means rollback to
> > previous savepoint. "!" means acquiring a lock. "?" means waiting for
> > a lock. "&" means requesting multiple locks simultaneously. See the
> > following transaction execution scenario for 3 transactions and 3
> > keys:
> > TA| TB |   TC
> > |  |   *
> > 

Re: Service grid redesign

[Denis Mekhanikov]

Slava,
I think, it's better to replace word "Change" with "Request".

Nik,
We have IgniteServiceProcessor and GridServiceProcessor with singular
"Service",
ServicesDeploymentManager and ServicesDeploymentTask with plural "Services"
for some reason.
So, you need to remember, where Service and where Services is used.
I think, we should unify these names.
And ServiceSingleDeploymentsResults name doesn't make sense to me.
"Single deployments" doesn't sound right.

ServicesFullDeploymentsMessage is derived
from GridDhtPartitionsFullMessage.
It doesn't really reflect its function. This message is supposed to mark
the point in time, when deployment is finished.

Denis


пт, 14 дек. 2018 г. в 11:30, Vyacheslav Daradur :

> >*1. Testing of the cache-based implementation of the service grid.*
> > I think, we should make a test suite, that will test the old
> implementation
> > until we remove it from the project.
>
> Agree. This is exactly what should be done as the first step once
> phase 1 will be merged.
> I think all tests in the package:
> "org.apache.ignite.internal.processors.service" should be moved to
> separate test-suite and new build-plan should be added on TC and
> included in RunAll.
>
> > *2. DynamicServiceChangeRequest.*
> > I think, this class should be splat into two.
>
> Personally, I agree, but I have faced opposition at the design step.
> I changed to the following structure:
>
> abstract class ServiceAbstractChange implements Serializable {
> protected final IgniteUuid srvcId;
> }
>
> class ServiceDeploymentChange extends ServiceAbstractChange {
> ServiceConfiguration cfg;
> }
>
> class ServiceUndeploymentChange extends ServiceAbstractChange { }
>
> I hope that further reviewers will agree with us.
>
> > *3. Naming.*
>
> About "Services" -> "Service" and "Deployments" -> "Deployment"
> Personally, I agree with Nikolay, because it's more descriptive since
> manages several services, not single.
> But, I understand Denis's point of view, we have a lot of classes with
> "Service" prefix in naming and "Services" looks a bit alien.
>
> > *DynamicServicesChangeRequestBatchMessage -> DynamicServiceChangeRequest*
> Prefix "Dynamic" has no sense anymore since we reworked message
> structure as in p.2. so "ServiceChangeBatchRequest" will be better
> name.
>
> > *ServicesSingleDeploymentsMessage -> ServiceDeploymentResponse*
> It's not a response and is not sent to the sender. This message is
> sent to the coordinator and contains *single node* deployments.
>
> > *ServicesFullDeploymentsMessage -> ServiceDeploymentFinishMessage*
> This should be named similar way as the previous one, but the message
> contains deployments of *full set of nodes*.
>
>
> On Fri, Dec 14, 2018 at 10:58 AM Nikolay Izhikov 
> wrote:
> >
> > Hello, Denis.
> >
> > Great news.
> >
> > > *1. Testing of the cache-based implementation of the service grid.*
> > > I think, we should make a test suite, that will test the old
> implementation> until we> remove it from the project.
> >
> > Aggree. Let's do it.
> >
> > > *2. DynamicServiceChangeRequest.*
> > > I think, this class should be splat into two.
> >
> > Agree. Lets's do it.
> >
> > > *ServicesDeploymentManager*, *ServicesDeploymentTask *and all other
> classes> with Services word in them.
> > > I think, they would look better if we use a singular word *Service
> *instead.
> > > Same for *Deployments*.
> >
> > Personally, I want that names as clearly as possible reflects class
> content for reader.
> > If we deploy *several* services then it has to be Service*S*.
> >
> > Same for deployment - if this message will initiate single deployment
> process then it should use deployment.
> > otherwise - deployments.
> >
> > So my opinion - it's better to keep current naming.
> >
> > В Чт, 13/12/2018 в 19:36 +0300, Denis Mekhanikov пишет:
> > > Guys,
> > >
> > > I've been looking through the PR by Vyacheslav for past few weeks.
> > > Slava, great job! You've done an impressive amount of work.
> > >
> > > I posted my comments to the PR and had a few calls with Slava.
> > > I am close to finishing my review.
> > > There are some points, that I'd like to settle in this discussion to
> avoid
> > > controversy.
> > >
> > > *1. Testing of the cache-based implementation of the service grid.*
> > > I think, we should make a test suite, that will test the old
> implementation
> > > until we
> > > remove it from the project.
> > >
> > > *2. DynamicServiceChangeRequest.*
> > > I think, this class should be splat into two.
> > > I don't see any point in having a single class with "*flags"* field,
> that
> > > shows, what action it actually represents.
> > > Usage of *deploy(), markDeploy(...), undeploy(), markUndeploy(...)*
> looks
> > > wrong.
> > > Why not have a separate message type for each action instead?
> > >
> > > *3. Naming.*
> > > I suggest renaming the following classes:
> > > *ServicesDeploymentManager*, *ServicesDeploymentTask *and all other
> classes
> > > with Services word in t

[jira] [Created] (IGNITE-10740) Add documentation for IGNITE_DISABLE_TRIGGERING_CACHE_INTERCEPTOR_ON_CONFLICT

[Sergey Antonov (JIRA)]

Sergey Antonov created IGNITE-10740:
---

 Summary: Add documentation for 
IGNITE_DISABLE_TRIGGERING_CACHE_INTERCEPTOR_ON_CONFLICT
 Key: IGNITE-10740
 URL: https://issues.apache.org/jira/browse/IGNITE-10740
 Project: Ignite
  Issue Type: Task
  Components: documentation
Reporter: Sergey Antonov
Assignee: Artem Budnikov
 Fix For: 2.8


We should add to documentation 
IGNITE_DISABLE_TRIGGERING_CACHE_INTERCEPTOR_ON_CONFLICT option.

As a reference you could get javadoc from skipInterceptor() :

{{Checks, that cache interceptor should be skipped. It is expects by default 
behavior that Interceptor methods (\{@link 
CacheInterceptor#onBeforePut(Cache.Entry, Object)}, \{@link 
CacheInterceptor#onAfterPut(Cache.Entry)}, \{@link 
CacheInterceptor#onBeforeRemove(Cache.Entry)} and \{@link 
CacheInterceptor#onAfterRemove(Cache.Entry)}) will be called, but \{@link 
CacheInterceptor#onGet(Object, Object)}. This can even make DR-update flow 
broken in case of non-idempotent Interceptor and force users to call onGet 
manually as the only workaround. Also, user may want to skip Interceptor to 
avoid redundant entry transformation for DR updates and exchange with internal 
data b/w data centres which is a normal case.}}

 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (IGNITE-10741) MVCC: Document disabled page evictions for in-memory MVCC caches.

[Roman Kondakov (JIRA)]

Roman Kondakov created IGNITE-10741:
---

 Summary: MVCC: Document disabled page evictions for in-memory MVCC 
caches.
 Key: IGNITE-10741
 URL: https://issues.apache.org/jira/browse/IGNITE-10741
 Project: Ignite
  Issue Type: Task
  Components: documentation, mvcc
Reporter: Roman Kondakov


Currently data pages evictions are disabled for {{TRANSACTIONAL_SNAPSHOT}} 
caches because it can cause violations for repeatable read guarantees.
We should reflect it in our documentation.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

RE: Asynchronous index rebuild

[Stanislav Lukyanov]

Hi Vladimir,

Thanks for this summary!

Why the third option and not the second?

The process of join-leave-build indexes-rejoin sounds kind of heavy.
Topology changes are complex because of PME, so I think the less PME – the 
better.

Rejoin to build indexes also means that not having indexes for one cache 
prevents the node
to serve operations for other caches (for which the node has indexes or which 
don’t have indexes at all).

I really like the idea of re-using late affinity assignment here.
The semantics is simple, the implementation looks sort-of simple as well 
(although it adds another bit of complexity to PME).

Why do you prefer to go with the join-leave-rejoin approach?

Thanks,
Stan

From: Vladimir Ozerov
Sent: 30 ноября 2018 г. 13:09
To: dev
Subject: Asynchronous index rebuild

Igniters,

During work on index rebuild bug [1] I realized that our asynchronous
nature of index rebuild logic may lead to long SQL queries in user
applications, indistinguishable from hangs.

Fundamental problem here is that we think that node is ready to become
partition owner when partition data is rebalanced. But correct rule form
user standpoint is that both partitions and indexes are ready. This is
always true for in-memory mode and when fresh node with persistent is
started, as indexes are filled with data during rebalance. But this is not
true for several other cases:
1) When new index were created while a node was done
2) When index.bin was corrupted or removed
3) Possible in future: rebalance through partition files [2], as current
IEP doesn't cover secondary indexes.

Let's think on how to overcome this problem. First, we may cover it on SQL
level: extract used indexes on query planning phase, attach them to query
messages, check on mapper side that indexes are ready, re-try if not.
Second, we may think of making index rebuild as a part of rebalance
procedure, so that node do not become primary until both partitions and
indexes are ready.
Third, we may detect missing indexes during join. Then node may leave
topology, rebuild indexes locally, then re-join. For missing index.bin
case, rebuild process may be started even without joining node to the
cluster, though it potentially may lead to rebuild of an index which
doesn't exist in the cluster anymore.

The third option looks appears to be the most promising to me. What do you
think?

Vladimir.

[1] https://issues.apache.org/jira/browse/IGNITE-10291
[2]
https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing

Re: Service grid redesign

[Vyacheslav Daradur]

> We have IgniteServiceProcessor and GridServiceProcessor with singular 
> "Service"

Maybe we should rename new 'IgniteServiceProcessor' to
'IgniteServicesProcessor'?

> And ServiceSingleDeploymentsResults name doesn't make sense to me.
> "Single deployments" doesn't sound right.

'Single' means 'single node', maybe we should use one of the following:
- 'ServicesSingleNodeDeploymentsResults'
- 'ServicesNodeDeploymentsResults'
- 'ServicesInstanceDeploymentsResults'

On Wed, Dec 19, 2018 at 4:26 PM Denis Mekhanikov  wrote:
>
> Slava,
> I think, it's better to replace word "Change" with "Request".
>
> Nik,
> We have IgniteServiceProcessor and GridServiceProcessor with singular
> "Service",
> ServicesDeploymentManager and ServicesDeploymentTask with plural "Services"
> for some reason.
> So, you need to remember, where Service and where Services is used.
> I think, we should unify these names.
> And ServiceSingleDeploymentsResults name doesn't make sense to me.
> "Single deployments" doesn't sound right.
>
> ServicesFullDeploymentsMessage is derived
> from GridDhtPartitionsFullMessage.
> It doesn't really reflect its function. This message is supposed to mark
> the point in time, when deployment is finished.
>
> Denis
>
>
> пт, 14 дек. 2018 г. в 11:30, Vyacheslav Daradur :
>
> > >*1. Testing of the cache-based implementation of the service grid.*
> > > I think, we should make a test suite, that will test the old
> > implementation
> > > until we remove it from the project.
> >
> > Agree. This is exactly what should be done as the first step once
> > phase 1 will be merged.
> > I think all tests in the package:
> > "org.apache.ignite.internal.processors.service" should be moved to
> > separate test-suite and new build-plan should be added on TC and
> > included in RunAll.
> >
> > > *2. DynamicServiceChangeRequest.*
> > > I think, this class should be splat into two.
> >
> > Personally, I agree, but I have faced opposition at the design step.
> > I changed to the following structure:
> >
> > abstract class ServiceAbstractChange implements Serializable {
> > protected final IgniteUuid srvcId;
> > }
> >
> > class ServiceDeploymentChange extends ServiceAbstractChange {
> > ServiceConfiguration cfg;
> > }
> >
> > class ServiceUndeploymentChange extends ServiceAbstractChange { }
> >
> > I hope that further reviewers will agree with us.
> >
> > > *3. Naming.*
> >
> > About "Services" -> "Service" and "Deployments" -> "Deployment"
> > Personally, I agree with Nikolay, because it's more descriptive since
> > manages several services, not single.
> > But, I understand Denis's point of view, we have a lot of classes with
> > "Service" prefix in naming and "Services" looks a bit alien.
> >
> > > *DynamicServicesChangeRequestBatchMessage -> DynamicServiceChangeRequest*
> > Prefix "Dynamic" has no sense anymore since we reworked message
> > structure as in p.2. so "ServiceChangeBatchRequest" will be better
> > name.
> >
> > > *ServicesSingleDeploymentsMessage -> ServiceDeploymentResponse*
> > It's not a response and is not sent to the sender. This message is
> > sent to the coordinator and contains *single node* deployments.
> >
> > > *ServicesFullDeploymentsMessage -> ServiceDeploymentFinishMessage*
> > This should be named similar way as the previous one, but the message
> > contains deployments of *full set of nodes*.
> >
> >
> > On Fri, Dec 14, 2018 at 10:58 AM Nikolay Izhikov 
> > wrote:
> > >
> > > Hello, Denis.
> > >
> > > Great news.
> > >
> > > > *1. Testing of the cache-based implementation of the service grid.*
> > > > I think, we should make a test suite, that will test the old
> > implementation> until we> remove it from the project.
> > >
> > > Aggree. Let's do it.
> > >
> > > > *2. DynamicServiceChangeRequest.*
> > > > I think, this class should be splat into two.
> > >
> > > Agree. Lets's do it.
> > >
> > > > *ServicesDeploymentManager*, *ServicesDeploymentTask *and all other
> > classes> with Services word in them.
> > > > I think, they would look better if we use a singular word *Service
> > *instead.
> > > > Same for *Deployments*.
> > >
> > > Personally, I want that names as clearly as possible reflects class
> > content for reader.
> > > If we deploy *several* services then it has to be Service*S*.
> > >
> > > Same for deployment - if this message will initiate single deployment
> > process then it should use deployment.
> > > otherwise - deployments.
> > >
> > > So my opinion - it's better to keep current naming.
> > >
> > > В Чт, 13/12/2018 в 19:36 +0300, Denis Mekhanikov пишет:
> > > > Guys,
> > > >
> > > > I've been looking through the PR by Vyacheslav for past few weeks.
> > > > Slava, great job! You've done an impressive amount of work.
> > > >
> > > > I posted my comments to the PR and had a few calls with Slava.
> > > > I am close to finishing my review.
> > > > There are some points, that I'd like to settle in this discussion to
> > avoid
> > > > controversy.
> > > >
> > > > *1. Testing

Re: Service grid redesign

[Nikolay Izhikov]

Denis,

I don't think that differences with your and my naming is huge :)
And, it's definetely a matter of taste.

If there is no any other issues with PR let's rename and move on! :)

ср, 19 дек. 2018 г. в 17:32, Vyacheslav Daradur :

> > We have IgniteServiceProcessor and GridServiceProcessor with singular
> "Service"
>
> Maybe we should rename new 'IgniteServiceProcessor' to
> 'IgniteServicesProcessor'?
>
> > And ServiceSingleDeploymentsResults name doesn't make sense to me.
> > "Single deployments" doesn't sound right.
>
> 'Single' means 'single node', maybe we should use one of the following:
> - 'ServicesSingleNodeDeploymentsResults'
> - 'ServicesNodeDeploymentsResults'
> - 'ServicesInstanceDeploymentsResults'
>
> On Wed, Dec 19, 2018 at 4:26 PM Denis Mekhanikov 
> wrote:
> >
> > Slava,
> > I think, it's better to replace word "Change" with "Request".
> >
> > Nik,
> > We have IgniteServiceProcessor and GridServiceProcessor with singular
> > "Service",
> > ServicesDeploymentManager and ServicesDeploymentTask with plural
> "Services"
> > for some reason.
> > So, you need to remember, where Service and where Services is used.
> > I think, we should unify these names.
> > And ServiceSingleDeploymentsResults name doesn't make sense to me.
> > "Single deployments" doesn't sound right.
> >
> > ServicesFullDeploymentsMessage is derived
> > from GridDhtPartitionsFullMessage.
> > It doesn't really reflect its function. This message is supposed to mark
> > the point in time, when deployment is finished.
> >
> > Denis
> >
> >
> > пт, 14 дек. 2018 г. в 11:30, Vyacheslav Daradur :
> >
> > > >*1. Testing of the cache-based implementation of the service grid.*
> > > > I think, we should make a test suite, that will test the old
> > > implementation
> > > > until we remove it from the project.
> > >
> > > Agree. This is exactly what should be done as the first step once
> > > phase 1 will be merged.
> > > I think all tests in the package:
> > > "org.apache.ignite.internal.processors.service" should be moved to
> > > separate test-suite and new build-plan should be added on TC and
> > > included in RunAll.
> > >
> > > > *2. DynamicServiceChangeRequest.*
> > > > I think, this class should be splat into two.
> > >
> > > Personally, I agree, but I have faced opposition at the design step.
> > > I changed to the following structure:
> > >
> > > abstract class ServiceAbstractChange implements Serializable {
> > > protected final IgniteUuid srvcId;
> > > }
> > >
> > > class ServiceDeploymentChange extends ServiceAbstractChange {
> > > ServiceConfiguration cfg;
> > > }
> > >
> > > class ServiceUndeploymentChange extends ServiceAbstractChange { }
> > >
> > > I hope that further reviewers will agree with us.
> > >
> > > > *3. Naming.*
> > >
> > > About "Services" -> "Service" and "Deployments" -> "Deployment"
> > > Personally, I agree with Nikolay, because it's more descriptive since
> > > manages several services, not single.
> > > But, I understand Denis's point of view, we have a lot of classes with
> > > "Service" prefix in naming and "Services" looks a bit alien.
> > >
> > > > *DynamicServicesChangeRequestBatchMessage ->
> DynamicServiceChangeRequest*
> > > Prefix "Dynamic" has no sense anymore since we reworked message
> > > structure as in p.2. so "ServiceChangeBatchRequest" will be better
> > > name.
> > >
> > > > *ServicesSingleDeploymentsMessage -> ServiceDeploymentResponse*
> > > It's not a response and is not sent to the sender. This message is
> > > sent to the coordinator and contains *single node* deployments.
> > >
> > > > *ServicesFullDeploymentsMessage -> ServiceDeploymentFinishMessage*
> > > This should be named similar way as the previous one, but the message
> > > contains deployments of *full set of nodes*.
> > >
> > >
> > > On Fri, Dec 14, 2018 at 10:58 AM Nikolay Izhikov 
> > > wrote:
> > > >
> > > > Hello, Denis.
> > > >
> > > > Great news.
> > > >
> > > > > *1. Testing of the cache-based implementation of the service grid.*
> > > > > I think, we should make a test suite, that will test the old
> > > implementation> until we> remove it from the project.
> > > >
> > > > Aggree. Let's do it.
> > > >
> > > > > *2. DynamicServiceChangeRequest.*
> > > > > I think, this class should be splat into two.
> > > >
> > > > Agree. Lets's do it.
> > > >
> > > > > *ServicesDeploymentManager*, *ServicesDeploymentTask *and all other
> > > classes> with Services word in them.
> > > > > I think, they would look better if we use a singular word *Service
> > > *instead.
> > > > > Same for *Deployments*.
> > > >
> > > > Personally, I want that names as clearly as possible reflects class
> > > content for reader.
> > > > If we deploy *several* services then it has to be Service*S*.
> > > >
> > > > Same for deployment - if this message will initiate single deployment
> > > process then it should use deployment.
> > > > otherwise - deployments.
> > > >
> > > > So my opinion - it's better to keep current naming.
> > > >

Re: Pre-touch for Ignite off-heap memory

[Stanislav Lukyanov]

Well, I'm not saying "we need all three features all the time", I'm just
summarizing why someone would use pre-touch.
And having c) is better than having none.

Also, we do get a) and b) as well, the timing is just a bit different.
But I find it better to fail at the time of dynamic cache creation (which
should be approached with caution anyway) than
during the normal execution.

Stan

On Tue, Dec 18, 2018 at 4:01 PM Павлухин Иван  wrote:

> Stan,
>
> In one of your emails you wrote why you need pre-touch:
> > a) JVM doesn’t have to do it during the actual work (avoiding overhead
> for physical page allocation, possible contention with page cache, etc)
> > b) JVM fails fast if the Xmx is greater than available RAM
> > c) New processes will not be able to claim the memory JVM took for itself
>
> As far as I see only c) can be satisfied if lazy region initialization
> is enabled. Am I missing something?
> пн, 17 дек. 2018 г. в 18:11, Stanislav Lukyanov :
> >
> > I don’t think these two issues require to be solved together, although I
> agree there is some connection.
> >
> >
> >
> > I guess we agree that pre-touch should a be off by default.
> >
> > Similarly, lazy data region initialization from IGNITE-9113 can be on by
> default.
> >
> > We can have two boolean properties controlling each feature, e.g.
> >
> > IGNITE_EAGER_DATA_REGION_INITIALIZATION
> >
> > IGNITE_PRE_TOUCH_OFF_HEAP_MEMORY
> >
> > Setting both to true will make sure that all memory is committed as
> early as possible.
> >
> > Different combinations will allow for all 4 variants you’ve mentioned. I
> can see a use case for each one:
> >
> > - lazy region init + no pre-touch (default) – easier configuration (at
> least for simple use cases without a lot of regions and node filters) and
> faster startup
> >
> > - lazy region init + pre-touch – allows to reuse server config on a
> client (the goal of the IGNITE-9113) but still allows to fail-fast on cache
> creation
> >
> > - eager region init + no pre-touch – for cases when you want to eagerly
> allocate memory but don’t care about committing physical pages; e.g. if you
> have overcommit disabled then you don’t need to pre-touch everything to
> make sure the memory is there
> >
> > - eager region init + pre-touch – to fail as early as possible if there
> is not enough RAM
> >
> >
> >
> > Stan
> >
> >
> >
> > From: Nikolay Izhikov
> > Sent: 14 декабря 2018 г. 14:42
> > To: dev; vololo...@gmail.com; stanlukya...@gmail.com
> > Subject: Re: Pre-touch for Ignite off-heap memory
> >
> >
> >
> > Bump.
> >
> >
> >
> > Stanislav, Ivan, can you answer my questions?
> >
> > Let's discuss these improvements.
> >
> >
> >
> > ср, 12 дек. 2018 г. в 22:14, Павлухин Иван :
> >
> >
> >
> > > Hi Stan,
> >
> > >
> >
> > > As I participated in discussion in current thread I would like to
> >
> > > leave a comment.
> >
> > >
> >
> > > Your concerns looks clear for me and if you believe that pre-touch
> >
> > > will help product users then I have nothing against it.
> >
> > > ср, 12 дек. 2018 г. в 11:09, Nikolay Izhikov :
> >
> > > >
> >
> > > > Hello, Stanislav.
> >
> > > >
> >
> > > > As far as I can see, we have controversal ticket based on previous
> >
> > > dev-list discussion:
> >
> > > >
> >
> > > > IGNITE-9113 - Allocate memory for a data region when first cache
> >
> > > assigned to this region is created [1]
> >
> > > >
> >
> > > > I planned to implement it soon.
> >
> > > > Looks like we should have several options of memory(data regions)
> >
> > > allocation:
> >
> > > >
> >
> > > > - allocate all on startup (AFAIK this is how current
> >
> > > implementation behave)
> >
> > > > - allocate all on startup AND pre touch.
> >
> > > > - allocate specific data region for first assignment.
> >
> > > > - allocate specific data region for first assignment AND pre
> >
> > > touch.
> >
> > > >
> >
> > > > What do you think?
> >
> > > >
> >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-9113
> >
> > > >
> >
> > > > В Вт, 11/12/2018 в 19:39 +0300, Stanislav Lukyanov пишет:
> >
> > > > > Igniters,
> >
> > > > >
> >
> > > > > What is being suggested here is an Ignite off-heap’s version of
> Java’s
> >
> > > -XX:+AlwaysPreTouch.
> >
> > > > > The latter is known to be used to guarantee that the committed
> memory
> >
> > > is backed by physical RAM.
> >
> > > > > This ensures that
> >
> > > > > a) JVM doesn’t have to do it during the actual work (avoiding
> overhead
> >
> > > for physical page allocation, possible contention with page cache, etc)
> >
> > > > > b) JVM fails fast if the Xmx is greater than available RAM
> >
> > > > > c) New processes will not be able to claim the memory JVM took for
> >
> > > itself
> >
> > > > >
> >
> > > > > Currently one can’t get the same benefits for Ignite because we use
> >
> > > off-heap as well as heap.
> >
> > > > > So, we can implement a similar feature for Ignite – and make sure
> the
> >
> > > users can get all the memory pr

[jira] [Created] (IGNITE-10742) Replace java.io.Console to GridConsole

[Sergey Antonov (JIRA)]

Sergey Antonov created IGNITE-10742:
---

 Summary: Replace java.io.Console to  GridConsole
 Key: IGNITE-10742
 URL: https://issues.apache.org/jira/browse/IGNITE-10742
 Project: Ignite
  Issue Type: Improvement
Reporter: Sergey Antonov
Assignee: Sergey Antonov
 Fix For: 2.8


We should create and use GridConsole instead of java.io.Console because of 
using Console in tests impossible. The API of GridConsole must be identical 
Console API.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Service grid redesign

[Denis Mekhanikov]

Guys,

I finished my code review. The pool request looks good to me.

Does anybody else want to look at the changes?
There are a few points, that we didn't meet an agreement on,
though they don't affect the behaviour in any way:

   - *Class naming. * See the discussion above.
   - *Unnecessary task object cleaning. *
   IMO, ServicesDeploymentTask#clear() method doesn't do anything useful,
   and it should be removed.
   By the moment, when this method is called, the task object is removed
   from all collections anyway, so it's ready for garbage collection.
   Removing data from it doesn't help anybody.
   -
*Unnecessary tests. *ServiceInfoSelfTest and
   ServicesDeploymentProcessIdSelfTest look excessive to me.
   I don't see any point in testing an interface implementation, that only
   saves some objects and returns them from certain methods.
   - Interface for events with servicesDeploymentActions() method.
   Take a look at the discussion:
   
https://github.com/apache/ignite/pull/4434/files/30e69d9a53ce6ea16c4e9d15354e94360caa719d#r239442342

Also solution with *DiscoveryCustomEvent#nullifyingCustomMsgLock* looks
clumsy to me.
The problem with nullifying of *DiscoveryCustomEvent#customMsg* field can
be solved
by making *ServiceDiscoveryListener* a high priority listener.

Or *DiscoveryCustomEvent#customMessage()* method could be marked
synchronized and
*GridEventStorageManager#notifyListeners(..)* method could synchronize on
the event object.
But this solution is the same, it's just a matter of taste.

If anybody wants to look the the code of the PR, please consider these
points as well.

Denis

ср, 19 дек. 2018 г. в 17:37, Nikolay Izhikov :

> Denis,
>
> I don't think that differences with your and my naming is huge :)
> And, it's definetely a matter of taste.
>
> If there is no any other issues with PR let's rename and move on! :)
>
> ср, 19 дек. 2018 г. в 17:32, Vyacheslav Daradur :
>
> > > We have IgniteServiceProcessor and GridServiceProcessor with singular
> > "Service"
> >
> > Maybe we should rename new 'IgniteServiceProcessor' to
> > 'IgniteServicesProcessor'?
> >
> > > And ServiceSingleDeploymentsResults name doesn't make sense to me.
> > > "Single deployments" doesn't sound right.
> >
> > 'Single' means 'single node', maybe we should use one of the following:
> > - 'ServicesSingleNodeDeploymentsResults'
> > - 'ServicesNodeDeploymentsResults'
> > - 'ServicesInstanceDeploymentsResults'
> >
> > On Wed, Dec 19, 2018 at 4:26 PM Denis Mekhanikov 
> > wrote:
> > >
> > > Slava,
> > > I think, it's better to replace word "Change" with "Request".
> > >
> > > Nik,
> > > We have IgniteServiceProcessor and GridServiceProcessor with singular
> > > "Service",
> > > ServicesDeploymentManager and ServicesDeploymentTask with plural
> > "Services"
> > > for some reason.
> > > So, you need to remember, where Service and where Services is used.
> > > I think, we should unify these names.
> > > And ServiceSingleDeploymentsResults name doesn't make sense to me.
> > > "Single deployments" doesn't sound right.
> > >
> > > ServicesFullDeploymentsMessage is derived
> > > from GridDhtPartitionsFullMessage.
> > > It doesn't really reflect its function. This message is supposed to
> mark
> > > the point in time, when deployment is finished.
> > >
> > > Denis
> > >
> > >
> > > пт, 14 дек. 2018 г. в 11:30, Vyacheslav Daradur :
> > >
> > > > >*1. Testing of the cache-based implementation of the service grid.*
> > > > > I think, we should make a test suite, that will test the old
> > > > implementation
> > > > > until we remove it from the project.
> > > >
> > > > Agree. This is exactly what should be done as the first step once
> > > > phase 1 will be merged.
> > > > I think all tests in the package:
> > > > "org.apache.ignite.internal.processors.service" should be moved to
> > > > separate test-suite and new build-plan should be added on TC and
> > > > included in RunAll.
> > > >
> > > > > *2. DynamicServiceChangeRequest.*
> > > > > I think, this class should be splat into two.
> > > >
> > > > Personally, I agree, but I have faced opposition at the design step.
> > > > I changed to the following structure:
> > > >
> > > > abstract class ServiceAbstractChange implements Serializable {
> > > > protected final IgniteUuid srvcId;
> > > > }
> > > >
> > > > class ServiceDeploymentChange extends ServiceAbstractChange {
> > > > ServiceConfiguration cfg;
> > > > }
> > > >
> > > > class ServiceUndeploymentChange extends ServiceAbstractChange { }
> > > >
> > > > I hope that further reviewers will agree with us.
> > > >
> > > > > *3. Naming.*
> > > >
> > > > About "Services" -> "Service" and "Deployments" -> "Deployment"
> > > > Personally, I agree with Nikolay, because it's more descriptive since
> > > > manages several services, not single.
> > > > But, I understand Denis's point of view, we have a lot of classes
> with
> > > > "Service" prefix in naming and "Services" looks a bit alien.
> > > >
> > > > > *DynamicServices

Hibernate 5.3 merged!

[Ilya Kasnacheev]

Hello!

I am proud to announce that I have just merged Hibernate 5.3 module,
courtesy Scott Feldstein.

There were also important fixes which should make TC greener than ever.

Since this is a large patch there might be rough edges, please notify me if
anything went awry, I will try to follow it up.

Regards,
-- 
Ilya Kasnacheev

Re: Hibernate 5.3 merged!

[Павлухин Иван]

Good news!

I also believe that a lot of users will be happy once it is released. Cheers!

By the way, I noticed that the latest Hibernate version is 5.4. It is
compatible with 5.3? Should it be somehow addressed by Ignite
developers?
ср, 19 дек. 2018 г. в 19:09, Ilya Kasnacheev :
>
> Hello!
>
> I am proud to announce that I have just merged Hibernate 5.3 module,
> courtesy Scott Feldstein.
>
> There were also important fixes which should make TC greener than ever.
>
> Since this is a large patch there might be rough edges, please notify me if
> anything went awry, I will try to follow it up.
>
> Regards,
> --
> Ilya Kasnacheev



--
Best regards,
Ivan Pavlukhin

Re: Hibernate 5.3 merged!

[Ilya Kasnacheev]

Hello!

I have just checked, and it looks like hibernate-5.3 module works correctly
with Hibernate 5.4.0.Final!

It builds without errors and all tests pass. So maybe we can rebrand it as
hibernate-5.3+ for the duration.

Regards,
-- 
Ilya Kasnacheev


ср, 19 дек. 2018 г. в 19:53, Павлухин Иван :

> Good news!
>
> I also believe that a lot of users will be happy once it is released.
> Cheers!
>
> By the way, I noticed that the latest Hibernate version is 5.4. It is
> compatible with 5.3? Should it be somehow addressed by Ignite
> developers?
> ср, 19 дек. 2018 г. в 19:09, Ilya Kasnacheev :
> >
> > Hello!
> >
> > I am proud to announce that I have just merged Hibernate 5.3 module,
> > courtesy Scott Feldstein.
> >
> > There were also important fixes which should make TC greener than ever.
> >
> > Since this is a large patch there might be rough edges, please notify me
> if
> > anything went awry, I will try to follow it up.
> >
> > Regards,
> > --
> > Ilya Kasnacheev
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

Re: Dealing with changing class definitions in Ignite

[Ilya Kasnacheev]

Hello!

I have found that you can avoid compute job type incompatibility problem
(please see history below) by setting

.setMarshaller(new OptimizedMarshaller())

in your Ignite configuration on all nodes.

However, it is not clear at all why this is needed. Anybody can help? Why
compute jobs are processed by capricious BinaryMarshaller?

Regards,
-- 
Ilya Kasnacheev


ср, 19 дек. 2018 г. в 02:29, Gert Dubois :

> Hey Ilya,
>
> I opened a ticket on the ignite Jira about it.
> https://issues.apache.org/jira/browse/IGNITE-10717
>
> I attached a zip file containing a maven project with sample code that
> reproduces our issue. Reproducing the issue is rather easy though
>
> 1. Have a client + server, with peer class loading enabled
> 2. Create a simple class that implements IgniteRunnable with some class
> field
> 3. Execute an instance of this class on the ignite cluster
> 4. Change the type of the field
> 5. Recompile and execute again. Now it breaks because the class can't be
> serialized using the binary marshaller.
>
> Code to execute these steps is included in the maven project.
>
> On Tue, Dec 18, 2018 at 12:04 PM Ilya Kasnacheev <
> ilya.kasnach...@gmail.com> wrote:
>
>> Hello!
>>
>> Can you show an example of "Ignite Runnables conflicting on the Binary
>> Marshaller"? As a small code snippet perhaps?
>>
>> Maybe I could recommend something but I lack understanding of your use
>> case.
>>
>> Regards,
>> --
>> Ilya Kasnacheev
>>
>>
>> пн, 17 дек. 2018 г. в 18:12, Gert Dubois :
>>
>>> Thanks for the reply.
>>>
>>> Everything related to the cache we understand the current architecture,
>>> in our code base we'll probably treat everything cache related the same as
>>> schema migrations in a DB (migration scripts, etc.).
>>> Our real issue is with Ignite Runnables conflicting on the Binary
>>> Marshaller. Every class that gets executed on Ignite as a job gets a stored
>>> definition, this means we can't refactor classes that get used internally
>>> by our Ignite Runnables, because doing so would prevent the updated code
>>> from running. Even worse, if we update our libraries and they changed class
>>> definitions we might run into the same issue, without changing a letter in
>>> our own code. From the documentation it looked like Deployment Mode could
>>> provide a solution for this issue but the Binary Marshaller seems to run
>>> completely separate from the Deployment Mode.
>>>
>>> On Mon, Dec 17, 2018 at 3:18 PM Ilya Kasnacheev <
>>> ilya.kasnach...@gmail.com> wrote:
>>>
 Hello!

 As far as my understanding goes:
 You can't peer class load your Key/Value types.
 You also can't redeploy your Key/Value types.
 They even survive node restart via WORKDIR/marshaller directory, and
 come back to haunt you.

 There are plans to maybe ease some of those limitations in 3.0, but
 nothing concrete yet. It is not a bug rather a pillar of current Ignite
 architecture. You will have to route around it, such as introducing new
 fields instead of changing types. And maybe avoid having those types on
 server nodes at all, and relying on BinaryObject.

 Regards,
 --
 Ilya Kasnacheev


 пн, 17 дек. 2018 г. в 15:38, Gert Dubois :

> The issue is still present in 2.7.
> I added a ticket on Jira with sample code that reproduces the issue.
>
> https://issues.apache.org/jira/browse/IGNITE-10717
>
> For now I think we can work around the issue by overriding the default
> BinaryNameMapper, but this feels rather hacky to me.
>
> On Mon, Dec 17, 2018 at 11:54 AM Denis Mekhanikov <
> dmekhani...@gmail.com> wrote:
>
>> Gert,
>>
>> Could you check if the problem with a deployment mode reproduces on
>> Ignite 2.7?
>> If it does, please file a ticket with an explanation and a reproducer
>> to https://issues.apache.org/jira/
>>
>> Thanks!
>> Denis
>>
>>
>> пн, 17 дек. 2018 г. в 12:12, Gert Dubois > >:
>>
>>> I investigated the issue further and narrowed the issue down to the
>>> Binary Marshaller not working as expected given the configured 
>>> Deployment
>>> Mode. When forcing my clients to use unique class names in the metadata 
>>> of
>>> the Binary Marshaller (I forced this by overriding the global
>>> BinaryNameMapper and appending a per-client unique suffix to every class
>>> name) the Deployment Mode behaves as expected. I assume the Binary
>>> Marshaller keeps a cluster wide state of the Metadata of classes and it
>>> merges it whenever we serialize a class on a node (regardless of the
>>> configured Deployment Mode).
>>> Why is the behaviour of the Binary Marshaller not consistent with
>>> the way the Deployment Mode works? Is there a cleaner way to solve this,
>>> besides overriding the BinaryNameMapper?
>>>
>>> On Fri, Dec 14, 2018 at 1:07 PM Gert Dubois <
>>> g

[jira] [Created] (IGNITE-10743) MVCC: Mute flaky mvcc tests

[Roman Kondakov (JIRA)]

Roman Kondakov created IGNITE-10743:
---

 Summary: MVCC: Mute flaky mvcc tests
 Key: IGNITE-10743
 URL: https://issues.apache.org/jira/browse/IGNITE-10743
 Project: Ignite
  Issue Type: Task
  Components: mvcc
Reporter: Roman Kondakov
Assignee: Roman Kondakov


We should mute all flaky MVCC tests on TC with appropriate tickets links. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

RE: Volunteer needed: AWS Elastic Load Balancer IP Findersimplemented

[Stanislav Lukyanov]

Hi,

Regarding the name - a not-so-good name isn’t always a sufficient justification 
for renaming.
Public products such as Ignite have to also take into account convenience for 
existing users.
Even in 3.0 when we technically can break compatibility I would avoid breaking 
it just to
rename “ELB” to “Classic ELB”.

Also, I believe the official names Amazon uses for these technologies are ELB 
and ALB,
not “classic ELB” and “application ELB”.

Regarding the code - `extends TcpDiscoveryClassicElbIpFinder` doesn’t really 
solve it.
For example, now you have an unused loadBalancerName property in the 
TcpDiscoveryApplicationElbIpFinder.

I guess, to move this forward, let’s just add a new class, 
TcpDiscoveryAlbIpFinder.
It doesn’t have to extend the existing TcpDiscoveryElbIpFinder – maybe we could 
merge them, but let’s not be 
stuck on this any longer.
Also let’s not change the existing TcpDiscoveryElbIpFinder – no need to rename 
it or deprecate it. We can thing of the
two IpFinders as of just independent features.

Thanks,
Stan

From: uday kale
Sent: 9 октября 2018 г. 19:34
To: dev@ignite.apache.org
Subject: Re: Volunteer needed: AWS Elastic Load Balancer IP Findersimplemented

Hi Stanislav,

I have removed extended the TcpDiscoveryApplicationElbIpFinder from
TcpDiscoveryClassicElbIpFinder to limi the code duplication. Please share
your feedback.

Thanks,
Uday

On Wed, Oct 3, 2018 at 1:12 AM Dmitriy Pavlov  wrote:

> Hi Uday,
>
> We should keep classes name as is within all minor releases (2.x). We can
> schedule rename only to 3.0. We need to keep both classes and methods
> naming as is to provide compilation guarantee. If someone used existing
> TcpDiscoveryElbIpFinder from 2.6 release than his or her code should
> compile and run well.
>
> I've updated checklist, thank you for pointing to this.
>
> So I suggest keeping existing naming of TcpDiscoveryElbIpFinder as it is
> part of API.
>
> Sincerely,
> Dmitriy Pavlov
>
> вт, 2 окт. 2018 г. в 21:04, uday kale :
>
> > Thanks for the review Stan.
> > I renamed the existing class TcpDiscoveryElbIpFinder since the name is
> not
> > clear regarding the actual load balancer being used. The names
> > TcpDiscoveryClassicElbIpFinder
> > and TcpDiscoveryApplicationElbIpFinder provide a clear distinction.
> > I deprecated the existing class because of review checklist [1] point 1.a
> > under checklist. It requires methods to be deprecated instead of being
> > renamed. I assumed this will extend to classes too.
> > Regarding the your suggestion for having the tests I agree. It will be
> > helpful to know whether TC can accept properties while initiating a run.
> >
> > [1] https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist
> >
> > Uday
> >
> > On Tue, Oct 2, 2018 at 9:58 PM Stanislav Lukyanov <
> stanlukya...@gmail.com>
> > wrote:
> >
> > > Also, it makes me nervous that we’re lacking any sort of functional
> tests
> > > for AWS-based IP finders (same for GCE, actually).
> > > I understand that it’s hard to include tests like that in regular TC
> runs
> > > because we’d have to include some sort of credentials.
> > > But let’s think of some sort of a middle ground.
> > >
> > > I’m thinking about a functional test suite in the ignite-aws module
> that
> > > would pick up a properties file with AWS credentials.
> > > It wouldn’t be included in any of the TC runs, but someone making
> changes
> > > to ignite-aws would be able to run it locally providing their own
> > > credentials.
> > > They’d then share the results of their testing together with the usual
> TC
> > > link.
> > >
> > > Stan
> > >
> > > From: Stanislav Lukyanov
> > > Sent: 2 октября 2018 г. 19:08
> > > To: dev@ignite.apache.org
> > > Subject: RE: Volunteer needed: AWS Elastic Load Balancer IP
> > > Findersimplemented
> > >
> > > Hi,
> > >
> > > I took a look at the code, and I believe the patch needs to be
> enhanced.
> > >
> > > The patch is about adding TcpDiscoveryApplicationElbIpFinder, but it
> also
> > > deprecates the existing TcpDiscoveryElbIpFinder
> > > and replaces it with its copy named TcpDiscoveryClassicElbIpFinder.
> > >
> > > I’d really prefer if the existing class were enhanced to support both
> > > classic ELB and Application ELB.
> > > If it can’t be done, let’s  use inheritance to reduce the copypaste.
> > > In any case, let’s avoid deprecating the existing class – I don’t think
> > > that changing the name is that important in this case.
> > >
> > > Thanks,
> > > Stan
> > >
> > > From: Denis Magda
> > > Sent: 26 сентября 2018 г. 18:52
> > > To: dev
> > > Subject: Re: Volunteer needed: AWS Elastic Load Balancer IP
> > > Findersimplemented
> > >
> > > Igniters,
> > >
> > > Don't we have experts in our networking component? I do believe that's
> a
> > > small improvement that can be verified and merged quickly.
> > >
> > > --
> > > Denis
> > >
> > > On Wed, Sep 26, 2018 at 8:50 AM Dmitriy Pavlov 
> > > wrote:
> > >
> > > > Igniters, Friendly reminder.
> > > >
> > > > D

[jira] [Created] (IGNITE-10744) class org.apache.ignite.IgniteCheckedException: Failed to read WAL record at position: 5237375 size: -1

[ARomantsov (JIRA)]

ARomantsov created IGNITE-10744:
---

 Summary: class org.apache.ignite.IgniteCheckedException: Failed to 
read WAL record at position: 5237375 size: -1
 Key: IGNITE-10744
 URL: https://issues.apache.org/jira/browse/IGNITE-10744
 Project: Ignite
  Issue Type: Bug
  Components: data structures
Affects Versions: 2.8
Reporter: ARomantsov
 Fix For: 2.8


Scenario:

-Start 4 nodes with disc storage
-Little load
-Deactivate
-Activate

Got next message in log


{code:java}
[20:54:42,746][INFO][wal-file-compressor-%null%-3-#70][FileWriteAheadLogManager]
 Stopping WAL iteration due to an exception: Failed to read WAL record at 
position: 5237375 size: -1, ptr=FileWALPointer [idx=4, fileOff=5237375, len=0]
[20:54:42,747][SEVERE][wal-file-compressor-%null%-3-#70][FileWriteAheadLogManager]
 Compression of WAL segment [idx=4] was skipped due to unexpected error
class org.apache.ignite.IgniteCheckedException: Failed to read WAL record at 
position: 5237375 size: -1
at 
org.apache.ignite.internal.processors.cache.persistence.wal.AbstractWalRecordsIterator.handleRecordException(AbstractWalRecordsIterator.java:294)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.AbstractWalRecordsIterator.advanceRecord(AbstractWalRecordsIterator.java:258)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.AbstractWalRecordsIterator.advance(AbstractWalRecordsIterator.java:154)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.SingleSegmentLogicalRecordsIterator.advance(SingleSegmentLogicalRecordsIterator.java:119)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.AbstractWalRecordsIterator.onNext(AbstractWalRecordsIterator.java:123)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.AbstractWalRecordsIterator.onNext(AbstractWalRecordsIterator.java:52)
at 
org.apache.ignite.internal.util.GridCloseableIteratorAdapter.nextX(GridCloseableIteratorAdapter.java:41)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.FileWriteAheadLogManager$FileCompressorWorker.compressSegmentToFile(FileWriteAheadLogManager.java:2186)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.FileWriteAheadLogManager$FileCompressorWorker.body0(FileWriteAheadLogManager.java:2111)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.FileWriteAheadLogManager$FileCompressorWorker.body(FileWriteAheadLogManager.java:2081)
at 
org.apache.ignite.internal.util.worker.GridWorker.run(GridWorker.java:120)
at java.lang.Thread.run(Thread.java:748)
Caused by: class org.apache.ignite.IgniteCheckedException: Failed to read WAL 
record at position: 5237375 size: -1
at 
org.apache.ignite.internal.processors.cache.persistence.wal.serializer.RecordV1Serializer.readWithCrc(RecordV1Serializer.java:394)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.serializer.RecordV2Serializer.readRecord(RecordV2Serializer.java:235)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.AbstractWalRecordsIterator.advanceRecord(AbstractWalRecordsIterator.java:243)
... 10 more
Caused by: java.nio.channels.ClosedByInterruptException
at 
java.nio.channels.spi.AbstractInterruptibleChannel.end(AbstractInterruptibleChannel.java:202)
at sun.nio.ch.FileChannelImpl.read(FileChannelImpl.java:164)
at 
org.apache.ignite.internal.processors.cache.persistence.file.RandomAccessFileIO.read(RandomAccessFileIO.java:58)
at 
org.apache.ignite.internal.processors.cache.persistence.file.FileIODecorator.read(FileIODecorator.java:51)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.io.SimpleFileInput.ensure(SimpleFileInput.java:119)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.io.FileInput$Crc32CheckingFileInput.ensure(FileInput.java:89)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.io.FileInput$Crc32CheckingFileInput.skipBytes(FileInput.java:130)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.serializer.RecordV2Serializer$2.readWithHeaders(RecordV2Serializer.java:130)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.serializer.RecordV1Serializer.readWithCrc(RecordV1Serializer.java:373)
... 12 more
Suppressed: class 
org.apache.ignite.internal.processors.cache.persistence.wal.crc.IgniteDataIntegrityViolationException:
 val: 505117685 writtenCrc: 0
at 
org.apache.ignite.internal.processors.cache.persistence.wal.io.FileInput$Crc32CheckingFileInput.close(FileInput.java:106)
at 
org.apache.ignite.internal.processors.cache.persistence.wal.serializer.RecordV1Serializer.readWithCrc(RecordV1Serializer.java:380)
... 12 more
{code}




--
This 

[jira] [Created] (IGNITE-10745) SQL: jdbc metadata's getColumns returns wrong value for "ORDINAL_POSITION"

[Pavel Kuznetsov (JIRA)]

Pavel Kuznetsov created IGNITE-10745:


 Summary: SQL: jdbc metadata's getColumns returns wrong value for 
"ORDINAL_POSITION" 
 Key: IGNITE-10745
 URL: https://issues.apache.org/jira/browse/IGNITE-10745
 Project: Ignite
  Issue Type: Bug
  Components: jdbc
Reporter: Pavel Kuznetsov


Affected both thin and jdbc v2 drivers.
jdbc spec says : 
{noformat}
ORDINAL_POSITION int => index of column in table (starting at 1)
{noformat}

but in fact it is a position in the metadata table itself, not position in the 
original table. 
For example we have table 
{code:sql}
Person(id int primary key, val1 int, val2 bigint, val3 int)
{code:sql}

Oridinal number for {{val3}} is 4, but if we specified patterns that leave only 
1 result  ({{PUBLIC.Person.val3}}) returned value will be 1. If we select 2 
tables - 2 or 1 and so on.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Hibernate 5.3 merged!

[Scott Feldstein]

Hi Ilya, that’s great news!

What’s the likelihood of pushing the artifacts to a public maven repo along
with the other ignite artifacts? Are we constrained by the LGPL2 license?
If so, I wonder how spring works around that with their jpa impl?

Scott
On Wed, Dec 19, 2018 at 09:03 Ilya Kasnacheev 
wrote:

> Hello!
>
> I have just checked, and it looks like hibernate-5.3 module works correctly
> with Hibernate 5.4.0.Final!
>
> It builds without errors and all tests pass. So maybe we can rebrand it as
> hibernate-5.3+ for the duration.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 19 дек. 2018 г. в 19:53, Павлухин Иван :
>
> > Good news!
> >
> > I also believe that a lot of users will be happy once it is released.
> > Cheers!
> >
> > By the way, I noticed that the latest Hibernate version is 5.4. It is
> > compatible with 5.3? Should it be somehow addressed by Ignite
> > developers?
> > ср, 19 дек. 2018 г. в 19:09, Ilya Kasnacheev  >:
> > >
> > > Hello!
> > >
> > > I am proud to announce that I have just merged Hibernate 5.3 module,
> > > courtesy Scott Feldstein.
> > >
> > > There were also important fixes which should make TC greener than ever.
> > >
> > > Since this is a large patch there might be rough edges, please notify
> me
> > if
> > > anything went awry, I will try to follow it up.
> > >
> > > Regards,
> > > --
> > > Ilya Kasnacheev
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
> >
>

Re: Service grid redesign

[Nikolay Izhikov]

Denis, great news!

Alexey, Vova, Yakov, do you want to take a look at this PR?



В Ср, 19/12/2018 в 18:47 +0300, Denis Mekhanikov пишет:
> Guys,
> 
> I finished my code review. The pool request looks good to me.
> 
> Does anybody else want to look at the changes?
> There are a few points, that we didn't meet an agreement on,
> though they don't affect the behaviour in any way:
> 
>- *Class naming. * See the discussion above.
>- *Unnecessary task object cleaning. *
>IMO, ServicesDeploymentTask#clear() method doesn't do anything useful,
>and it should be removed.
>By the moment, when this method is called, the task object is removed
>from all collections anyway, so it's ready for garbage collection.
>Removing data from it doesn't help anybody.
>-
> *Unnecessary tests. *ServiceInfoSelfTest and
>ServicesDeploymentProcessIdSelfTest look excessive to me.
>I don't see any point in testing an interface implementation, that only
>saves some objects and returns them from certain methods.
>- Interface for events with servicesDeploymentActions() method.
>Take a look at the discussion:
>
> https://github.com/apache/ignite/pull/4434/files/30e69d9a53ce6ea16c4e9d15354e94360caa719d#r239442342
> 
> Also solution with *DiscoveryCustomEvent#nullifyingCustomMsgLock* looks
> clumsy to me.
> The problem with nullifying of *DiscoveryCustomEvent#customMsg* field can
> be solved
> by making *ServiceDiscoveryListener* a high priority listener.
> 
> Or *DiscoveryCustomEvent#customMessage()* method could be marked
> synchronized and
> *GridEventStorageManager#notifyListeners(..)* method could synchronize on
> the event object.
> But this solution is the same, it's just a matter of taste.
> 
> If anybody wants to look the the code of the PR, please consider these
> points as well.
> 
> Denis
> 
> ср, 19 дек. 2018 г. в 17:37, Nikolay Izhikov :
> 
> > Denis,
> > 
> > I don't think that differences with your and my naming is huge :)
> > And, it's definetely a matter of taste.
> > 
> > If there is no any other issues with PR let's rename and move on! :)
> > 
> > ср, 19 дек. 2018 г. в 17:32, Vyacheslav Daradur :
> > 
> > > > We have IgniteServiceProcessor and GridServiceProcessor with singular
> > > 
> > > "Service"
> > > 
> > > Maybe we should rename new 'IgniteServiceProcessor' to
> > > 'IgniteServicesProcessor'?
> > > 
> > > > And ServiceSingleDeploymentsResults name doesn't make sense to me.
> > > > "Single deployments" doesn't sound right.
> > > 
> > > 'Single' means 'single node', maybe we should use one of the following:
> > > - 'ServicesSingleNodeDeploymentsResults'
> > > - 'ServicesNodeDeploymentsResults'
> > > - 'ServicesInstanceDeploymentsResults'
> > > 
> > > On Wed, Dec 19, 2018 at 4:26 PM Denis Mekhanikov 
> > > wrote:
> > > > 
> > > > Slava,
> > > > I think, it's better to replace word "Change" with "Request".
> > > > 
> > > > Nik,
> > > > We have IgniteServiceProcessor and GridServiceProcessor with singular
> > > > "Service",
> > > > ServicesDeploymentManager and ServicesDeploymentTask with plural
> > > 
> > > "Services"
> > > > for some reason.
> > > > So, you need to remember, where Service and where Services is used.
> > > > I think, we should unify these names.
> > > > And ServiceSingleDeploymentsResults name doesn't make sense to me.
> > > > "Single deployments" doesn't sound right.
> > > > 
> > > > ServicesFullDeploymentsMessage is derived
> > > > from GridDhtPartitionsFullMessage.
> > > > It doesn't really reflect its function. This message is supposed to
> > 
> > mark
> > > > the point in time, when deployment is finished.
> > > > 
> > > > Denis
> > > > 
> > > > 
> > > > пт, 14 дек. 2018 г. в 11:30, Vyacheslav Daradur :
> > > > 
> > > > > > *1. Testing of the cache-based implementation of the service grid.*
> > > > > > I think, we should make a test suite, that will test the old
> > > > > 
> > > > > implementation
> > > > > > until we remove it from the project.
> > > > > 
> > > > > Agree. This is exactly what should be done as the first step once
> > > > > phase 1 will be merged.
> > > > > I think all tests in the package:
> > > > > "org.apache.ignite.internal.processors.service" should be moved to
> > > > > separate test-suite and new build-plan should be added on TC and
> > > > > included in RunAll.
> > > > > 
> > > > > > *2. DynamicServiceChangeRequest.*
> > > > > > I think, this class should be splat into two.
> > > > > 
> > > > > Personally, I agree, but I have faced opposition at the design step.
> > > > > I changed to the following structure:
> > > > > 
> > > > > abstract class ServiceAbstractChange implements Serializable {
> > > > > protected final IgniteUuid srvcId;
> > > > > }
> > > > > 
> > > > > class ServiceDeploymentChange extends ServiceAbstractChange {
> > > > > ServiceConfiguration cfg;
> > > > > }
> > > > > 
> > > > > class ServiceUndeploymentChange extends ServiceAbstractChange { }
> > > > > 
> > > > > I hope that further reviewers