[index] OneLevelIndex removal

[Emmanuel Lécharny <el...@gmail.com>]

Hi guys !

so I completely removed the OneLevelIndex from the server. The branch 
(index) has been successfully merged back into trunk, and I will now 
work on removing the SublevelIndex from the index branch.

In the process, I spent 3 days closing all the cursors that weren't 
closed after having been used. This was *BORING*. In the future, I would 
really appreciate if those who use the cursors double check that they 
have closed them.

To do that, I added some logs in every cursor constructor and every 
close() method, and matched the opens with the closes. Do'nt ask me if 
this was fun to match them... I created a small program which was able 
to do that for me, but this is not enough to know that a cursor has been 
created but not closed, we also have to know where it has been created.

I think we should add some mechanism in the server to check that 
automatically, to avoid doing it by hand (there are hundreds of tests to 
check...). One solution would be to keep a track of every cursor 
construction in a HashMap, and to remove them when the cursor is closed. 
The remaining cursors are likely not closed. The pb is that it gives no 
clue about where those cursors have been created, unless we associate a 
stackTrace to this information. Really not possible in production, but 
we might add an extended request to activate this mode, or a flag in the 
config.

If anyone has a better idea ?

Thanks !


-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: [index] OneLevelIndex removal

[Alex Karasulu <ak...@apache.org>]

On Thu, Apr 12, 2012 at 7:00 PM, Selcuk AYA <ay...@gmail.com> wrote:

> On Thu, Apr 12, 2012 at 7:14 AM, Alex Karasulu <ak...@apache.org>
> wrote:
> >
> >
> > On Thu, Apr 12, 2012 at 4:35 PM, Emmanuel Lécharny <el...@gmail.com>
> > wrote:
> >>
> >> Forgot to reply to this mail, which raises interesting points.
> >>
> >> More inside.
> >>
> >> Le 4/11/12 10:38 PM, Alex Karasulu a écrit :
> >>>
> >>> On Wed, Apr 11, 2012 at 4:04 PM, Emmanuel
> >>> Lécharny<el...@gmail.com>wrote:
> >>>
> >>>> I think we should add some mechanism in the server to check that
> >>>> automatically, to avoid doing it by hand (there are hundreds of tests
> to
> >>>> check...). One solution would be to keep a track of every cursor
> >>>> construction in a HashMap, and to remove them when the cursor is
> closed.
> >>>> The remaining cursors are likely not closed.
> >>>
> >>>
> >>> It would be nice to have a Cursor monitor that every opened Cursor
> >>> registers with but this needs to happen automatically. Then when out of
> >>> the
> >>> creation scope the Cursor is expected to be closed and if not this is
> >>> handled automatically. However does creation scope work well since
> >>> sometimes we create Cursors and pass them up?
> >>
> >> We do have a monitor, which is currently used to check that the cursor
> is
> >> not closed when we try to use it. We certainly can use this monitor for
> more
> >> than just checking such thing.
> >>
> >> Now, the pb is that the scope is not as easy to determinate than for a
> >> variable in Java. For instance, if we consider persistent searches, or
> paged
> >> searches, or even an abandonned search request, the scope is pretty
> wide...
> >>
> >> Though we can have a set of rules that help us to close the cursor
> >> automatically :
> >> - if we get an exception during a SearchRequest, then the cursors must
> be
> >> closed immediately. As soon as we store the cursors into the
> SearchContext,
> >> this is pretty easy to do
> >> - an AbandonRequest will close the cursor automatically too (getting the
> >> cursor from the abandonned request)
> >> - when we process the SearchResultDone, we can also close the cursor for
> >> the current search request (this work for PagedSearch too)
> >> - for pagedSearch, if the user reset the search by sending 0 as the
> >> expected number of entries to return, then the cursor will be freed
> >> - for persistent searches, as it will be closed by an unbind or an
> abandon
> >> request, we are fine
> >> - when a client unbinds, then all the pending cursors will be closed.
> >>
> >> All in all, we have everything needed to close the cursors
> automatically,
> >> assuming we keep all the cursors into the session.
>
> For the server side, I would suggest policing this with a test. When
> cursors open, they can bump a global counter atomically. When they
> close, they can decrement it. We can have a test such that after a
> bunch of operations, this counter at the server side should be zero.
>
>
Great idea!

-- 
Best Regards,
-- Alex

Re: [index] OneLevelIndex removal

[Selcuk AYA <ay...@gmail.com>]

