You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@phoenix.apache.org by Jack Steenkamp <st...@gmail.com> on 2018/08/21 13:48:45 UTC

CursorUtil -> mapCursorIDQuery

Hi All,

In my application I make heavy use of Apache Phoenix's Cursors
(https://phoenix.apache.org/cursors.html) - and for the majority of
cases it works great and makes my life a lot easier (thanks for
implementing them). However, in very rare cases (fiendishly rare
cases), I get NullpointerExceptions such as the following:

java.lang.NullPointerException: null
at org.apache.phoenix.util.CursorUtil.updateCursor(CursorUtil.java:179)
at org.apache.phoenix.iterate.CursorResultIterator.next(CursorResultIterator.java:46)
at org.apache.phoenix.jdbc.PhoenixResultSet.next(PhoenixResultSet.java:779)

(This is for 4.13.1 - but seems that
org.apache.phoenix.util.CursorUtil has not changed much since it was
first created).

Upon closer inspection it would seem that on line 124 of CursorUtil, a
HashMap is used to keep state which is then exposed via a number
static methods, which, one has to assume, can be accessed by many
different threads. Using a plain old HashMap in cases like these have
(at least in my experience) been to blame for rare issues such as
these.

Would you perhaps consider changing the implementation of
mapCursorIDQuery to a ConcurrentHashMap instead? That should hopefully
tighten up the class and prevent any potential inconsistencies.

Regards,

Jack Steenkamp

Re: CursorUtil -> mapCursorIDQuery

Posted by Jack Steenkamp <st...@gmail.com>.
Hi Josh / All,

Just following up on the two patches left on PHOENIX-4860
<https://issues.apache.org/jira/browse/PHOENIX-4860> (the quick, albeit
hacky, fix and the more extensive one). Not sure if anyone has had a chance
to look at these yet? Can appreciate if other things take priority (if so,
will then park this for now and workaround locally).

The inclusion of the more extensive patch would be better (I think), though
even with the quick-fix it would make my app/build process more stable /
thread-safe.

Thanks,

On Tue, 21 Aug 2018 at 17:04, Josh Elser <el...@apache.org> wrote:

