From a111165bab37e74bcaa76b1ba6182549a785361d Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Wed, 20 Nov 2013 09:08:50 -0500 Subject: [PATCH 57/65] Ticket 47599 - Reduce lock scope in retro changelog plug-in Description: Use RW locks for protecting the change numbers. We still need to do the locking in retrocl_po.c as we need to serialize the actual updates. https://fedorahosted.org/389/ticket/47599 Reviewed by: richm(Thanks!!) (cherry picked from commit e2c42bced86bac235ac56ae98eed303f61ebd15e) (cherry picked from commit 03f6347eb72d3cbb49ae33312f32df9f91a2fd4c) --- ldap/servers/plugins/retrocl/retrocl.c | 3 ++- ldap/servers/plugins/retrocl/retrocl.h | 1 + ldap/servers/plugins/retrocl/retrocl_cn.c | 42 +++++++++++++++++++++---------- ldap/servers/plugins/retrocl/retrocl_po.c | 2 +- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/ldap/servers/plugins/retrocl/retrocl.c b/ldap/servers/plugins/retrocl/retrocl.c index 90c3455..3e426a7 100644 --- a/ldap/servers/plugins/retrocl/retrocl.c +++ b/ldap/servers/plugins/retrocl/retrocl.c @@ -465,7 +465,8 @@ retrocl_plugin_init(Slapi_PBlock *pb) if (!is_betxn) { rc= slapi_register_plugin_ext("internalpostoperation", 1 /* Enabled */, "retrocl_internalpostop_init", retrocl_internalpostop_init, "Retrocl internal postoperation plugin", NULL, identity, precedence); } - + retrocl_cn_lock = slapi_new_rwlock(); + if(retrocl_cn_lock == NULL) return -1; retrocl_internal_lock = PR_NewLock(); if (retrocl_internal_lock == NULL) return -1; } diff --git a/ldap/servers/plugins/retrocl/retrocl.h b/ldap/servers/plugins/retrocl/retrocl.h index 276912b..bfebe2e 100644 --- a/ldap/servers/plugins/retrocl/retrocl.h +++ b/ldap/servers/plugins/retrocl/retrocl.h @@ -130,6 +130,7 @@ extern const char *attr_nsuniqueid; extern const char *attr_isreplicated; extern PRLock *retrocl_internal_lock; +extern Slapi_RWLock *retrocl_cn_lock; /* Functions */ diff --git a/ldap/servers/plugins/retrocl/retrocl_cn.c b/ldap/servers/plugins/retrocl/retrocl_cn.c index d2b15a4..f816730 100644 --- a/ldap/servers/plugins/retrocl/retrocl_cn.c +++ b/ldap/servers/plugins/retrocl/retrocl_cn.c @@ -163,8 +163,9 @@ int retrocl_get_changenumbers(void) NULL,NULL,0,&cr,NULL,handle_cnum_result, handle_cnum_entry, NULL); - retrocl_first_cn = cr.cr_cnum; + slapi_rwlock_wrlock(retrocl_cn_lock); + retrocl_first_cn = cr.cr_cnum; slapi_ch_free(( void **) &cr.cr_time ); slapi_seq_callback(RETROCL_CHANGELOG_DN,SLAPI_SEQ_LAST, @@ -178,6 +179,8 @@ int retrocl_get_changenumbers(void) retrocl_first_cn, retrocl_internal_cn); + slapi_rwlock_unlock(retrocl_cn_lock); + slapi_ch_free(( void **) &cr.cr_time ); return 0; @@ -238,10 +241,10 @@ time_t retrocl_getchangetime( int type, int *err ) void retrocl_forget_changenumbers(void) { - PR_Lock(retrocl_internal_lock); + slapi_rwlock_wrlock(retrocl_cn_lock); retrocl_first_cn = 0; retrocl_internal_cn = 0; - PR_Unlock(retrocl_internal_lock); + slapi_rwlock_unlock(retrocl_cn_lock); } /* @@ -258,9 +261,11 @@ void retrocl_forget_changenumbers(void) changeNumber retrocl_get_first_changenumber(void) { changeNumber cn; - PR_Lock(retrocl_internal_lock); + + slapi_rwlock_rdlock(retrocl_cn_lock); cn = retrocl_first_cn; - PR_Unlock(retrocl_internal_lock); + slapi_rwlock_unlock(retrocl_cn_lock); + return cn; } @@ -277,9 +282,9 @@ changeNumber retrocl_get_first_changenumber(void) void retrocl_set_first_changenumber(changeNumber cn) { - PR_Lock(retrocl_internal_lock); + slapi_rwlock_wrlock(retrocl_cn_lock); retrocl_first_cn = cn; - PR_Unlock(retrocl_internal_lock); + slapi_rwlock_unlock(retrocl_cn_lock); } @@ -297,9 +302,11 @@ void retrocl_set_first_changenumber(changeNumber cn) changeNumber retrocl_get_last_changenumber(void) { changeNumber cn; - PR_Lock(retrocl_internal_lock); + + slapi_rwlock_rdlock(retrocl_cn_lock); cn = retrocl_internal_cn; - PR_Unlock(retrocl_internal_lock); + slapi_rwlock_unlock(retrocl_cn_lock); + return cn; } @@ -316,9 +323,11 @@ changeNumber retrocl_get_last_changenumber(void) void retrocl_commit_changenumber(void) { + slapi_rwlock_wrlock(retrocl_cn_lock); if ( retrocl_first_cn == 0) { retrocl_first_cn = retrocl_internal_cn; } + slapi_rwlock_unlock(retrocl_cn_lock); } /* @@ -333,8 +342,10 @@ void retrocl_commit_changenumber(void) */ void retrocl_release_changenumber(void) -{ +{ + slapi_rwlock_wrlock(retrocl_cn_lock); retrocl_internal_cn--; + slapi_rwlock_unlock(retrocl_cn_lock); } /* @@ -342,7 +353,7 @@ void retrocl_release_changenumber(void) * * Returns: 0/-1 * - * Arguments: none + * Arguments: none. The caller should have taken write lock for the change numbers * * Description: reads the last entry in the changelog to obtain * the last change number. @@ -355,6 +366,7 @@ int retrocl_update_lastchangenumber(void) if (retrocl_be_changelog == NULL) return -1; + slapi_rwlock_unlock(retrocl_cn_lock); cr.cr_cnum = 0; cr.cr_time = 0; slapi_seq_callback(RETROCL_CHANGELOG_DN,SLAPI_SEQ_LAST, @@ -362,7 +374,7 @@ int retrocl_update_lastchangenumber(void) NULL,NULL,0,&cr,NULL,handle_cnum_result, handle_cnum_entry, NULL); - + slapi_rwlock_wrlock(retrocl_cn_lock); retrocl_internal_cn = cr.cr_cnum; slapi_log_error(SLAPI_LOG_PLUGIN,"retrocl","Refetched last changenumber = %lu \n", retrocl_internal_cn); @@ -394,6 +406,8 @@ changeNumber retrocl_assign_changenumber(void) * validity of the internal assignment of retrocl_internal_cn * we had from the startup */ + slapi_rwlock_wrlock(retrocl_cn_lock); + if(retrocl_internal_cn <= retrocl_first_cn){ /* the numbers have become out of sync - retrocl_get_changenumbers * gets called only once during startup and it may have had a problem @@ -404,8 +418,10 @@ changeNumber retrocl_assign_changenumber(void) */ retrocl_update_lastchangenumber(); } - retrocl_internal_cn++; cn = retrocl_internal_cn; + + slapi_rwlock_unlock(retrocl_cn_lock); + return cn; } diff --git a/ldap/servers/plugins/retrocl/retrocl_po.c b/ldap/servers/plugins/retrocl/retrocl_po.c index 382c98a..cd290f2 100644 --- a/ldap/servers/plugins/retrocl/retrocl_po.c +++ b/ldap/servers/plugins/retrocl/retrocl_po.c @@ -372,7 +372,7 @@ write_replog_db( retrocl_release_changenumber(); } else { /* Tell the change numbering system this one's committed to disk */ - retrocl_commit_changenumber( ); + retrocl_commit_changenumber(); } } else { slapi_log_error( SLAPI_LOG_FATAL, RETROCL_PLUGIN_NAME, -- 1.8.1.4