directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject Re: How do we handle recursive requests wrt transactions ?
Date Wed, 21 Dec 2011 10:30:39 GMT
Hi Selcuk,

first of all, let me tell you what I'm currently doing.

I started to look at the server to see if I can get the tests working, 
and I did that in my spare time while I was visiting Roma, which means I 
wasn't able to spend a lot of time on the code. That can explain some of 
my vague mails. For instance, yesterday I was proposing something 
totally off base, like moving txns interfaces to shared, which is a 
totally insane proposal. Anyways...

On 12/21/11 7:13 AM, Selcuk AYA wrote:
> Since you cannot use triggers right now, I am assuming you are trying
> to do the recursive delete thorugh ldap core connection?

Yes. We have a test which does a recursive delete, and uses encapsulated 
searches for that. the code is the following :
     private void recursivelyDelete( Dn rdn ) throws Exception
     {
         EntryCursor results = reusableAdminCon.search( rdn.getName(), 
"(objectClass=*)",
             SearchScope.ONELEVEL, "*" );

         while ( results.next() )
         {
             Entry result = results.get();
             Dn childRdn = result.getDn();
             recursivelyDelete( childRdn );
         }

         results.close();
         reusableAdminCon.delete( rdn );
     }

The issue here is that once we have entered the recursion, we have 
already created a txn, and the second search creates a new one, as the 
reusableAdminCon is a LdapCoreSessionConnection, the place that starts 
the transaction with a beginTransaction (see later).

That being said, let me answers you mail.
>
> As I tried to explain in my previous emails, the first thing to decide
> is to where to put the transaction boundaries:
>
> * For Ldap requests over the wite, ldap protocol handlers insert the
> txn boundaries.
Yes. It took me a bit of time to gather my brain to understand that, 
even if I based my modification in LdapCoreeSessionConnection on what 
you have done in the handlers, as you told me to do.
> * For ldap core connection, it seems you guys discussed and wanted to
> put txn boundary per request and want to close the txn when cursor is
> closed.
Yes. It seems to be the best solution, as we may not want to expect the 
users to explicitely close the transactions. It's already an issue to 
expect users to close the cursors it creates, if we also have to 
requires that they commit/rollback the transactions, we will get endless 
discussions and mails from angry users on the users mailing list (IMHO).

Of course, this is my perception of what would be the best from the 
users, we can discuss this aspect.
>
> when discussing with Emmanuel, I remember clearly that user should be
> aware that LDAP is not transactional
Yep. LDAP is *not* transactional (even if a recent RFC is trying to add 
this features to LDAP).
> and users should be aware of it.
They should. Ahem, that means they can't ignore it, but they will ignore 
it, because users don't RTFM.
> So if you are doing a recursive delete through ldap core connection
> and transaction boundaries are per request, then you should know that
> the whole thing is not a txn itself.
Yep. This is where I have a problem. Because if the txns are started 
when I do a search, and committed when I close the cursor, then I need 
to match each search with each close. Or that a search done inside a 
trransaction can find out that it's done *inside* a started transaction 
(something I'm not sure we can do).

>
> That said, if you insert txn boundaries per request, then you need to
> deal with a couple of things:
>
> * Handle txn commit, abort for search. As mentioned in previous
> emails, this can be done in cursor close.
Yes. I modified the close() and close(Exception) methods of the 
EntryToResponseCursor class :
     public void close() throws Exception {
         wrapped.close();

         try { txnManager.commitTransaction(); }
         catch( Exception e ) { // Do nothing }
     }

     public void close( Exception e ) throws Exception {
         wrapped.close( e );

         try { txnManager.abortTransaction(); }
         catch( Exception ee ) { // Do nothing }
     }

As you can see, the abort is done if we close the cursor with an exception.

Of course, the txnManager is injected when creating the cursor :

     public EntryToResponseCursor( int messageId, Cursor<Entry> wrapped, 
TxnManager txnManager ) {
         this.wrapped = wrapped;
         this.messageId = messageId;
         this.txnManager = txnManager;
     }

Now, I'm not sure that it's the best approach.

> *When a search cursor is open, another search cursor can be opened.
> *When a search cursor is open, a delete/add/modify request might be made.

Yes. This has to be handled too.
>
> If txn boundary is per request, then you have two options to handle the above:
> * Introduce txn ref count and close the txn context when the ref count
> drops to zero. This is similar to nested txns concept and if a nested
> txn aborts, the final enclosing txn aborts.
> * When a cursor is open and a delete/add/modify is made, read only txn
> has to be upgraded to read write txn. When the final enclosing txn
> commits, it can get a conlfict exception and the whole request has to
> be repeated.
>
> So, although we try to hide the txns from the user, the above approach
> is clearly not intuitive for the user as final cursor close determines
> txn boundary and txn has to be retried after a conflict exception.

Understood. And clearly, the above approach I exposed is not sufficient...
>
> We have two options:
> * Have a txn context per request. Store context with cursor, then when
> user switches into cursor, restore its context, when it is leaving
> cursor, remove txn context. This way you dont have to deal with txn
> ref count or txn upgrade. I did something like this with jdbm browsers
> so that implementation there was totally hidden from the upper layers.
>
> *In embedded case, leave it to the user to determine the txn
> boundaries. This way things like recursive delete thorugh ldap core
> connection would meaningfully work .
Now, I'm thinking that option 1 is probably better. All in all, if some 
user of our server (not embedded) is doing a recursive search, we have 
to guarantee that it will work, whatever add/delete/modify he does in 
the middle, and without exposing the txns. In embedded mode, we should 
do the same thing, IMO. If it works for server-integ, it should also 
work for core-integ.

>
>
> Last and not least, since more than one person is involved in the code
> now, please let us know what you plan to do before you do any anything
> about code. Many of the points above were discussed here and there but
> I did not see a clear decision made about them, at least in the
> emails.
Right now, nothing was committed (except some obvious bug being fixed). 
My previous mails were pushed in order to avoid doing the big mistake to 
break something that was more or less in a better state than what I 
currently have.

People with commit access are quite sane, and we don't find funny to 
break the build :)

FYI, I also browse the classes and fixed all the places where we were 
using a cursor without closing them. I will commit this part, as it's 
not impacting the current code.

Thanks Selcuk ! (more to come later as I will make progress)




-- 
Regards,
Cordialement,
Emmanuel L├ęcharny
www.iktek.com


Mime
View raw message