> Thanks sir! Will take a look up there and continue.
>
> On 8/21/18 11:48 AM, Jack Steenkamp wrote:
> > Hi Josh,
> >
> > I've created https://issues.apache.org/jira/browse/PHOENIX-4860 for
> this.
> >
> > Thanks,
> > On Tue, 21 Aug 2018 at 16:34, Jack Steenkamp <st...@gmail.com>
> wrote:
> >>
> >> Hi Josh,
> >>
> >> Glad I could help. The CursorUtil class it seems has not changed since
> >> it was first created as part of PHOENIX-3572 - and is still the same
> >> in master (I checked a bit earlier).
> >>
> >> Sure - will have a go at creating a JIRA for this.
> >>
> >> Regards,
> >>
> >> On Tue, 21 Aug 2018 at 16:23, Josh Elser <el...@apache.org> wrote:
> >>>
> >>> Hi Jack,
> >>>
> >>> Given your assessment, it sounds like you've stumbled onto a race
> >>> condition! Thanks for bringing it to our attention.
> >>>
> >>> A few questions:
> >>>
> >>> * Have you checked if the same code exist in the latest
> >>> branches/releases (4.x-HBase-1.{2,3,4} or master)?
> >>> * Want to create a Jira issue to track this? Else, I can do this for
> ya.
> >>>
> >>> On 8/21/18 9:48 AM, Jack Steenkamp wrote:
> >>>> Hi All,
> >>>>
> >>>> In my application I make heavy use of Apache Phoenix's Cursors
> >>>> (https://phoenix.apache.org/cursors.html) - and for the majority of
> >>>> cases it works great and makes my life a lot easier (thanks for
> >>>> implementing them). However, in very rare cases (fiendishly rare
> >>>> cases), I get NullpointerExceptions such as the following:
> >>>>
> >>>> java.lang.NullPointerException: null
> >>>> at
> org.apache.phoenix.util.CursorUtil.updateCursor(CursorUtil.java:179)
> >>>> at
> org.apache.phoenix.iterate.CursorResultIterator.next(CursorResultIterator.java:46)
> >>>> at
> org.apache.phoenix.jdbc.PhoenixResultSet.next(PhoenixResultSet.java:779)
> >>>>
> >>>> (This is for 4.13.1 - but seems that
> >>>> org.apache.phoenix.util.CursorUtil has not changed much since it was
> >>>> first created).
> >>>>
> >>>> Upon closer inspection it would seem that on line 124 of CursorUtil, a
> >>>> HashMap is used to keep state which is then exposed via a number
> >>>> static methods, which, one has to assume, can be accessed by many
> >>>> different threads. Using a plain old HashMap in cases like these have
> >>>> (at least in my experience) been to blame for rare issues such as
> >>>> these.
> >>>>
> >>>> Would you perhaps consider changing the implementation of
> >>>> mapCursorIDQuery to a ConcurrentHashMap instead? That should hopefully
> >>>> tighten up the class and prevent any potential inconsistencies.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Jack Steenkamp
> >>>>
>

Re: CursorUtil -> mapCursorIDQuery

Posted by Josh Elser <el...@apache.org>.
Thanks sir! Will take a look up there and continue.

On 8/21/18 11:48 AM, Jack Steenkamp wrote:
> Hi Josh,
> 
> I've created https://issues.apache.org/jira/browse/PHOENIX-4860 for this.
> 
> Thanks,
> On Tue, 21 Aug 2018 at 16:34, Jack Steenkamp <st...@gmail.com> wrote:
>>
>> Hi Josh,
>>
>> Glad I could help. The CursorUtil class it seems has not changed since
>> it was first created as part of PHOENIX-3572 - and is still the same
>> in master (I checked a bit earlier).
>>
>> Sure - will have a go at creating a JIRA for this.
>>
>> Regards,
>>
>> On Tue, 21 Aug 2018 at 16:23, Josh Elser <el...@apache.org> wrote:
>>>
>>> Hi Jack,
>>>
>>> Given your assessment, it sounds like you've stumbled onto a race
>>> condition! Thanks for bringing it to our attention.
>>>
>>> A few questions:
>>>
>>> * Have you checked if the same code exist in the latest
>>> branches/releases (4.x-HBase-1.{2,3,4} or master)?
>>> * Want to create a Jira issue to track this? Else, I can do this for ya.
>>>
>>> On 8/21/18 9:48 AM, Jack Steenkamp wrote:
>>>> Hi All,
>>>>
>>>> In my application I make heavy use of Apache Phoenix's Cursors
>>>> (https://phoenix.apache.org/cursors.html) - and for the majority of
>>>> cases it works great and makes my life a lot easier (thanks for
>>>> implementing them). However, in very rare cases (fiendishly rare
>>>> cases), I get NullpointerExceptions such as the following:
>>>>
>>>> java.lang.NullPointerException: null
>>>> at org.apache.phoenix.util.CursorUtil.updateCursor(CursorUtil.java:179)
>>>> at org.apache.phoenix.iterate.CursorResultIterator.next(CursorResultIterator.java:46)
>>>> at org.apache.phoenix.jdbc.PhoenixResultSet.next(PhoenixResultSet.java:779)
>>>>
>>>> (This is for 4.13.1 - but seems that
>>>> org.apache.phoenix.util.CursorUtil has not changed much since it was
>>>> first created).
>>>>
>>>> Upon closer inspection it would seem that on line 124 of CursorUtil, a
>>>> HashMap is used to keep state which is then exposed via a number
>>>> static methods, which, one has to assume, can be accessed by many
>>>> different threads. Using a plain old HashMap in cases like these have
>>>> (at least in my experience) been to blame for rare issues such as
>>>> these.
>>>>
>>>> Would you perhaps consider changing the implementation of
>>>> mapCursorIDQuery to a ConcurrentHashMap instead? That should hopefully
>>>> tighten up the class and prevent any potential inconsistencies.
>>>>
>>>> Regards,
>>>>
>>>> Jack Steenkamp
>>>>

Re: CursorUtil -> mapCursorIDQuery

Posted by Jack Steenkamp <st...@gmail.com>.
Hi Josh,

I've created https://issues.apache.org/jira/browse/PHOENIX-4860 for this.

Thanks,
On Tue, 21 Aug 2018 at 16:34, Jack Steenkamp <st...@gmail.com> wrote:
>
> Hi Josh,
>
> Glad I could help. The CursorUtil class it seems has not changed since
> it was first created as part of PHOENIX-3572 - and is still the same
> in master (I checked a bit earlier).
>
> Sure - will have a go at creating a JIRA for this.
>
> Regards,
>
> On Tue, 21 Aug 2018 at 16:23, Josh Elser <el...@apache.org> wrote:
> >
> > Hi Jack,
> >
> > Given your assessment, it sounds like you've stumbled onto a race
> > condition! Thanks for bringing it to our attention.
> >
> > A few questions:
> >
> > * Have you checked if the same code exist in the latest
> > branches/releases (4.x-HBase-1.{2,3,4} or master)?
> > * Want to create a Jira issue to track this? Else, I can do this for ya.
> >
> > On 8/21/18 9:48 AM, Jack Steenkamp wrote:
> > > Hi All,
> > >
> > > In my application I make heavy use of Apache Phoenix's Cursors
> > > (https://phoenix.apache.org/cursors.html) - and for the majority of
> > > cases it works great and makes my life a lot easier (thanks for
> > > implementing them). However, in very rare cases (fiendishly rare
> > > cases), I get NullpointerExceptions such as the following:
> > >
> > > java.lang.NullPointerException: null
> > > at org.apache.phoenix.util.CursorUtil.updateCursor(CursorUtil.java:179)
> > > at org.apache.phoenix.iterate.CursorResultIterator.next(CursorResultIterator.java:46)
> > > at org.apache.phoenix.jdbc.PhoenixResultSet.next(PhoenixResultSet.java:779)
> > >
> > > (This is for 4.13.1 - but seems that
> > > org.apache.phoenix.util.CursorUtil has not changed much since it was
> > > first created).
> > >
> > > Upon closer inspection it would seem that on line 124 of CursorUtil, a
> > > HashMap is used to keep state which is then exposed via a number
> > > static methods, which, one has to assume, can be accessed by many
> > > different threads. Using a plain old HashMap in cases like these have
> > > (at least in my experience) been to blame for rare issues such as
> > > these.
> > >
> > > Would you perhaps consider changing the implementation of
> > > mapCursorIDQuery to a ConcurrentHashMap instead? That should hopefully
> > > tighten up the class and prevent any potential inconsistencies.
> > >
> > > Regards,
> > >
> > > Jack Steenkamp
> > >

Re: CursorUtil -> mapCursorIDQuery

Posted by Jack Steenkamp <st...@gmail.com>.
Hi Josh,

Glad I could help. The CursorUtil class it seems has not changed since
it was first created as part of PHOENIX-3572 - and is still the same
in master (I checked a bit earlier).

Sure - will have a go at creating a JIRA for this.

Regards,

On Tue, 21 Aug 2018 at 16:23, Josh Elser <el...@apache.org> wrote:
>
> Hi Jack,
>
> Given your assessment, it sounds like you've stumbled onto a race
> condition! Thanks for bringing it to our attention.
>
> A few questions:
>
> * Have you checked if the same code exist in the latest
> branches/releases (4.x-HBase-1.{2,3,4} or master)?
> * Want to create a Jira issue to track this? Else, I can do this for ya.
>
> On 8/21/18 9:48 AM, Jack Steenkamp wrote:
> > Hi All,
> >
> > In my application I make heavy use of Apache Phoenix's Cursors
> > (https://phoenix.apache.org/cursors.html) - and for the majority of
> > cases it works great and makes my life a lot easier (thanks for
> > implementing them). However, in very rare cases (fiendishly rare
> > cases), I get NullpointerExceptions such as the following:
> >
> > java.lang.NullPointerException: null
> > at org.apache.phoenix.util.CursorUtil.updateCursor(CursorUtil.java:179)
> > at org.apache.phoenix.iterate.CursorResultIterator.next(CursorResultIterator.java:46)
> > at org.apache.phoenix.jdbc.PhoenixResultSet.next(PhoenixResultSet.java:779)
> >
> > (This is for 4.13.1 - but seems that
> > org.apache.phoenix.util.CursorUtil has not changed much since it was
> > first created).
> >
> > Upon closer inspection it would seem that on line 124 of CursorUtil, a
> > HashMap is used to keep state which is then exposed via a number
> > static methods, which, one has to assume, can be accessed by many
> > different threads. Using a plain old HashMap in cases like these have
> > (at least in my experience) been to blame for rare issues such as
> > these.
> >
> > Would you perhaps consider changing the implementation of
> > mapCursorIDQuery to a ConcurrentHashMap instead? That should hopefully
> > tighten up the class and prevent any potential inconsistencies.
> >
> > Regards,
> >
> > Jack Steenkamp
> >

Re: CursorUtil -> mapCursorIDQuery

Posted by Josh Elser <el...@apache.org>.
Hi Jack,

Given your assessment, it sounds like you've stumbled onto a race 
condition! Thanks for bringing it to our attention.

A few questions:

* Have you checked if the same code exist in the latest 
branches/releases (4.x-HBase-1.{2,3,4} or master)?
* Want to create a Jira issue to track this? Else, I can do this for ya.

On 8/21/18 9:48 AM, Jack Steenkamp wrote:
> Hi All,
> 
> In my application I make heavy use of Apache Phoenix's Cursors
> (https://phoenix.apache.org/cursors.html) - and for the majority of
> cases it works great and makes my life a lot easier (thanks for
> implementing them). However, in very rare cases (fiendishly rare
> cases), I get NullpointerExceptions such as the following:
> 
> java.lang.NullPointerException: null
> at org.apache.phoenix.util.CursorUtil.updateCursor(CursorUtil.java:179)
> at org.apache.phoenix.iterate.CursorResultIterator.next(CursorResultIterator.java:46)
> at org.apache.phoenix.jdbc.PhoenixResultSet.next(PhoenixResultSet.java:779)
> 
> (This is for 4.13.1 - but seems that
> org.apache.phoenix.util.CursorUtil has not changed much since it was
> first created).
> 
> Upon closer inspection it would seem that on line 124 of CursorUtil, a
> HashMap is used to keep state which is then exposed via a number
> static methods, which, one has to assume, can be accessed by many
> different threads. Using a plain old HashMap in cases like these have
> (at least in my experience) been to blame for rare issues such as
> these.
> 
> Would you perhaps consider changing the implementation of
> mapCursorIDQuery to a ConcurrentHashMap instead? That should hopefully
> tighten up the class and prevent any potential inconsistencies.
> 
> Regards,
> 
> Jack Steenkamp
>