Hello,
My name is Michał Szulczyński, and I'm a student at Warszawska Wyższa Szkoła Informatyki in Poland. I'm on a practice at the company where Aleksander Adamowski works. As he said, I will try to implement dynamic list member 'caching'. The general idea is to "materialize" the member attribute of the dynamic group using existing static attribute values infrastructure (including indexes) and update them automatically when there is a add/modify/modrdn/delete operation on an object that matches the dynlist memberURL filter.
The operation of dynlist overlay would be identical as it is now without specifying an additional dynlist configuration option.
The plan is to, depending on the additional, optional dynlist-attrset configuration parameter, grab the dynamic list entries for that dynlist configuration from the database, and put them into a memory-resident list (which would hold all the dynamic groups that we are interested in, ie. the ones with the additional configuration option set). When there is a add/mod/modrdn/delete operation on any entry, we would check this list for a matching dynlist filter, and add/modify/delete the member attribute of the dynamic list for that entry. Much like a static group, but with automated addition/deletion to/from it, effectively making it a dynamic group. Manual updates to the member attribute would be prevented and would raise an error.
First, in addition to dynlist_info_t we would need to hold the pointer to our memory-resident dynamic group list per dynlist overlay instance. That's for speed optimization, since searching for all dynamic lists on each update operation would have a huge performance impact. As we can only hold only one backend-specific config data in Operation->o_bd->bd_info->on_bi.bi_private, we wold need to hold both pointers (dynlist_info_t and the pointer to our list) in an additional struct, and store that. But this method would mean changing much of the existing codebase, and I don't know if that would be good. If there would be another place where we could store the pointer to the dynamic group list, that would be better.
Second, I would like to know if there is a function to test whether one DN is contained in another DN (i.e. one DN is a descendant to another DN). I need this to compare the base DN of the dynamic group, and the added/modified/deleted/modrdn'ed entry (eg: if the entry DN <uid=test,l=City,ou=People,o=MyOrg> is in the dynamic group's member URL's base DN <ou=People,o=MyOrg>). I think that comparing the normalized DN's stringwise backwards from the strings' end would work, but I'm not sure.
Third: * can we do an add-or-replace with BackendDB->be_modify * when doing a LDAP_MOD_DELETE on a single attribute's value can we ignore a case where that attribute value does not exist in the entry (so we can avoid checking for its presence and save an unnecessary read operation)?
-- Michał Szulczyński Praktykant Altkom Akademia S.A. http://www.altkom.pl Warszawa, ul. Chłodna 51
Sąd Rejonowy dla m.st. Warszawy w Warszawie, XII Wydział Gospodarczy Krajowego Rejestru Sądowego, KRS: 0000120139, NIP 118-00-08-391, Kapitał zakładowy: 1000 000 PLN. Adres rejestrowy Firmy - ul. Stawki 2, 00-193 Warszawa. Niniejsza wiadomość zawiera informacje zastrzeżone i stanowiące tajemnicę przedsiębiorstwa firmy Altkom Akademia S.A. Ujawnianie tych informacji osobom trzecim lub nieuprawnione wykorzystanie ich do własnych celów jest zabronione. Jeżeli otrzymaliście Państwo niniejszą wiadomość omyłkowo, prosimy o niezwłoczne skontaktowanie się z nadawcą oraz usunięcie wszelkich kopii niniejszej wiadomości. This message contains proprietary information and trade secrets of Altkom Akademia S.A. company. Unauthorized use or disclosure of this information to any third party is prohibited. If you received this message by mistake, please contact the sender immediately and delete all copies of this message.
Michał Szulczyński wrote:
Hello,
My name is Michał Szulczyński, and I'm a student at Warszawska Wyższa Szkoła Informatyki in Poland. I'm on a practice at the company where Aleksander Adamowski works. As he said, I will try to implement dynamic list member 'caching'. The general idea is to "materialize" the member attribute of the dynamic group using existing static attribute values infrastructure (including indexes) and update them automatically when there is a add/modify/modrdn/delete operation on an object that matches the dynlist memberURL filter.
The operation of dynlist overlay would be identical as it is now without specifying an additional dynlist configuration option.
It is not clear from your description whether you plan to simply replace dynamic grouping (dynamically listing member DNs within a dynamic group entry) or dynamically listing any type of attribute. The dynlist overlay right now does both, but I think the feature you're considering only makes sense for the first case.
I suggest that for the duration of the development you branch your overlay into something other than dynlist (call it rdynlist, for really dynamic list?). When you get to something stable and performing better than dynlist (with "better" I don't barely mean performances, but also functionality, in your case including consistency), we could decide whether to move it back to dynlist or leave it alone and, in case, drop dynlist itself.
The plan is to, depending on the additional, optional dynlist-attrset configuration parameter, grab the dynamic list entries for that dynlist configuration from the database, and put them into a memory-resident list (which would hold all the dynamic groups that we are interested in, ie. the ones with the additional configuration option set). When there is a add/mod/modrdn/delete operation on any entry, we would check this list for a matching dynlist filter, and add/modify/delete the member attribute of the dynamic list for that entry. Much like a static group, but with automated addition/deletion to/from it, effectively making it a dynamic group. Manual updates to the member attribute would be prevented and would raise an error.
First, in addition to dynlist_info_t we would need to hold the pointer to our memory-resident dynamic group list per dynlist overlay instance. That's for speed optimization, since searching for all dynamic lists on each update operation would have a huge performance impact.
Not sure what do you mean by "dynamic group list" here. In any case, the Operation->o_bd->bd_info->on_bi.bi_private is supposed to be read-only, as it is shared among threads. If you need to modify any data in there you need to protect it from concurrency.
As we can only hold only one backend-specific config data in Operation->o_bd->bd_info->on_bi.bi_private, we wold need to hold both pointers (dynlist_info_t and the pointer to our list) in an additional struct, and store that. But this method would mean changing much of the existing codebase, and I don't know if that would be good. If there would be another place where we could store the pointer to the dynamic group list, that would be better.
Second, I would like to know if there is a function to test whether one DN is contained in another DN (i.e. one DN is a descendant to another DN).
dnIsSuffix(), provided both DNs are normalized.
I need this to compare the base DN of the dynamic group, and the added/modified/deleted/modrdn'ed entry (eg: if the entry DN <uid=test,l=City,ou=People,o=MyOrg> is in the dynamic group's member URL's base DN <ou=People,o=MyOrg>). I think that comparing the normalized DN's stringwise backwards from the strings' end would work, but I'm not sure.
Third:
- can we do an add-or-replace with BackendDB->be_modify
what is an "add-or-replace"? If you mean the ldap modify operation type, LDAP_MOD_ADD adds the value(s), unless it (any of them) already exist(s) (in this case the return code will be LDAP_TYPE_OR_VALUE_EXISTS). The LDAP_MOD_REPLACE replaces all existing values with the provided ones. If what you need to do is add a value unless it already exists, you can set the o_permissive_modify flag, so that LDAP_MOD_ADD on existing values are ignored.
- when doing a LDAP_MOD_DELETE on a single attribute's value can we
ignore a case where that attribute value does not exist in the entry (so we can avoid checking for its presence and save an unnecessary read operation)?
Yes: you perform an internal LDAP_MOD_DELETE, and ignore the return code LDAP_NO_SUCH_ATTRIBUTE.
p.
Ing. Pierangelo Masarati OpenLDAP Core Team
SysNet s.r.l. via Dossi, 8 - 27100 Pavia - ITALIA http://www.sys-net.it --------------------------------------- Office: +39 02 23998309 Mobile: +39 333 4963172 Email: pierangelo.masarati@sys-net.it ---------------------------------------
Pierangelo Masarati wrote:
It is not clear from your description whether you plan to simply replace dynamic grouping (dynamically listing member DNs within a dynamic group entry) or dynamically listing any type of attribute. The dynlist overlay right now does both, but I think the feature you're considering only makes sense for the first case.
I mean the first case, as in holding the member DNs in the group's member attribute, dynamically updating it.
I suggest that for the duration of the development you branch your overlay into something other than dynlist (call it rdynlist, for really dynamic list?). When you get to something stable and performing better than dynlist (with "better" I don't barely mean performances, but also functionality, in your case including consistency), we could decide whether to move it back to dynlist or leave it alone and, in case, drop dynlist itself.
That might be a good idea, because while dynlist and what I'm planning to write do similar things, the code path differs significantly.
-- Michał Szulczyński Praktykant Altkom Akademia S.A. http://www.altkom.pl Warszawa, ul. Chłodna 51
Sąd Rejonowy dla m.st. Warszawy w Warszawie, XII Wydział Gospodarczy Krajowego Rejestru Sądowego, KRS: 0000120139, NIP 118-00-08-391, Kapitał zakładowy: 1000 000 PLN. Adres rejestrowy Firmy - ul. Stawki 2, 00-193 Warszawa. Niniejsza wiadomość zawiera informacje zastrzeżone i stanowiące tajemnicę przedsiębiorstwa firmy Altkom Akademia S.A. Ujawnianie tych informacji osobom trzecim lub nieuprawnione wykorzystanie ich do własnych celów jest zabronione. Jeżeli otrzymaliście Państwo niniejszą wiadomość omyłkowo, prosimy o niezwłoczne skontaktowanie się z nadawcą oraz usunięcie wszelkich kopii niniejszej wiadomości. This message contains proprietary information and trade secrets of Altkom Akademia S.A. company. Unauthorized use or disclosure of this information to any third party is prohibited. If you received this message by mistake, please contact the sender immediately and delete all copies of this message.
Hello,
I'm attaching the "Really dynamic list" overlay, on which I have been working for the past 2 weeks. This is the Technology Preview version, so I need your input on the implementation. Also feel free to criticize, or to point out the flaws in my understanding of OpenLDAP, or the implementation of this overlay.
I have tested it, and it works without problems (at least on my setup).
This overlay works by updating the dynamic list entry's member attribute (which is not modifiable by the user) on an add/delete/modify/modrdn operation, adding or deleting the updated entry's DN to/from the dynamic list when the entry matches the any of the memberURL filters of the dynamic list.
I have modified the dyngroup schema slightly, adding the 'member' attribute to the MAY clause. This is needed to store the 'materialized' member DN's in the dynamic list entry in the database.
The config is similar to the dynlist overlay, but with mandatory member attribute:
rdynlist-attrset <group-oc> <URL-ad> <member-ad>
* rdynlist.patch is the patch for the schema and the configure/makefiles. -- Michał Szulczyński Praktykant Altkom Akademia S.A. http://www.altkom.pl Warszawa, ul. Chłodna 51
Sąd Rejonowy dla m.st. Warszawy w Warszawie, XII Wydział Gospodarczy Krajowego Rejestru Sądowego, KRS: 0000120139, NIP 118-00-08-391, Kapitał zakładowy: 1000 000 PLN. Adres rejestrowy Firmy - ul. Stawki 2, 00-193 Warszawa. Niniejsza wiadomość zawiera informacje zastrzeżone i stanowiące tajemnicę przedsiębiorstwa firmy Altkom Akademia S.A. Ujawnianie tych informacji osobom trzecim lub nieuprawnione wykorzystanie ich do własnych celów jest zabronione. Jeżeli otrzymaliście Państwo niniejszą wiadomość omyłkowo, prosimy o niezwłoczne skontaktowanie się z nadawcą oraz usunięcie wszelkich kopii niniejszej wiadomości. This message contains proprietary information and trade secrets of Altkom Akademia S.A. company. Unauthorized use or disclosure of this information to any third party is prohibited. If you received this message by mistake, please contact the sender immediately and delete all copies of this message.
Michał Szulczyński wrote:
Hello,
I'm attaching the "Really dynamic list" overlay, on which I have been working for the past 2 weeks. This is the Technology Preview version, so I need your input on the implementation. Also feel free to criticize, or to point out the flaws in my understanding of OpenLDAP, or the implementation of this overlay.
Nice job, getting to working code.
You don't need to use config_generic_wrapper in this overlay, that's primarily for backward compatibility with nested databases. Fully dynamic modules just need to set bi_cf_ocs and leave bi_db_config unset.
I think the amount of comments is too few. It takes more than one read thru the code to understand what an rdynlist_entry_t is used for; this should have been noted in a comment. Likewise for rdynlist_filter_t.
In rdynlist_search_cb you shouldn't free rs->sr_entry yourself, that's the frontend's job. Calling entry_free() directly is usually the wrong thing to do anyway; you should use be_entry_release_rw or overlay_entry_release_ov.
In rdynlist_add_group() not sure what your "TODO check if alloc was successful" comment is about. The ch_* routines never return on failure, they simply abort. As such, it's unnecessary to check for success. You can of course use ber_memalloc directly and check for success/failure if you wish but there's usually no good reason to do so. I.e., once the server starts running out of memory, it's pretty much dead anyway.
Looks like you leak the URL descriptor on a normal run thru this function.
Again in rdynlist_group_add_cb don't free rs_sr_entry yourself.
In rdynlist_response() you call attrs_find to obtain an entry's objectclass (and other attributes) and then immediately release the entry. This is not thread-safe; once the entry is released it is possible for it to disappear (due to other operations cycling the backend's entry cache, if any. etc...). At that point the attribute pointer will be invalid. Either dup the attribute first, or don't release the entry until you're done with the attribute.
You should call build_new_dn() with the current op->o_tmpmemctx and use op->o_tmpfree() instead of ch_free since new_dn is just a temporary value.
Same entry_get/entry_release problems in rdynlist_modify_entry().
You have mismatched functionality in rdynlist_db_open/rdynlist_db_destroy. I.e., what you allocate in db_open should be freed in db_close, not db_destroy.
I have tested it, and it works without problems (at least on my setup).
Try it with a database with about 1 million users, with 900,000 or so members of a dynamic group.
While it's good that you got this code working, I still believe you've completely missed the point of dynamic groups.
This overlay works by updating the dynamic list entry's member attribute (which is not modifiable by the user) on an add/delete/modify/modrdn operation, adding or deleting the updated entry's DN to/from the dynamic list when the entry matches the any of the memberURL filters of the dynamic list.
I have modified the dyngroup schema slightly, adding the 'member' attribute to the MAY clause. This is needed to store the 'materialized' member DN's in the dynamic list entry in the database.
The config is similar to the dynlist overlay, but with mandatory member attribute:
rdynlist-attrset <group-oc> <URL-ad> <member-ad>
- rdynlist.patch is the patch for the schema and the
configure/makefiles.
Michał Szulczyński Praktykant Altkom Akademia S.A. http://www.altkom.pl Warszawa, ul. Chłodna 51
Sąd Rejonowy dla m.st. Warszawy w Warszawie, XII Wydział Gospodarczy Krajowego Rejestru Sądowego, KRS: 0000120139, NIP 118-00-08-391, Kapitał zakładowy: 1000 000 PLN. Adres rejestrowy Firmy - ul. Stawki 2, 00-193 Warszawa. Niniejsza wiadomość zawiera informacje zastrzeżone i stanowiące tajemnicę przedsiębiorstwa firmy Altkom Akademia S.A. Ujawnianie tych informacji osobom trzecim lub nieuprawnione wykorzystanie ich do własnych celów jest zabronione. Jeżeli otrzymaliście Państwo niniejszą wiadomość omyłkowo, prosimy o niezwłoczne skontaktowanie się z nadawcą oraz usunięcie wszelkich kopii niniejszej wiadomości. This message contains proprietary information and trade secrets of Altkom Akademia S.A. company. Unauthorized use or disclosure of this information to any third party is prohibited. If you received this message by mistake, please contact the sender immediately and delete all copies of this message.
Howard Chu wrote:
I think the amount of comments is too few. It takes more than one read thru the code to understand what an rdynlist_entry_t is used for; this should have been noted in a comment. Likewise for rdynlist_filter_t.
In the attached Technology Preview version no. 2 I have fixed all of the bugs You pointed out, and a few additional ones. I have also added the much needed comments.
I have tested it, and it works without problems (at least on my setup).
Try it with a database with about 1 million users, with 900,000 or so members of a dynamic group.
I see the problem, but even static groups with 900,000 member attributes are slow in updating, so this slowdown is unfortunately unavoidable.
While it's good that you got this code working, I still believe you've completely missed the point of dynamic groups.
I agree with you that this is not a truly dynamic group. This overlay is more of a "automatically updated static group," so maybe the name of this overlay is not representing what it does exactly. But without modifying much of the OpenLDAP codebase I don't see a better way to do it (but maybe I'm wrong, so feel free to correct me). -- Michał Szulczyński Praktykant Altkom Akademia S.A. http://www.altkom.pl Warszawa, ul. Chłodna 51
Sąd Rejonowy dla m.st. Warszawy w Warszawie, XII Wydział Gospodarczy Krajowego Rejestru Sądowego, KRS: 0000120139, NIP 118-00-08-391, Kapitał zakładowy: 1000 000 PLN. Adres rejestrowy Firmy - ul. Stawki 2, 00-193 Warszawa. Niniejsza wiadomość zawiera informacje zastrzeżone i stanowiące tajemnicę przedsiębiorstwa firmy Altkom Akademia S.A. Ujawnianie tych informacji osobom trzecim lub nieuprawnione wykorzystanie ich do własnych celów jest zabronione. Jeżeli otrzymaliście Państwo niniejszą wiadomość omyłkowo, prosimy o niezwłoczne skontaktowanie się z nadawcą oraz usunięcie wszelkich kopii niniejszej wiadomości. This message contains proprietary information and trade secrets of Altkom Akademia S.A. company. Unauthorized use or disclosure of this information to any third party is prohibited. If you received this message by mistake, please contact the sender immediately and delete all copies of this message.
Michał Szulczyński wrote:
While it's good that you got this code working, I still believe you've completely missed the point of dynamic groups.
I agree with you that this is not a truly dynamic group. This overlay is more of a "automatically updated static group," so maybe the name of this overlay is not representing what it does exactly. But without modifying much of the OpenLDAP codebase I don't see a better way to do it (but maybe I'm wrong, so feel free to correct me)
Actually, I originally thought of this as a "materialized" dynamic groups, per analogy to materialized views in RDBMS world.
So a more proper name for the overlay would be "mdynlist", not "rdynlist".
Aleksander Adamowski wrote:
Michał Szulczyński wrote:
While it's good that you got this code working, I still believe you've completely missed the point of dynamic groups.
I agree with you that this is not a truly dynamic group. This overlay is more of a "automatically updated static group," so maybe the name of this overlay is not representing what it does exactly. But without modifying much of the OpenLDAP codebase I don't see a better way to do it (but maybe I'm wrong, so feel free to correct me)
Actually, I originally thought of this as a "materialized" dynamic groups, per analogy to materialized views in RDBMS world.
So a more proper name for the overlay would be "mdynlist", not "rdynlist".
I think we can drop the dynlist reference since dynlist is much more general and this is specific to groups. I think something like "autogroup" would be more appropriate. If you're satisfied with the functionality of the code as-is we can probably add it to the contrib/slapd-modules tree. Read the contributing guidelines, then submit your package to the ITS.
-
Aleksander Adamowski -
Howard Chu -
Michał Szulczyński -
Pierangelo Masarati