On Thu, Apr 12, 2012 at 7:14 AM, Alex Karasulu <ak...@apache.org> wrote:
>
>
> On Thu, Apr 12, 2012 at 4:35 PM, Emmanuel Lécharny <el...@gmail.com>
> wrote:
>>
>> Forgot to reply to this mail, which raises interesting points.
>>
>> More inside.
>>
>> Le 4/11/12 10:38 PM, Alex Karasulu a écrit :
>>>
>>> On Wed, Apr 11, 2012 at 4:04 PM, Emmanuel
>>> Lécharny<el...@gmail.com>wrote:
>>>
>>>> I think we should add some mechanism in the server to check that
>>>> automatically, to avoid doing it by hand (there are hundreds of tests to
>>>> check...). One solution would be to keep a track of every cursor
>>>> construction in a HashMap, and to remove them when the cursor is closed.
>>>> The remaining cursors are likely not closed.
>>>
>>>
>>> It would be nice to have a Cursor monitor that every opened Cursor
>>> registers with but this needs to happen automatically. Then when out of
>>> the
>>> creation scope the Cursor is expected to be closed and if not this is
>>> handled automatically. However does creation scope work well since
>>> sometimes we create Cursors and pass them up?
>>
>> We do have a monitor, which is currently used to check that the cursor is
>> not closed when we try to use it. We certainly can use this monitor for more
>> than just checking such thing.
>>
>> Now, the pb is that the scope is not as easy to determinate than for a
>> variable in Java. For instance, if we consider persistent searches, or paged
>> searches, or even an abandonned search request, the scope is pretty wide...
>>
>> Though we can have a set of rules that help us to close the cursor
>> automatically :
>> - if we get an exception during a SearchRequest, then the cursors must be
>> closed immediately. As soon as we store the cursors into the SearchContext,
>> this is pretty easy to do
>> - an AbandonRequest will close the cursor automatically too (getting the
>> cursor from the abandonned request)
>> - when we process the SearchResultDone, we can also close the cursor for
>> the current search request (this work for PagedSearch too)
>> - for pagedSearch, if the user reset the search by sending 0 as the
>> expected number of entries to return, then the cursor will be freed
>> - for persistent searches, as it will be closed by an unbind or an abandon
>> request, we are fine
>> - when a client unbinds, then all the pending cursors will be closed.
>>
>> All in all, we have everything needed to close the cursors automatically,
>> assuming we keep all the cursors into the session.

For the server side, I would suggest policing this with a test. When
cursors open, they can bump a global counter atomically. When they
close, they can decrement it. We can have a test such that after a
bunch of operations, this counter at the server side should be zero.

>>
>
> These are really great suggestions and make the ideas I tried to express
> really tangible. Thanks for it Emmanuel.
>
> One technical point, we need to make Cursor close() operations idempotent if
> they are not already - meaning if we close a second time this should not
> cause an exception or change the outcome.
>
>>
>> On the client side, this is another issue... As cursors are created by the
>> client code, we have no easy way to determinate when we should close the
>> cursors, except when the connection is closed or an abandon request/unbind
>> request is sent. Of course, when the server returns a searchResultDone we
>> could also close the cursor. Remains the situations where the client has
>> fetched some entries (but not all), and haven't unbind nor abandonned the
>> search.
>>
>
> I think the aspect for automatic closing of cursors is left to be managed
> inside the server even though the API overlaps here.
>
>>
>> In any case, this is less critical as we don't have to deal with the txn
>> layer. The client will just blow away with some nasty OOM sooner or later...
>> but this is not worse than what we get with NamingEnumeration in JNDI, nah ?
>>
>
> Yup +1
>
>>
>> Have I covered all the server options ? Or did I miss something ?
>>
>>>
>>> This sounds like something that can be handled nicely using an aspect
>>> oriented solution. Now these things are heavy if you use AspectJ or
>>> something like that but other simpler solutions exist to bytecode splice
>>> compiled code to automatically handle these things. Maybe our past
>>> experiences with Aspects might make us reconsider.
>>
>> A bit overkilling, IMO?
>>
>
> I'm feeling the same but thought it should be just put out there. However we
> can achieve the same results perhaps with code or using a lighter mechanism
> with Proxy's via CGlib or something similar. These are just raw thought
> dumps so it's not a we SHOULD recommendation. Something to think about.
>
> --
> Best Regards,
> -- Alex
>

