Full_Name: Hallvard B Furuseth Version: HEAD, RE23, RE24 OS: URL: Submission from: (NULL) (129.240.6.233) Submitted by: hallvard slapd/operation.c:slap_op_time() can decrease last_time: thread 1: *t = slap_get_time(); <clock: increment time()> thread 2: *t = slap_get_time(); thread 2: with mutex lock: increment last_time, set last_incr = 0; thread 1: with mutex lock: decrement last_time, set last_incr = 0; The simplest fix is to move the slap_get_time() call inside the (global) slap_op_mutex lock. However as far as I can tell, only the accesslog overlay uses op->o_tincr, the value which needs the mutex. And accesslog calls slap_op_time() itself when it needs that value. So maybe we should remove the op->o_tincr field. Other calls to slap_op_time() can be replaced with slap_get_time().
h.b.furuseth@usit.uio.no wrote: > Full_Name: Hallvard B Furuseth > Version: HEAD, RE23, RE24 > OS: > URL: > Submission from: (NULL) (129.240.6.233) > Submitted by: hallvard > > > slapd/operation.c:slap_op_time() can decrease last_time: > > thread 1: *t = slap_get_time(); > <clock: increment time()> > thread 2: *t = slap_get_time(); > thread 2: with mutex lock: increment last_time, set last_incr = 0; > thread 1: with mutex lock: decrement last_time, set last_incr = 0; > > The simplest fix is to move the slap_get_time() call inside the > (global) slap_op_mutex lock. > > However as far as I can tell, only the accesslog overlay uses > op->o_tincr, the value which needs the mutex. And accesslog > calls slap_op_time() itself when it needs that value. > So maybe we should remove the op->o_tincr field. Other calls > to slap_op_time() can be replaced with slap_get_time(). Sounds like a good idea. It was somewhat redundant with the CSN counter (except that this was for all operations, and CSN was only for write operations) and it never became generally useful. (Shifted to microsecond timestamps in 2.4, because tincr isn't useful across multiple servers.) I guess it makes more sense to make it specific to accesslog. -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
moved from Incoming to Software Bugs
changed notes changed state Open to Test
hyc@symas.com wrote: > h.b.furuseth@usit.uio.no wrote: >> However as far as I can tell, only the accesslog overlay uses >> op->o_tincr, the value which needs the mutex. And accesslog >> calls slap_op_time() itself when it needs that value. >> So maybe we should remove the op->o_tincr field. Other calls >> to slap_op_time() can be replaced with slap_get_time(). > > Sounds like a good idea. It was somewhat redundant with the CSN counter > (except that this was for all operations, and CSN was only for write > operations) and it never became generally useful. (Shifted to microsecond > timestamps in 2.4, because tincr isn't useful across multiple servers.) I > guess it makes more sense to make it specific to accesslog. > Except that this will take a lot more restructuring in accesslog. Going with the quick fix for now. -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
hyc@symas.com writes: > Except that this will take a lot more restructuring in accesslog. Going with > the quick fix for now. Looks like I didn't notice the first use of o_tincr in accesslog:-( I guess accesslog can set an "I need o_tincr" flag somewhere, which slapd will then obey. -- Hallvard
moved from Software Bugs to Development
changed notes changed state Test to Partial
quick fix in HEAD needs to be restructured later