ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitry Pavlov <dpavlov....@gmail.com>
Subject Re: IGNITE-5123 Review
Date Sun, 23 Jul 2017 18:21:39 GMT
Hi Evgeniy,

>From my point of view there are no problems with this fix. My testing
didn't show any issues with fix.

Igniters,

Are there any additional comments on this issue? Can we proceed?

Sincerely,
Dmitriy Pavlov

чт, 20 июл. 2017 г. в 20:04, Dmitry Pavlov <dpavlov.spb@gmail.com>:

> Hi Evgeniy,
>
> Thank you for such a careful research of the issue.
>
> If don’t mind, I would like to do additional tests with this PR changes.
>
> I will come back with result in couple of days
>
> Sincerely,
> Dmitriy Pavlov
>
> чт, 20 июл. 2017 г. в 19:18, Evgeniy Ignatiev <yevgeniy.ignatyev@gmail.com
> >:
>
>> onIgniteStart was called in Ignite 1.X in
>> GridPluginComponent#onKernalStart as one of the calls to the component
>> callbacks, probably the order in which components were called, ensured
>> that contract of PluginProvider#onIgniteStart was not violated. But in
>> 2.0 the GridPluginComponent instances are explicitly skipped from this
>> cycle (lines 1019-1020 in Ignite 2.0 release source) and PluginProviders
>> are notified before the internal component callbacks.
>>
>> As far as I can see the change, that moved PluginProvider#onIgniteStart
>> notification before component callbacks, was introduced by this commit -
>>
>> https://github.com/apache/ignite/commit/6b7bf97158c097b80bcf5c2150e67a5210269e6d
>> - but I have no clue what was the reason.
>>
>>
>> On 7/20/2017 7:51 PM, Dmitry Pavlov wrote:
>> > Hi Nick,
>> >
>> > Thank you for your comment. Was onIgniteStart called after
>> onKernalStart in
>> > 1.9? Or caches were available, but other of initialization was the same?
>> >
>> > Sincerely.
>> > Dmitriy Pavlov
>> >
>> > ср, 19 июл. 2017 г. в 17:06, Nick Pordash <nickpordash@gmail.com>:
>> >
>> >> Hi Dmitriy,
>> >>
>> >> The ticket was a regression from 1.9 to 2.0. I don't think anyone
>> would be
>> >> expecting the behavior in 2.0 as it doesn't align with the javadoc and
>> has
>> >> only been broken since the 2.0 release.
>> >>
>> >> -Nick
>> >>
>> >> On Wed, Jul 19, 2017, 6:55 AM Dmitry Pavlov <dpavlov.spb@gmail.com>
>> wrote:
>> >>
>> >>> Hi Evgeniy,
>> >>>
>> >>> Thank you. Ignite Basic is one suite from approximately 80 suites that
>> >>> covers Ignite by automated tests. Which is why I suggested to use
>> RunAll
>> >>> chain in ignite 2.0 group. Yes, several tests may fail, especially if
>> it
>> >> is
>> >>> flaky tests or failure is related to the specific JIRA issue.
>> >>>
>> >>> About change itself: This change seems to be very impact. There is
>> >>> possiblity that many of existing plugins relies on existing order of
>> >>> initialization. This change may break plugin initialization in
>> unexpected
>> >>> manner.
>> >>>
>> >>> Could we
>> >>> - fix javadoc according to existing order in code
>> >>> - find out new solution?
>> >>>
>> >>> Sincerely,
>> >>> Dmitriy Pavlov
>> >>>
>> >>>
>> >>> ср, 19 июл. 2017 г. в 16:40, Evgeniy Ignatiev <
>> >> yevgeniy.ignatyev@gmail.com
>> >>>> :
>> >>>>
>> >>
>> http://ci.ignite.apache.org/viewLog.html?buildId=720722&tab=buildResultsDiv&buildTypeId=IgniteTests_IgniteBasic
>> >>>> - this one - there seem to be no new failed platform tests, other
>> >> failed
>> >>>> tests seem to fail in several other reviews too and are unrelated
to
>> my
>> >>>> changes.
>> >>>>
>> >>>>
>> >>>> On 19.07.2017 17:35, Dmitry Pavlov wrote:
>> >>>>> Hi Evgeniy,
>> >>>>>
>> >>>>> I was not able to find Teamcity run for this change.
>> >>>>> Could you please run http://ci.ignite.apache.org test for example
>> on
>> >>>> branch
>> >>>>> pull/2285/head using 'Ignite 2.0 Tests' target 'Run All'.
>> >>>>> Or could you please share link to previous run on this changes?
>> >>>>>
>> >>>>> Sincerely,
>> >>>>> Dmitriy Pavlov
>> >>>>>
>> >>>>> ср, 19 июл. 2017 г. в 15:26, Anton Vinogradov <av@apache.org>:
>> >>>>>
>> >>>>>> Igniters,
>> >>>>>>
>> >>>>>> Could somebody review the fix today?
>> >>>>>>
>> >>>>>> On Wed, Jul 19, 2017 at 1:30 PM, Evgeniy Ignatiev <
>> >>>>>> yevgeniy.ignatyev@gmail.com> wrote:
>> >>>>>>
>> >>>>>>> Hello, Igniters.
>> >>>>>>>
>> >>>>>>> Could anyone review my request - https://issues.apache.org/jira
>> >>>>>>> /browse/IGNITE-5123? - My previous pings seems to got
lost.
>> >>>>>>>
>> >>>>>>> Best regards,
>> >>>>>>>
>> >>>>>>> Yevgeniy
>> >>>>>>>
>> >>>>>>>
>> >>>>
>>
>>

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