On Thu, Apr 12, 2012 at 4:35 PM, Emmanuel Lécharny <elecharny@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<elecharny@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