[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#7182) back-ldap monitoring improvements

Hash: SHA1

On 03/27/2012 11:25 AM, Howard Chu wrote:
> Ondrej Kuznik wrote:
>>> - better connection handling (connections should have stable identifiers)
>> So far, I have had no revelation on how to proceed on this one.
> What do you intend to do with such a connection identifier? If it's merely for
> monitor purposes, just use a simple counter. Obviously nothing in back-ldap
> currently needs such a thing, so I can't imagine that it would be used for
> anything else. You can increment the counter in getconn, when the connection
> is inserted into the tree (under the mutex).

It is merely for monitoring purposes, such that it is always obvious
which entry corresponds to which connection over repeated searches in

> Your patch 4 seems to be quite invasive already. How will the new monitor
> output compare to the existing output? Will this break any scripts that were
> using the old monitor entries?

Thanks for the input, when comparing the data before submission briefly,
I missed the difference that olmDbURIList and labeledURI are in
"cn=Connections,cn=database #,..." in HEAD while after my patching they
ended up in the "cn=database #,..." monitor entry (where they make more
sense but could indeed make for an incompatible change). I will update
this in the next patchset. There should be no other compatibility issues
as there was no other useful information and slapo-chain seems not to
actually enable monitoring of its backends (fixing that would make for
another change I do not have ready).

> You seem to have no-op'd out the monitor_db_close() cleanups. It would be a
> good idea to do all of the proper unregister/cleanup here.

Since I dropped the ad-hoc entry creation, there are now only the
monitor subsystems registered, for which there currently is no
unregister callback. During slapd shutdown back-monitor handles the
destruction and callbacks itself, the only time the no-oping could hurt
is when SLAPD_CONFIG_DELETE is enabled (as mentioned in the comment).

I can certainly add that callback to back-monitor bi->extra and use it,
however such change would either be a little naive or require changing
the actual type of the monitor_subsys variable to an AVL or an
LDAP_LIST_*. I did not feel confident mucking with this part of
back-monitor myself and judged that confining the intrusive changes only
to back-ldap/monitor.c would make the patchset's chances of inclusion
somewhat higher.

As you are the person to make the call, what is your opinion on this?
Shall I update the patch to do proper subsystem deregistration (and
prepare another that introduces such a thing to back-monitor), leave
that part as it is or do something completely different?

- --
Ondrej Kuznik
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/


This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you for understanding.