On 11/09/2009 05:32 PM, Simo Sorce wrote: > On Mon, 2009-11-09 at 15:57 -0500, Stephen Gallagher wrote: >> On 11/09/2009 02:20 PM, Simo Sorce wrote: >> >>> So here are 2 patches: >>> 0001 add some functions I needed for the task to sysdb and reworks a few >>> others >> >> Nack. >> sysdb_delete_user_send() should check whether both name and UID are >> specified and fail if they are. Right now, if they are both set, we are >> silently only using the name. Similarly, if neither is set (name==NULL >> and uid==0) we should fail. >> >> Same comments about sysdb_delete_group_send(). >> >> Otherwise it looks fine. > > Ok I added code to verify that the uid/gid match the name being > searched. > >>> >>> 0002 the actual cleanup task files. >> >> Nack. >> >> Please update manpages for ldap_purge_cache timeout, as well as the >> SSSDConfig API configuration files. > > Like for other timeouts I'd like to keep this undocumented for now. > I added the option for the API configuration file. > >> I don't like using SDAP_CACHE_PURGE_TIMEOUT for both the interval >> between purge attempts AND the timeout for the purge running. Please add >> one second to the interval time. Otherwise we're going to start queuing >> up transaction waits on subsequent timeout tries if for some reason the >> remote server is being very slow or unresponsive to our requests. Over a >> long enough period of time, this could be a genuine resource problem. >> I'd much prefer to guarantee that only one such request can be running >> at any given time. > > No this can't happen, new runs are not schedule until the previous run > is over. > >> The general approach looks fine to me, though I see a lot of duplication >> between the cleanup_users_send() and cleanup_groups_send() functions and >> I can't help wondering if there's a way that these could be made into a >> single set of functions. > > I know but I prefer to do things right first and optimize later, > although there is some duplication here I preferred to keep things > simpler than trying to add switch/case or if/else along the code to > address all the differences. > >> Also, can you add a set of tests to the sysdb-tests to manually set some >> cache expiration times to "1" and verify that the cleanup task clears >> them appropriately. (For regression-detection) > > I'll open a bug to add sysdb-tests for these cases. > > Also while checking the enumeration/cleanup code about the tiemouts I > find out there where 2 places where the wrong timeout being set. That > would have caused busy loops, because we were telling the code to > schedule the next task for "now" in case we were offline, oops :) > > I also changed the cleanup send function to take a memory context so > that when called from the enumeration task it will actually be a child > of that task (therefore it will be killed if the enum task timeout > kicks-in). > > Simo. > > > > > > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel
Patch 0001: Nack, causes segfault in sysdb-tests. Attaching backtrace. Patch 0002: Ack. -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
#0 0x000000000041225f in sldb_request_callback (ldbreq=0x205fe00,
ldbreply=0x205ffa0) at ../../server/db/sysdb_ops.c:145
req = 0x0
state = 0x7f06945d359b
err = 0
__FUNCTION__ = "sldb_request_callback"
#1 0x00007f06945cd1ca in asq_reqs_callback (req=<value optimized out>,
ares=0x2061470) at modules/asq.c:184
ret = <value optimized out>
#2 0x00007f06945df7bb in ltdb_index_filter (ac=<value optimized out>,
dn_list=<value optimized out>)
at ldb_tdb/ldb_index.c:1056
dn = 0x20679c0
ret = 0
ldb = 0x206b300
msg = 0x20675b0
i = <value optimized out>
#3 ltdb_search_indexed (ac=<value optimized out>, dn_list=<value optimized
out>) at ldb_tdb/ldb_index.c:1138
ldb = <value optimized out>
data = <value optimized out>
ltdb = <value optimized out>
dn_list = 0x2061330
ret = <value optimized out>
#4 0x00007f06945dcad1 in ltdb_search (ctx=0x206d8f0) at
ldb_tdb/ldb_search.c:538
ldb = 0x206b300
module = 0x206b3f0
req = 0x2061d30
data = <value optimized out>
ret = 1
#5 0x00007f06945dbec8 in ltdb_callback (ev=<value optimized out>, te=<value
optimized out>, t={tv_sec = 0, tv_usec = 0},
private_data=<value optimized out>) at ldb_tdb/ldb_tdb.c:1096
ctx = 0x206d8f0
ret = 53
#6 0x0000003fbe002f25 in tevent_common_loop_timer_delay (ev=0x205f010) at
tevent_timed.c:254
current_time = {tv_sec = 0, tv_usec = 0}
te = 0x2067860
#7 0x0000003fbe00455b in std_event_loop_once (ev=0x205f010) at
tevent_standard.c:543
tval = {tv_sec = 0, tv_usec = 0}
#8 0x00000000004053c9 in test_loop (data=0x206c390) at
../../server/tests/sysdb-tests.c:174
No locals.
#9 0x0000000000409fd4 in test_sysdb_asq_search (_i=0) at
../../server/tests/sysdb-tests.c:2183
test_ctx = 0x205ee50
data = 0x206c390
ret = 0
msgs_count = 139666233484176
msgs = 0x2057740
i = 0
req = 0x2060fa0
user_dn = 0x206b470
gid_str = 0x2057c80 "\240w\5\2"
#10 0x00007f06943bfbb8 in srunner_run_all () from /usr/lib64/libcheck.so.0
No symbol table info available.
#11 0x000000000040b3dd in main (argc=1, argv=0x7ffffe9ac1f8) at
../../server/tests/sysdb-tests.c:2496
opt = -1
ret = 0
pc = 0x2057740
failure_count = 0
sysdb_suite = 0x2057a80
sr = 0x2057c80
long_options = {{longName = 0x0, shortName = 0 '\0', argInfo = 4, arg =
0x636d60, val = 0,
descrip = 0x42d434 "Help options:", argDescrip = 0x0}, {longName =
0x42d442 "debug-level", shortName = 100 'd',
argInfo = 2, arg = 0x636e40, val = 0, descrip = 0x42d44e "Debug
level", argDescrip = 0x0}, {
longName = 0x42d45a "debug-to-files", shortName = 102 'f', argInfo
= 0, arg = 0x636e48, val = 0,
descrip = 0x42d470 "Send the debug output to files instead of
stderr", argDescrip = 0x0}, {
longName = 0x42d4a1 "debug-timestamps", shortName = 0 '\0', argInfo
= 0, arg = 0x636e44, val = 0,
descrip = 0x42d4b2 "Add debug timestamps", argDescrip = 0x0},
{longName = 0x0, shortName = 0 '\0', argInfo = 0,
arg = 0x0, val = 0, descrip = 0x0, argDescrip = 0x0}}
signature.asc
Description: OpenPGP digital signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