Re: [index] OneLevelIndex removal

[Alex Karasulu <ak...@apache.org>]

On Thu, Apr 12, 2012 at 4:35 PM, Emmanuel Lécharny <el...@gmail.com>wrote:

> Forgot to reply to this mail, which raises interesting points.
>
> More inside.
>
> Le 4/11/12 10:38 PM, Alex Karasulu a écrit :
>
>> On Wed, Apr 11, 2012 at 4:04 PM, Emmanuel Lécharny<el...@gmail.com>**
>> wrote:
>>
>>  I think we should add some mechanism in the server to check that
>>> automatically, to avoid doing it by hand (there are hundreds of tests to
>>> check...). One solution would be to keep a track of every cursor
>>> construction in a HashMap, and to remove them when the cursor is closed.
>>> The remaining cursors are likely not closed.
>>>
>>
>> It would be nice to have a Cursor monitor that every opened Cursor
>> registers with but this needs to happen automatically. Then when out of
>> the
>> creation scope the Cursor is expected to be closed and if not this is
>> handled automatically. However does creation scope work well since
>> sometimes we create Cursors and pass them up?
>>
> We do have a monitor, which is currently used to check that the cursor is
> not closed when we try to use it. We certainly can use this monitor for
> more than just checking such thing.
>
> Now, the pb is that the scope is not as easy to determinate than for a
> variable in Java. For instance, if we consider persistent searches, or
> paged searches, or even an abandonned search request, the scope is pretty
> wide...
>
> Though we can have a set of rules that help us to close the cursor
> automatically :
> - if we get an exception during a SearchRequest, then the cursors must be
> closed immediately. As soon as we store the cursors into the SearchContext,
> this is pretty easy to do
> - an AbandonRequest will close the cursor automatically too (getting the
> cursor from the abandonned request)
> - when we process the SearchResultDone, we can also close the cursor for
> the current search request (this work for PagedSearch too)
> - for pagedSearch, if the user reset the search by sending 0 as the
> expected number of entries to return, then the cursor will be freed
> - for persistent searches, as it will be closed by an unbind or an abandon
> request, we are fine
> - when a client unbinds, then all the pending cursors will be closed.
>
> All in all, we have everything needed to close the cursors automatically,
> assuming we keep all the cursors into the session.
>
>
These are really great suggestions and make the ideas I tried to express
really tangible. Thanks for it Emmanuel.

One technical point, we need to make Cursor close() operations idempotent
if they are not already - meaning if we close a second time this should not
cause an exception or change the outcome.


> On the client side, this is another issue... As cursors are created by the
> client code, we have no easy way to determinate when we should close the
> cursors, except when the connection is closed or an abandon request/unbind
> request is sent. Of course, when the server returns a searchResultDone we
> could also close the cursor. Remains the situations where the client has
> fetched some entries (but not all), and haven't unbind nor abandonned the
> search.
>
>
I think the aspect for automatic closing of cursors is left to be managed
inside the server even though the API overlaps here.


> In any case, this is less critical as we don't have to deal with the txn
> layer. The client will just blow away with some nasty OOM sooner or
> later... but this is not worse than what we get with NamingEnumeration in
> JNDI, nah ?
>
>
Yup +1


> Have I covered all the server options ? Or did I miss something ?
>
>
>> This sounds like something that can be handled nicely using an aspect
>> oriented solution. Now these things are heavy if you use AspectJ or
>> something like that but other simpler solutions exist to bytecode splice
>> compiled code to automatically handle these things. Maybe our past
>> experiences with Aspects might make us reconsider.
>>
> A bit overkilling, IMO?
>
>
I'm feeling the same but thought it should be just put out there. However
we can achieve the same results perhaps with code or using a lighter
mechanism with Proxy's via CGlib or something similar. These are just raw
thought dumps so it's not a we SHOULD recommendation. Something to think
about.

-- 
Best Regards,
-- Alex

Re: [index] OneLevelIndex removal

[Emmanuel Lécharny <el...@gmail.com>]

Forgot to reply to this mail, which raises interesting points.

More inside.

