-----BEGIN PGP SIGNED MESSAGE----- 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 cn=monitor. > 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 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk9zKp4ACgkQ9GWxeeH+cXsTEgCeLmBRsixan4iDngXPRsIFHuAt Ka0AoJyRSGpVzBzdPC4OJiH14lq1q2j3 =YBtx -----END PGP SIGNATURE----- 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.