directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Selcuk AYA <ayasel...@gmail.com>
Subject Re: How do we handle recursive requests wrt transactions ?
Date Wed, 21 Dec 2011 17:36:03 GMT
On Wed, Dec 21, 2011 at 12:30 PM, Emmanuel Lecharny <elecharny@gmail.com> wrote:
> 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.

I agree that option 1 is better and what the user experiences will be
the same with what he experiences through ldap request handlers.

One thing that might make life easier is if we do this thing at
CoreSession layer. I originally was against this as layers above
CoreSession might want to group requests into txns. However we can
still allow the layers above CoreSession group multiple requests into
a txn by checking if txn is already enabled at CoreSession layer.
Overall, this might decrease the number of changes.

>
>
>>
>>
>> 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
>

regards
Selcuk

Mime
View raw message