Le 4/11/12 10:38 PM, Alex Karasulu a écrit :
> On Wed, Apr 11, 2012 at 4:04 PM, Emmanuel Lécharny<el...@gmail.com>wrote:
>
>> I think we should add some mechanism in the server to check that
>> automatically, to avoid doing it by hand (there are hundreds of tests to
>> check...). One solution would be to keep a track of every cursor
>> construction in a HashMap, and to remove them when the cursor is closed.
>> The remaining cursors are likely not closed.
>
> It would be nice to have a Cursor monitor that every opened Cursor
> registers with but this needs to happen automatically. Then when out of the
> creation scope the Cursor is expected to be closed and if not this is
> handled automatically. However does creation scope work well since
> sometimes we create Cursors and pass them up?
We do have a monitor, which is currently used to check that the cursor 
is not closed when we try to use it. We certainly can use this monitor 
for more than just checking such thing.

Now, the pb is that the scope is not as easy to determinate than for a 
variable in Java. For instance, if we consider persistent searches, or 
paged searches, or even an abandonned search request, the scope is 
pretty wide...

Though we can have a set of rules that help us to close the cursor 
automatically :
- if we get an exception during a SearchRequest, then the cursors must 
be closed immediately. As soon as we store the cursors into the 
SearchContext, this is pretty easy to do
- an AbandonRequest will close the cursor automatically too (getting the 
cursor from the abandonned request)
- when we process the SearchResultDone, we can also close the cursor for 
the current search request (this work for PagedSearch too)
- for pagedSearch, if the user reset the search by sending 0 as the 
expected number of entries to return, then the cursor will be freed
- for persistent searches, as it will be closed by an unbind or an 
abandon request, we are fine
- when a client unbinds, then all the pending cursors will be closed.

All in all, we have everything needed to close the cursors 
automatically, assuming we keep all the cursors into the session.

On the client side, this is another issue... As cursors are created by 
the client code, we have no easy way to determinate when we should close 
the cursors, except when the connection is closed or an abandon 
request/unbind request is sent. Of course, when the server returns a 
searchResultDone we could also close the cursor. Remains the situations 
where the client has fetched some entries (but not all), and haven't 
unbind nor abandonned the search.

In any case, this is less critical as we don't have to deal with the txn 
layer. The client will just blow away with some nasty OOM sooner or 
later... but this is not worse than what we get with NamingEnumeration 
in JNDI, nah ?

Have I covered all the server options ? Or did I miss something ?
>
> This sounds like something that can be handled nicely using an aspect
> oriented solution. Now these things are heavy if you use AspectJ or
> something like that but other simpler solutions exist to bytecode splice
> compiled code to automatically handle these things. Maybe our past
> experiences with Aspects might make us reconsider.
A bit overkilling, IMO?

Thanks !


-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: [index] OneLevelIndex removal

[Alex Karasulu <ak...@apache.org>]

On Wed, Apr 11, 2012 at 4:04 PM, Emmanuel Lécharny <el...@gmail.com>wrote:

> Hi guys !
>
> so I completely removed the OneLevelIndex from the server. The branch
> (index) has been successfully merged back into trunk, and I will now work
> on removing the SublevelIndex from the index branch.
>
> In the process, I spent 3 days closing all the cursors that weren't closed
> after having been used. This was *BORING*. In the future, I would really
> appreciate if those who use the cursors double check that they have closed
> them.
>
> To do that, I added some logs in every cursor constructor and every
> close() method, and matched the opens with the closes. Do'nt ask me if this
> was fun to match them... I created a small program which was able to do
> that for me, but this is not enough to know that a cursor has been created
> but not closed, we also have to know where it has been created.
>
> I think we should add some mechanism in the server to check that
> automatically, to avoid doing it by hand (there are hundreds of tests to
> check...). One solution would be to keep a track of every cursor
> construction in a HashMap, and to remove them when the cursor is closed.
> The remaining cursors are likely not closed.


It would be nice to have a Cursor monitor that every opened Cursor
registers with but this needs to happen automatically. Then when out of the
creation scope the Cursor is expected to be closed and if not this is
handled automatically. However does creation scope work well since
sometimes we create Cursors and pass them up?

This sounds like something that can be handled nicely using an aspect
oriented solution. Now these things are heavy if you use AspectJ or
something like that but other simpler solutions exist to bytecode splice
compiled code to automatically handle these things. Maybe our past
experiences with Aspects might make us reconsider.


> The pb is that it gives no clue about where those cursors have been
> created, unless we associate a stackTrace to this information. Really not
> possible in production, but we might add an extended request to activate
> this mode, or a flag in the config.
>
> If anyone has a better idea ?
>
>
An aspect oriented solution might work well here.


> Thanks !
>
>
> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>
>


-- 
Best Regards,
-- Alex