IGNITE-4487 - NPE on query execution

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

IGNITE-4487 - NPE on query execution

Александр Меньшиков
Hello everyone.

I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487
<https://issues.apache.org/jira/browse/IGNITE-4487>* as my
first issue.

Please add me as contributor.

I already found that: in inner class
'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in constructor is
called with method 'init()', but method 'init()' cannot be called with an
empty field 'nodes'. In source code it looks like:

private ScanQueryFallbackClosableIterator(int part, GridCacheQueryAdapter
qry,
            GridCacheQueryManager qryMgr, GridCacheContext cctx) {
            this.qry = qry;
            this.qryMgr = qryMgr;
            this.cctx = cctx;
            this.part = part;

            nodes = fallbacks(cctx.discovery().topologyVersionEx());
            // !!! Here nodes.isEmpty()==true, and init() will fail in the
future. !!!
            init();
        }

I can fix it by adding some check in code, but i must know what behavior
are best in this case? As I understand it, the list of nodes is empty if
there are no nodes with the current partition, which means data loss, and
either need to return a meaningful exception, or ignore this situation. But
maybe I missed something.
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

dmagda
Alexander,

I need to know your JIRA ID in order to add you to the contributors list.

As for your questions, this situation might be caused by the race when a cache is being stopped and there are still scan queries running in parallel. So, in general it’s not about data loss.

Sam, Alex G., could you share your thoughts in regards to the proper fix?


Denis
 

> On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <[hidden email]> wrote:
>
> Hello everyone.
>
> I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487
> <https://issues.apache.org/jira/browse/IGNITE-4487>* as my
> first issue.
>
> Please add me as contributor.
>
> I already found that: in inner class
> 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in constructor is
> called with method 'init()', but method 'init()' cannot be called with an
> empty field 'nodes'. In source code it looks like:
>
> private ScanQueryFallbackClosableIterator(int part, GridCacheQueryAdapter
> qry,
>            GridCacheQueryManager qryMgr, GridCacheContext cctx) {
>            this.qry = qry;
>            this.qryMgr = qryMgr;
>            this.cctx = cctx;
>            this.part = part;
>
>            nodes = fallbacks(cctx.discovery().topologyVersionEx());
>            // !!! Here nodes.isEmpty()==true, and init() will fail in the
> future. !!!
>            init();
>        }
>
> I can fix it by adding some check in code, but i must know what behavior
> are best in this case? As I understand it, the list of nodes is empty if
> there are no nodes with the current partition, which means data loss, and
> either need to return a meaningful exception, or ignore this situation. But
> maybe I missed something.

Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

Александр Меньшиков
Username: sharpler

Full Name: Alexander Menshikov



2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email]>:

> Alexander,
>
> I need to know your JIRA ID in order to add you to the contributors list.
>
> As for your questions, this situation might be caused by the race when a
> cache is being stopped and there are still scan queries running in
> parallel. So, in general it’s not about data loss.
>
> Sam, Alex G., could you share your thoughts in regards to the proper fix?
>
> —
> Denis
>
> > On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <[hidden email]>
> wrote:
> >
> > Hello everyone.
> >
> > I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487
> > <https://issues.apache.org/jira/browse/IGNITE-4487>* as my
> > first issue.
> >
> > Please add me as contributor.
> >
> > I already found that: in inner class
> > 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
> constructor is
> > called with method 'init()', but method 'init()' cannot be called with an
> > empty field 'nodes'. In source code it looks like:
> >
> > private ScanQueryFallbackClosableIterator(int part,
> GridCacheQueryAdapter
> > qry,
> >            GridCacheQueryManager qryMgr, GridCacheContext cctx) {
> >            this.qry = qry;
> >            this.qryMgr = qryMgr;
> >            this.cctx = cctx;
> >            this.part = part;
> >
> >            nodes = fallbacks(cctx.discovery().topologyVersionEx());
> >            // !!! Here nodes.isEmpty()==true, and init() will fail in the
> > future. !!!
> >            init();
> >        }
> >
> > I can fix it by adding some check in code, but i must know what behavior
> > are best in this case? As I understand it, the list of nodes is empty if
> > there are no nodes with the current partition, which means data loss, and
> > either need to return a meaningful exception, or ignore this situation.
> But
> > maybe I missed something.
>
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

dmagda
Alexander, added you to the contributors list. Please check that you can assign the ticket on yourself.


Denis

