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