> On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <[hidden email]> wrote:
>
>
> Username: sharpler
>
> Full Name: Alexander Menshikov
>
>
>
> 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:[hidden email]>>:
> Alexander,
>
> I need to know your JIRA ID in order to add you to the contributors list.
>
> As for your questions, this situation might be caused by the race when a cache is being stopped and there are still scan queries running in parallel. So, in general it’s not about data loss.
>
> Sam, Alex G., could you share your thoughts in regards to the proper fix?
>
> —
> Denis
>
> > On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <[hidden email] <mailto:[hidden email]>> wrote:
> >
> > Hello everyone.
> >
> > I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487 <https://issues.apache.org/jira/browse/IGNITE-4487>
> > <https://issues.apache.org/jira/browse/IGNITE-4487 <https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
> > first issue.
> >
> > Please add me as contributor.
> >
> > I already found that: in inner class
> > 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in constructor is
> > called with method 'init()', but method 'init()' cannot be called with an
> > empty field 'nodes'. In source code it looks like:
> >
> > private ScanQueryFallbackClosableIterator(int part, GridCacheQueryAdapter
> > qry,
> >            GridCacheQueryManager qryMgr, GridCacheContext cctx) {
> >            this.qry = qry;
> >            this.qryMgr = qryMgr;
> >            this.cctx = cctx;
> >            this.part = part;
> >
> >            nodes = fallbacks(cctx.discovery().topologyVersionEx());
> >            // !!! Here nodes.isEmpty()==true, and init() will fail in the
> > future. !!!
> >            init();
> >        }
> >
> > I can fix it by adding some check in code, but i must know what behavior
> > are best in this case? As I understand it, the list of nodes is empty if
> > there are no nodes with the current partition, which means data loss, and
> > either need to return a meaningful exception, or ignore this situation. But
> > maybe I missed something.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

Alexey Goncharuk
I think that If fallbacks(...) returns an empty nodes collection, then we
should fail with an exception.

2016-12-28 22:06 GMT+03:00 Denis Magda <[hidden email]>:

> Alexander, added you to the contributors list. Please check that you can
> assign the ticket on yourself.
>
> —
> Denis
>
> > On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <[hidden email]>
> wrote:
> >
> >
> > Username: sharpler
> >
> > Full Name: Alexander Menshikov
> >
> >
> >
> > 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:
> [hidden email]>>:
> > Alexander,
> >
> > I need to know your JIRA ID in order to add you to the contributors list.
> >
> > As for your questions, this situation might be caused by the race when a
> cache is being stopped and there are still scan queries running in
> parallel. So, in general it’s not about data loss.
> >
> > Sam, Alex G., could you share your thoughts in regards to the proper fix?
> >
> > —
> > Denis
> >
> > > On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <[hidden email]
> <mailto:[hidden email]>> wrote:
> > >
> > > Hello everyone.
> > >
> > > I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487 <
> https://issues.apache.org/jira/browse/IGNITE-4487>
> > > <https://issues.apache.org/jira/browse/IGNITE-4487 <
> https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
> > > first issue.
> > >
> > > Please add me as contributor.
> > >
> > > I already found that: in inner class
> > > 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
> constructor is
> > > called with method 'init()', but method 'init()' cannot be called with
> an
> > > empty field 'nodes'. In source code it looks like:
> > >
> > > private ScanQueryFallbackClosableIterator(int part,
> GridCacheQueryAdapter
> > > qry,
> > >            GridCacheQueryManager qryMgr, GridCacheContext cctx) {
> > >            this.qry = qry;
> > >            this.qryMgr = qryMgr;
> > >            this.cctx = cctx;
> > >            this.part = part;
> > >
> > >            nodes = fallbacks(cctx.discovery().topologyVersionEx());
> > >            // !!! Here nodes.isEmpty()==true, and init() will fail in
> the
> > > future. !!!
> > >            init();
> > >        }
> > >
> > > I can fix it by adding some check in code, but i must know what
> behavior
> > > are best in this case? As I understand it, the list of nodes is empty
> if
> > > there are no nodes with the current partition, which means data loss,
> and
> > > either need to return a meaningful exception, or ignore this
> situation. But
> > > maybe I missed something.
> >
> >
>
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

Александр Меньшиков
Alexey, I'm already make pull request where throw exception in that place.

https://github.com/apache/ignite/pull/1388/commits

2016-12-29 11:16 GMT+03:00 Alexey Goncharuk <[hidden email]>:

> I think that If fallbacks(...) returns an empty nodes collection, then we
> should fail with an exception.
>
> 2016-12-28 22:06 GMT+03:00 Denis Magda <[hidden email]>:
>
> > Alexander, added you to the contributors list. Please check that you can
> > assign the ticket on yourself.
> >
> > —
> > Denis
> >
> > > On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <[hidden email]
> >
> > wrote:
> > >
> > >
> > > Username: sharpler
> > >
> > > Full Name: Alexander Menshikov
> > >
> > >
> > >
> > > 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:
> > [hidden email]>>:
> > > Alexander,
> > >
> > > I need to know your JIRA ID in order to add you to the contributors
> list.
> > >
> > > As for your questions, this situation might be caused by the race when
> a
> > cache is being stopped and there are still scan queries running in
> > parallel. So, in general it’s not about data loss.
> > >
> > > Sam, Alex G., could you share your thoughts in regards to the proper
> fix?
> > >
> > > —
> > > Denis
> > >
> > > > On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <
> [hidden email]
> > <mailto:[hidden email]>> wrote:
> > > >
> > > > Hello everyone.
> > > >
> > > > I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487
> <
> > https://issues.apache.org/jira/browse/IGNITE-4487>
> > > > <https://issues.apache.org/jira/browse/IGNITE-4487 <
> > https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
> > > > first issue.
> > > >
> > > > Please add me as contributor.
> > > >
> > > > I already found that: in inner class
> > > > 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
> > constructor is
> > > > called with method 'init()', but method 'init()' cannot be called
> with
> > an
> > > > empty field 'nodes'. In source code it looks like:
> > > >
> > > > private ScanQueryFallbackClosableIterator(int part,
> > GridCacheQueryAdapter
> > > > qry,
> > > >            GridCacheQueryManager qryMgr, GridCacheContext cctx) {
> > > >            this.qry = qry;
> > > >            this.qryMgr = qryMgr;
> > > >            this.cctx = cctx;
> > > >            this.part = part;
> > > >
> > > >            nodes = fallbacks(cctx.discovery().topologyVersionEx());
> > > >            // !!! Here nodes.isEmpty()==true, and init() will fail in
> > the
> > > > future. !!!
> > > >            init();
> > > >        }
> > > >
> > > > I can fix it by adding some check in code, but i must know what
> > behavior
> > > > are best in this case? As I understand it, the list of nodes is empty
> > if
> > > > there are no nodes with the current partition, which means data loss,
> > and
> > > > either need to return a meaningful exception, or ignore this
> > situation. But
> > > > maybe I missed something.
> > >
> > >
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

dmagda
Alexander, thanks.

Please move the ticket from “open” into “patch available” state in JIRA and run the tests on TeamCity. Refer to the details covered there
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-1.CreateGitHubpull-request <https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-1.CreateGitHubpull-request>


Denis

> On Dec 29, 2016, at 3:45 AM, Александр Меньшиков <[hidden email]> wrote:
>
> Alexey, I'm already make pull request where throw exception in that place.
>
> https://github.com/apache/ignite/pull/1388/commits
>
> 2016-12-29 11:16 GMT+03:00 Alexey Goncharuk <[hidden email]>:
>
>> I think that If fallbacks(...) returns an empty nodes collection, then we
>> should fail with an exception.
>>
>> 2016-12-28 22:06 GMT+03:00 Denis Magda <[hidden email]>:
>>
>>> Alexander, added you to the contributors list. Please check that you can
>>> assign the ticket on yourself.
>>>
>>> —
>>> Denis
>>>
>>>> On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <[hidden email]
>>>
>>> wrote:
>>>>
>>>>
>>>> Username: sharpler
>>>>
>>>> Full Name: Alexander Menshikov
>>>>
>>>>
>>>>
>>>> 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:
>>> [hidden email]>>:
>>>> Alexander,
>>>>
>>>> I need to know your JIRA ID in order to add you to the contributors
>> list.
>>>>
>>>> As for your questions, this situation might be caused by the race when
>> a
>>> cache is being stopped and there are still scan queries running in
>>> parallel. So, in general it’s not about data loss.
>>>>
>>>> Sam, Alex G., could you share your thoughts in regards to the proper
>> fix?
>>>>
>>>> —
>>>> Denis
>>>>
>>>>> On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <
>> [hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>>>
>>>>> Hello everyone.
>>>>>
>>>>> I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487
>> <
>>> https://issues.apache.org/jira/browse/IGNITE-4487>
>>>>> <https://issues.apache.org/jira/browse/IGNITE-4487 <
>>> https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
>>>>> first issue.
>>>>>
>>>>> Please add me as contributor.
>>>>>
>>>>> I already found that: in inner class
>>>>> 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
>>> constructor is
>>>>> called with method 'init()', but method 'init()' cannot be called
>> with
>>> an
>>>>> empty field 'nodes'. In source code it looks like:
>>>>>
>>>>> private ScanQueryFallbackClosableIterator(int part,
>>> GridCacheQueryAdapter
>>>>> qry,
>>>>>           GridCacheQueryManager qryMgr, GridCacheContext cctx) {
>>>>>           this.qry = qry;
>>>>>           this.qryMgr = qryMgr;
>>>>>           this.cctx = cctx;
>>>>>           this.part = part;
>>>>>
>>>>>           nodes = fallbacks(cctx.discovery().topologyVersionEx());
>>>>>           // !!! Here nodes.isEmpty()==true, and init() will fail in
>>> the
>>>>> future. !!!
>>>>>           init();
>>>>>       }
>>>>>
>>>>> I can fix it by adding some check in code, but i must know what
>>> behavior
>>>>> are best in this case? As I understand it, the list of nodes is empty
>>> if
>>>>> there are no nodes with the current partition, which means data loss,
>>> and
>>>>> either need to return a meaningful exception, or ignore this
>>> situation. But
>>>>> maybe I missed something.
>>>>
>>>>
>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

Александр Меньшиков
http://ci.ignite.apache.org/project.html?projectId=IgniteTests&tab=projectOverview&branch_IgniteTests=pull%2F1388%2Fmerge

There are overview of tests. It seems okay. A little unclear because there
are tests which  failed also in master branch.

2016-12-29 19:49 GMT+03:00 Denis Magda <[hidden email]>:

> Alexander, thanks.
>
> Please move the ticket from “open” into “patch available” state in JIRA
> and run the tests on TeamCity. Refer to the details covered there
> https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-1.CreateGitHubpull-request <
> https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-1.CreateGitHubpull-request>
>
> —
> Denis
>
> > On Dec 29, 2016, at 3:45 AM, Александр Меньшиков <[hidden email]>
> wrote:
> >
> > Alexey, I'm already make pull request where throw exception in that
> place.
> >
> > https://github.com/apache/ignite/pull/1388/commits
> >
> > 2016-12-29 11:16 GMT+03:00 Alexey Goncharuk <[hidden email]
> >:
> >
> >> I think that If fallbacks(...) returns an empty nodes collection, then
> we
> >> should fail with an exception.
> >>
> >> 2016-12-28 22:06 GMT+03:00 Denis Magda <[hidden email]>:
> >>
> >>> Alexander, added you to the contributors list. Please check that you
> can
> >>> assign the ticket on yourself.
> >>>
> >>> —
> >>> Denis
> >>>
> >>>> On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <
> [hidden email]
> >>>
> >>> wrote:
> >>>>
> >>>>
> >>>> Username: sharpler
> >>>>
> >>>> Full Name: Alexander Menshikov
> >>>>
> >>>>
> >>>>
> >>>> 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:
> >>> [hidden email]>>:
> >>>> Alexander,
> >>>>
> >>>> I need to know your JIRA ID in order to add you to the contributors
> >> list.
> >>>>
> >>>> As for your questions, this situation might be caused by the race when
> >> a
> >>> cache is being stopped and there are still scan queries running in
> >>> parallel. So, in general it’s not about data loss.
> >>>>
> >>>> Sam, Alex G., could you share your thoughts in regards to the proper
> >> fix?
> >>>>
> >>>> —
> >>>> Denis
> >>>>
> >>>>> On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <
> >> [hidden email]
> >>> <mailto:[hidden email]>> wrote:
> >>>>>
> >>>>> Hello everyone.
> >>>>>
> >>>>> I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487
> >> <
> >>> https://issues.apache.org/jira/browse/IGNITE-4487>
> >>>>> <https://issues.apache.org/jira/browse/IGNITE-4487 <
> >>> https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
> >>>>> first issue.
> >>>>>
> >>>>> Please add me as contributor.
> >>>>>
> >>>>> I already found that: in inner class
> >>>>> 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
> >>> constructor is
> >>>>> called with method 'init()', but method 'init()' cannot be called
> >> with
> >>> an
> >>>>> empty field 'nodes'. In source code it looks like:
> >>>>>
> >>>>> private ScanQueryFallbackClosableIterator(int part,
> >>> GridCacheQueryAdapter
> >>>>> qry,
> >>>>>           GridCacheQueryManager qryMgr, GridCacheContext cctx) {
> >>>>>           this.qry = qry;
> >>>>>           this.qryMgr = qryMgr;
> >>>>>           this.cctx = cctx;
> >>>>>           this.part = part;
> >>>>>
> >>>>>           nodes = fallbacks(cctx.discovery().topologyVersionEx());
> >>>>>           // !!! Here nodes.isEmpty()==true, and init() will fail in
> >>> the
> >>>>> future. !!!
> >>>>>           init();
> >>>>>       }
> >>>>>
> >>>>> I can fix it by adding some check in code, but i must know what
> >>> behavior
> >>>>> are best in this case? As I understand it, the list of nodes is empty
> >>> if
> >>>>> there are no nodes with the current partition, which means data loss,
> >>> and
> >>>>> either need to return a meaningful exception, or ignore this
> >>> situation. But
> >>>>> maybe I missed something.
> >>>>
> >>>>
> >>>
> >>>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

dmagda
Thanks Alexander.

Someone will review and merge your changes in the nearest time. Alex G., can you do that?


Denis

> On Jan 6, 2017, at 5:07 AM, Александр Меньшиков <[hidden email]> wrote:
>
> http://ci.ignite.apache.org/project.html?projectId=IgniteTests&tab=projectOverview&branch_IgniteTests=pull%2F1388%2Fmerge
>
> There are overview of tests. It seems okay. A little unclear because there
> are tests which  failed also in master branch.
>
> 2016-12-29 19:49 GMT+03:00 Denis Magda <[hidden email]>:
>
>> Alexander, thanks.
>>
>> Please move the ticket from “open” into “patch available” state in JIRA
>> and run the tests on TeamCity. Refer to the details covered there
>> https://cwiki.apache.org/confluence/display/IGNITE/How+
>> to+Contribute#HowtoContribute-1.CreateGitHubpull-request <
>> https://cwiki.apache.org/confluence/display/IGNITE/How+
>> to+Contribute#HowtoContribute-1.CreateGitHubpull-request>
>>
>> —
>> Denis
>>
>>> On Dec 29, 2016, at 3:45 AM, Александр Меньшиков <[hidden email]>
>> wrote:
>>>
>>> Alexey, I'm already make pull request where throw exception in that
>> place.
>>>
>>> https://github.com/apache/ignite/pull/1388/commits
>>>
>>> 2016-12-29 11:16 GMT+03:00 Alexey Goncharuk <[hidden email]
>>> :
>>>
>>>> I think that If fallbacks(...) returns an empty nodes collection, then
>> we
>>>> should fail with an exception.
>>>>
>>>> 2016-12-28 22:06 GMT+03:00 Denis Magda <[hidden email]>:
>>>>
>>>>> Alexander, added you to the contributors list. Please check that you
>> can
>>>>> assign the ticket on yourself.
>>>>>
>>>>> —
>>>>> Denis
>>>>>
>>>>>> On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <
>> [hidden email]
>>>>>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> Username: sharpler
>>>>>>
>>>>>> Full Name: Alexander Menshikov
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:
>>>>> [hidden email]>>:
>>>>>> Alexander,
>>>>>>
>>>>>> I need to know your JIRA ID in order to add you to the contributors
>>>> list.
>>>>>>
>>>>>> As for your questions, this situation might be caused by the race when
>>>> a
>>>>> cache is being stopped and there are still scan queries running in
>>>>> parallel. So, in general it’s not about data loss.
>>>>>>
>>>>>> Sam, Alex G., could you share your thoughts in regards to the proper
>>>> fix?
>>>>>>
>>>>>> —
>>>>>> Denis
>>>>>>
>>>>>>> On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <
>>>> [hidden email]
>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>
>>>>>>> Hello everyone.
>>>>>>>
>>>>>>> I want to pick up *https://issues.apache.org/jira/browse/IGNITE-4487
>>>> <
>>>>> https://issues.apache.org/jira/browse/IGNITE-4487>
>>>>>>> <https://issues.apache.org/jira/browse/IGNITE-4487 <
>>>>> https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
>>>>>>> first issue.
>>>>>>>
>>>>>>> Please add me as contributor.
>>>>>>>
>>>>>>> I already found that: in inner class
>>>>>>> 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
>>>>> constructor is
>>>>>>> called with method 'init()', but method 'init()' cannot be called
>>>> with
>>>>> an
>>>>>>> empty field 'nodes'. In source code it looks like:
>>>>>>>
>>>>>>> private ScanQueryFallbackClosableIterator(int part,
>>>>> GridCacheQueryAdapter
>>>>>>> qry,
>>>>>>>          GridCacheQueryManager qryMgr, GridCacheContext cctx) {
>>>>>>>          this.qry = qry;
>>>>>>>          this.qryMgr = qryMgr;
>>>>>>>          this.cctx = cctx;
>>>>>>>          this.part = part;
>>>>>>>
>>>>>>>          nodes = fallbacks(cctx.discovery().topologyVersionEx());
>>>>>>>          // !!! Here nodes.isEmpty()==true, and init() will fail in
>>>>> the
>>>>>>> future. !!!
>>>>>>>          init();
>>>>>>>      }
>>>>>>>
>>>>>>> I can fix it by adding some check in code, but i must know what
>>>>> behavior
>>>>>>> are best in this case? As I understand it, the list of nodes is empty
>>>>> if
>>>>>>> there are no nodes with the current partition, which means data loss,
>>>>> and
>>>>>>> either need to return a meaningful exception, or ignore this
>>>>> situation. But
>>>>>>> maybe I missed something.
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

Alexey Goncharuk
Alexander,

I left my comments in the ticket.

2017-01-06 20:35 GMT+03:00 Denis Magda <[hidden email]>:

> Thanks Alexander.
>
> Someone will review and merge your changes in the nearest time. Alex G.,
> can you do that?
>
> —
> Denis
>
> > On Jan 6, 2017, at 5:07 AM, Александр Меньшиков <[hidden email]>
> wrote:
> >
> > http://ci.ignite.apache.org/project.html?projectId=IgniteTests&tab=
> projectOverview&branch_IgniteTests=pull%2F1388%2Fmerge
> >
> > There are overview of tests. It seems okay. A little unclear because
> there
> > are tests which  failed also in master branch.
> >
> > 2016-12-29 19:49 GMT+03:00 Denis Magda <[hidden email]>:
> >
> >> Alexander, thanks.
> >>
> >> Please move the ticket from “open” into “patch available” state in JIRA
> >> and run the tests on TeamCity. Refer to the details covered there
> >> https://cwiki.apache.org/confluence/display/IGNITE/How+
> >> to+Contribute#HowtoContribute-1.CreateGitHubpull-request <
> >> https://cwiki.apache.org/confluence/display/IGNITE/How+
> >> to+Contribute#HowtoContribute-1.CreateGitHubpull-request>
> >>
> >> —
> >> Denis
> >>
> >>> On Dec 29, 2016, at 3:45 AM, Александр Меньшиков <[hidden email]
> >
> >> wrote:
> >>>
> >>> Alexey, I'm already make pull request where throw exception in that
> >> place.
> >>>
> >>> https://github.com/apache/ignite/pull/1388/commits
> >>>
> >>> 2016-12-29 11:16 GMT+03:00 Alexey Goncharuk <
> [hidden email]
> >>> :
> >>>
> >>>> I think that If fallbacks(...) returns an empty nodes collection, then
> >> we
> >>>> should fail with an exception.
> >>>>
> >>>> 2016-12-28 22:06 GMT+03:00 Denis Magda <[hidden email]>:
> >>>>
> >>>>> Alexander, added you to the contributors list. Please check that you
> >> can
> >>>>> assign the ticket on yourself.
> >>>>>
> >>>>> —
> >>>>> Denis
> >>>>>
> >>>>>> On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <
> >> [hidden email]
> >>>>>
> >>>>> wrote:
> >>>>>>
> >>>>>>
> >>>>>> Username: sharpler
> >>>>>>
> >>>>>> Full Name: Alexander Menshikov
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:
> >>>>> [hidden email]>>:
> >>>>>> Alexander,
> >>>>>>
> >>>>>> I need to know your JIRA ID in order to add you to the contributors
> >>>> list.
> >>>>>>
> >>>>>> As for your questions, this situation might be caused by the race
> when
> >>>> a
> >>>>> cache is being stopped and there are still scan queries running in
> >>>>> parallel. So, in general it’s not about data loss.
> >>>>>>
> >>>>>> Sam, Alex G., could you share your thoughts in regards to the proper
> >>>> fix?
> >>>>>>
> >>>>>> —
> >>>>>> Denis
> >>>>>>
> >>>>>>> On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <
> >>>> [hidden email]
> >>>>> <mailto:[hidden email]>> wrote:
> >>>>>>>
> >>>>>>> Hello everyone.
> >>>>>>>
> >>>>>>> I want to pick up *https://issues.apache.org/
> jira/browse/IGNITE-4487
> >>>> <
> >>>>> https://issues.apache.org/jira/browse/IGNITE-4487>
> >>>>>>> <https://issues.apache.org/jira/browse/IGNITE-4487 <
> >>>>> https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
> >>>>>>> first issue.
> >>>>>>>
> >>>>>>> Please add me as contributor.
> >>>>>>>
> >>>>>>> I already found that: in inner class
> >>>>>>> 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
> >>>>> constructor is
> >>>>>>> called with method 'init()', but method 'init()' cannot be called
> >>>> with
> >>>>> an
> >>>>>>> empty field 'nodes'. In source code it looks like:
> >>>>>>>
> >>>>>>> private ScanQueryFallbackClosableIterator(int part,
> >>>>> GridCacheQueryAdapter
> >>>>>>> qry,
> >>>>>>>          GridCacheQueryManager qryMgr, GridCacheContext cctx) {
> >>>>>>>          this.qry = qry;
> >>>>>>>          this.qryMgr = qryMgr;
> >>>>>>>          this.cctx = cctx;
> >>>>>>>          this.part = part;
> >>>>>>>
> >>>>>>>          nodes = fallbacks(cctx.discovery().topologyVersionEx());
> >>>>>>>          // !!! Here nodes.isEmpty()==true, and init() will fail in
> >>>>> the
> >>>>>>> future. !!!
> >>>>>>>          init();
> >>>>>>>      }
> >>>>>>>
> >>>>>>> I can fix it by adding some check in code, but i must know what
> >>>>> behavior
> >>>>>>> are best in this case? As I understand it, the list of nodes is
> empty
> >>>>> if
> >>>>>>> there are no nodes with the current partition, which means data
> loss,
> >>>>> and
> >>>>>>> either need to return a meaningful exception, or ignore this
> >>>>> situation. But
> >>>>>>> maybe I missed something.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-4487 - NPE on query execution

Alexey Goncharuk
Alexander,

I merged your PR to master, however I had to rewrite the test completely.
Please review my changes carefully for further contributions:
 *  The test was missing Apache License header
 * We do not create Ignite configuration and start nodes manually.
startGrids() takes care of the config. If you need to change ignite
configuration, override the getConfiguration(String) method
 * Do no start threads manually. For async test operations we have many
GridTestUtils#...Multithreaded() methods

2017-01-09 11:07 GMT+03:00 Alexey Goncharuk <[hidden email]>:

> Alexander,
>
> I left my comments in the ticket.
>
> 2017-01-06 20:35 GMT+03:00 Denis Magda <[hidden email]>:
>
>> Thanks Alexander.
>>
>> Someone will review and merge your changes in the nearest time. Alex G.,
>> can you do that?
>>
>> —
>> Denis
>>
>> > On Jan 6, 2017, at 5:07 AM, Александр Меньшиков <[hidden email]>
>> wrote:
>> >
>> > http://ci.ignite.apache.org/project.html?projectId=IgniteTes
>> ts&tab=projectOverview&branch_IgniteTests=pull%2F1388%2Fmerge
>> >
>> > There are overview of tests. It seems okay. A little unclear because
>> there
>> > are tests which  failed also in master branch.
>> >
>> > 2016-12-29 19:49 GMT+03:00 Denis Magda <[hidden email]>:
>> >
>> >> Alexander, thanks.
>> >>
>> >> Please move the ticket from “open” into “patch available” state in JIRA
>> >> and run the tests on TeamCity. Refer to the details covered there
>> >> https://cwiki.apache.org/confluence/display/IGNITE/How+
>> >> to+Contribute#HowtoContribute-1.CreateGitHubpull-request <
>> >> https://cwiki.apache.org/confluence/display/IGNITE/How+
>> >> to+Contribute#HowtoContribute-1.CreateGitHubpull-request>
>> >>
>> >> —
>> >> Denis
>> >>
>> >>> On Dec 29, 2016, at 3:45 AM, Александр Меньшиков <
>> [hidden email]>
>> >> wrote:
>> >>>
>> >>> Alexey, I'm already make pull request where throw exception in that
>> >> place.
>> >>>
>> >>> https://github.com/apache/ignite/pull/1388/commits
>> >>>
>> >>> 2016-12-29 11:16 GMT+03:00 Alexey Goncharuk <
>> [hidden email]
>> >>> :
>> >>>
>> >>>> I think that If fallbacks(...) returns an empty nodes collection,
>> then
>> >> we
>> >>>> should fail with an exception.
>> >>>>
>> >>>> 2016-12-28 22:06 GMT+03:00 Denis Magda <[hidden email]>:
>> >>>>
>> >>>>> Alexander, added you to the contributors list. Please check that you
>> >> can
>> >>>>> assign the ticket on yourself.
>> >>>>>
>> >>>>> —
>> >>>>> Denis
>> >>>>>
>> >>>>>> On Dec 28, 2016, at 2:15 AM, Александр Меньшиков <
>> >> [hidden email]
>> >>>>>
>> >>>>> wrote:
>> >>>>>>
>> >>>>>>
>> >>>>>> Username: sharpler
>> >>>>>>
>> >>>>>> Full Name: Alexander Menshikov
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>> 2016-12-27 22:57 GMT+03:00 Denis Magda <[hidden email] <mailto:
>> >>>>> [hidden email]>>:
>> >>>>>> Alexander,
>> >>>>>>
>> >>>>>> I need to know your JIRA ID in order to add you to the contributors
>> >>>> list.
>> >>>>>>
>> >>>>>> As for your questions, this situation might be caused by the race
>> when
>> >>>> a
>> >>>>> cache is being stopped and there are still scan queries running in
>> >>>>> parallel. So, in general it’s not about data loss.
>> >>>>>>
>> >>>>>> Sam, Alex G., could you share your thoughts in regards to the
>> proper
>> >>>> fix?
>> >>>>>>
>> >>>>>> —
>> >>>>>> Denis
>> >>>>>>
>> >>>>>>> On Dec 26, 2016, at 2:43 AM, Александр Меньшиков <
>> >>>> [hidden email]
>> >>>>> <mailto:[hidden email]>> wrote:
>> >>>>>>>
>> >>>>>>> Hello everyone.
>> >>>>>>>
>> >>>>>>> I want to pick up *https://issues.apache.org/jir
>> a/browse/IGNITE-4487
>> >>>> <
>> >>>>> https://issues.apache.org/jira/browse/IGNITE-4487>
>> >>>>>>> <https://issues.apache.org/jira/browse/IGNITE-4487 <
>> >>>>> https://issues.apache.org/jira/browse/IGNITE-4487>>* as my
>> >>>>>>> first issue.
>> >>>>>>>
>> >>>>>>> Please add me as contributor.
>> >>>>>>>
>> >>>>>>> I already found that: in inner class
>> >>>>>>> 'GridCacheQueryAdapter.ScanQueryFallbackClosableIterator' in
>> >>>>> constructor is
>> >>>>>>> called with method 'init()', but method 'init()' cannot be called
>> >>>> with
>> >>>>> an
>> >>>>>>> empty field 'nodes'. In source code it looks like:
>> >>>>>>>
>> >>>>>>> private ScanQueryFallbackClosableIterator(int part,
>> >>>>> GridCacheQueryAdapter
>> >>>>>>> qry,
>> >>>>>>>          GridCacheQueryManager qryMgr, GridCacheContext cctx) {
>> >>>>>>>          this.qry = qry;
>> >>>>>>>          this.qryMgr = qryMgr;
>> >>>>>>>          this.cctx = cctx;
>> >>>>>>>          this.part = part;
>> >>>>>>>
>> >>>>>>>          nodes = fallbacks(cctx.discovery().topologyVersionEx());
>> >>>>>>>          // !!! Here nodes.isEmpty()==true, and init() will fail
>> in
>> >>>>> the
>> >>>>>>> future. !!!
>> >>>>>>>          init();
>> >>>>>>>      }
>> >>>>>>>
>> >>>>>>> I can fix it by adding some check in code, but i must know what
>> >>>>> behavior
>> >>>>>>> are best in this case? As I understand it, the list of nodes is
>> empty
>> >>>>> if
>> >>>>>>> there are no nodes with the current partition, which means data
>> loss,
>> >>>>> and
>> >>>>>>> either need to return a meaningful exception, or ignore this
>> >>>>> situation. But
>> >>>>>>> maybe I missed something.
>> >>>>>>
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>
>> >>
>> >>
>>
>>
>