[389-commits] ldap/servers
Noriko Hosoi
nhosoi at fedoraproject.org
Tue Apr 10 21:45:16 UTC 2012
ldap/servers/slapd/back-ldbm/cache.c | 2 -
ldap/servers/slapd/back-ldbm/id2entry.c | 16 ++++++---
ldap/servers/slapd/back-ldbm/import-threads.c | 2 -
ldap/servers/slapd/back-ldbm/index.c | 35 ++++++++++++---------
ldap/servers/slapd/back-ldbm/ldbm_add.c | 20 ++++++++++--
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 33 ++++++++++++++------
ldap/servers/slapd/back-ldbm/ldbm_modify.c | 41 ++++++++++++++++++++-----
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 23 ++++++++++++--
ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 2 -
9 files changed, 129 insertions(+), 45 deletions(-)
New commits:
commit bddb5a45f2cd27705cb6629f436ef9a7e2248677
Author: Noriko Hosoi <nhosoi at redhat.com>
Date: Tue Apr 10 14:40:27 2012 -0700
Trac Ticket #335 - transaction retries need to be cache aware
https://fedorahosted.org/389/ticket/335
Fix description:
When libdb returns DEADLOCK and backend update function retries
the operation, the target entry is reset to the original shape.
The target entry might be or might not be in the entry cache.
Regardless of the status, the original code just releases the
entry with backentry_free before going into the next loop, which
causes the cache error.
This patch checks the status of the entry. If it is in the entry
cache, it removes it from the entry cache and adds a new entry
back to the cache if necessary. To get the accurate cache status
of each entry, the output argument cache_res to id2entry_add_ext
is added.
Additinally, error checking for the conflict value in index_add_mods
was week (curr_attr). This patch is adding the check.
diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c
index 1ccfd29..2ef0f28 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -1050,7 +1050,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde,
}
#endif
/* adjust cache meta info */
- newe->ep_refcnt = 1;
+ newe->ep_refcnt++;
newe->ep_size = cache_entry_size(newe);
if (newe->ep_size > olde->ep_size) {
slapi_counter_add(cache->c_cursize, newe->ep_size - olde->ep_size);
diff --git a/ldap/servers/slapd/back-ldbm/id2entry.c b/ldap/servers/slapd/back-ldbm/id2entry.c
index ec7a7e9..e278a2a 100644
--- a/ldap/servers/slapd/back-ldbm/id2entry.c
+++ b/ldap/servers/slapd/back-ldbm/id2entry.c
@@ -48,9 +48,12 @@
/*
* The caller MUST check for DB_LOCK_DEADLOCK and DB_RUNRECOVERY returned
+ * If cache_res is not NULL, it stores the result of CACHE_ADD of the
+ * entry cache.
*/
int
-id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt )
+id2entry_add_ext(backend *be, struct backentry *e, back_txn *txn,
+ int encrypt, int *cache_res)
{
ldbm_instance *inst = (ldbm_instance *) be->be_instance_info;
DB *db = NULL;
@@ -138,8 +141,8 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
dblayer_release_id2entry( be, db );
- if (0 == rc)
- {
+ if (0 == rc) {
+ int cache_rc = 0;
/* Putting the entry into the entry cache.
* We don't use the encrypted entry here. */
if (entryrdn_get_switch()) {
@@ -200,7 +203,10 @@ id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt
* should be in the cache. Thus, this entry e won't be put into the
* entry cache. It'll be added by cache_replace.
*/
- (void) CACHE_ADD( &inst->inst_cache, e, NULL );
+ cache_rc = CACHE_ADD(&inst->inst_cache, e, NULL);
+ if (cache_res) {
+ *cache_res = cache_rc;
+ }
}
done:
@@ -217,7 +223,7 @@ done:
int
id2entry_add( backend *be, struct backentry *e, back_txn *txn )
{
- return id2entry_add_ext(be,e,txn,1);
+ return id2entry_add_ext(be, e, txn, 1, NULL);
}
/*
diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c
index 4c1129d..f60c749 100644
--- a/ldap/servers/slapd/back-ldbm/import-threads.c
+++ b/ldap/servers/slapd/back-ldbm/import-threads.c
@@ -2328,7 +2328,7 @@ import_foreman(void *param)
/* id2entry_add_ext replaces an entry if it already exists.
* therefore, the Entry ID stays the same.
*/
- ret = id2entry_add_ext(be, fi->entry, NULL, job->encrypt);
+ ret = id2entry_add_ext(be, fi->entry, NULL, job->encrypt, NULL);
if (ret) {
/* DB_RUNRECOVERY usually occurs if disk fills */
if (LDBM_OS_ERR_IS_DISKFULL(ret)) {
diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c
index da1952e..7e8aa12 100644
--- a/ldap/servers/slapd/back-ldbm/index.c
+++ b/ldap/servers/slapd/back-ldbm/index.c
@@ -550,8 +550,7 @@ index_add_mods(
/* Get a list of all values specified in the operation.
*/
if ( mods[i]->mod_bvalues != NULL ) {
- valuearray_init_bervalarray(mods[i]->mod_bvalues,
- &mods_valueArray);
+ valuearray_init_bervalarray(mods[i]->mod_bvalues, &mods_valueArray);
}
switch ( mods[i]->mod_op & ~LDAP_MOD_BVALUES ) {
@@ -622,20 +621,24 @@ index_add_mods(
} else {
/* Verify if the value is in newe.
* If it is in, we will add the attr value to the index file. */
- slapi_entry_attr_find( newe->ep_entry,
- mods[i]->mod_type, &curr_attr );
+ curr_attr = NULL;
+ slapi_entry_attr_find(newe->ep_entry,
+ mods[i]->mod_type, &curr_attr);
- for (j = 0; mods_valueArray[j] != NULL; j++) {
- /* mods_valueArray[j] is in curr_attr ==> return 0 */
- if (slapi_attr_value_find(curr_attr,
+ if (curr_attr) { /* found the type */
+ for (j = 0; mods_valueArray[j] != NULL; j++) {
+ /* mods_valueArray[j] is in curr_attr ==> return 0 */
+ if (slapi_attr_value_find(curr_attr,
slapi_value_get_berval(mods_valueArray[j]))) {
- /* The value is NOT in newe, remove it. */
- Slapi_Value *rval = valuearray_remove_value(curr_attr,
- mods_valueArray,
- mods_valueArray[j]);
- slapi_value_free( &rval );
- /* indicates there was some conflict */
- mods[i]->mod_op |= LDAP_MOD_IGNORE;
+ /* The value is NOT in newe, remove it. */
+ Slapi_Value *rval;
+ rval = valuearray_remove_value(curr_attr,
+ mods_valueArray,
+ mods_valueArray[j]);
+ slapi_value_free( &rval );
+ /* indicates there was some conflict */
+ mods[i]->mod_op |= LDAP_MOD_IGNORE;
+ }
}
}
if (mods_valueArray) {
@@ -724,7 +727,9 @@ index_add_mods(
* BE_INDEX_EQUALITY flag so the equality index is
* removed.
*/
- slapi_entry_attr_find( olde->ep_entry, mods[i]->mod_type, &curr_attr );
+ curr_attr = NULL;
+ slapi_entry_attr_find(olde->ep_entry,
+ mods[i]->mod_type, &curr_attr);
if (curr_attr) {
int found = 0;
for (j = 0; mods_valueArray[j] != NULL; j++ ) {
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c
index 1a0f882..1539c7c 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
@@ -695,16 +695,32 @@ ldbm_back_add( Slapi_PBlock *pb )
dblayer_txn_abort(li,&txn);
slapi_pblock_set(pb, SLAPI_TXN, parent_txn);
- backentry_free(&addingentry);
+ if (addingentry_in_cache) {
+ /* addingentry is in cache. Remove it once. */
+ CACHE_REMOVE(&inst->inst_cache, addingentry);
+ CACHE_RETURN(&inst->inst_cache, &addingentry);
+ } else {
+ backentry_free(&addingentry);
+ }
slapi_pblock_set( pb, SLAPI_ADD_ENTRY, originalentry->ep_entry );
addingentry = originalentry;
if ( (originalentry = backentry_dup( addingentry )) == NULL ) {
ldap_result_code= LDAP_OPERATIONS_ERROR;
goto error_return;
}
+ if (addingentry_in_cache) {
+ /* Adding the resetted addingentry to the cache. */
+ if (cache_add_tentative(&inst->inst_cache,
+ addingentry, NULL) != 0) {
+ LDAPDebug0Args(LDAP_DEBUG_CACHE,
+ "cache_add_tentative concurrency detected\n");
+ ldap_result_code = LDAP_ALREADY_EXISTS;
+ goto error_return;
+ }
+ }
/* We're re-trying */
- LDAPDebug( LDAP_DEBUG_TRACE, "Add Retrying Transaction\n", 0, 0, 0 );
+ LDAPDebug0Args(LDAP_DEBUG_BACKLDBM, "Add Retrying Transaction\n");
#ifndef LDBM_NO_BACKOFF_DELAY
{
PRIntervalTime interval;
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index 447ab84..fff93c9 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -90,6 +90,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from the DB */
int create_tombstone_entry = 0; /* We perform a "regular" LDAP delete but since we use */
/* replication, we must create a new tombstone entry */
+ int e_in_cache = 0;
int tombstone_in_cache = 0;
entry_address *addr;
int addordel_flags = 0; /* passed to index_addordel */
@@ -186,6 +187,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
/* retval is -1 */
goto error_return; /* error result sent by find_entry2modify() */
}
+ e_in_cache = 1;
if ( slapi_entry_has_children( e->ep_entry ) )
{
@@ -481,8 +483,18 @@ ldbm_back_delete( Slapi_PBlock *pb )
if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) {
dblayer_txn_abort(li,&txn);
slapi_pblock_set(pb, SLAPI_TXN, parent_txn);
-
- backentry_free(&e);
+ if (e_in_cache) {
+ /* entry 'e' is in the entry cache. Since we reset 'e' to
+ * the original_entry, remove it from the cache. */
+ CACHE_REMOVE(&inst->inst_cache, e);
+ cache_unlock_entry(&inst->inst_cache, e);
+ CACHE_RETURN(&inst->inst_cache, &e);
+ /* As we are about to delete it,
+ * we don't put the entry back to cache */
+ e_in_cache = 0;
+ } else {
+ backentry_free(&e);
+ }
slapi_pblock_set( pb, SLAPI_DELETE_EXISTING_ENTRY, original_entry->ep_entry );
e = original_entry;
if ( (original_entry = backentry_dup( e )) == NULL ) {
@@ -490,10 +502,11 @@ ldbm_back_delete( Slapi_PBlock *pb )
goto error_return;
}
/* We're re-trying */
- LDAPDebug( LDAP_DEBUG_TRACE, "Delete Retrying Transaction\n", 0, 0, 0 );
+ LDAPDebug0Args(LDAP_DEBUG_BACKLDBM,
+ "Delete Retrying Transaction\n");
#ifndef LDBM_NO_BACKOFF_DELAY
- {
- PRIntervalTime interval;
+ {
+ PRIntervalTime interval;
interval = PR_MillisecondsToInterval(slapi_rand() % 100);
DS_Sleep(interval);
}
@@ -1006,9 +1019,11 @@ ldbm_back_delete( Slapi_PBlock *pb )
}
/* delete from cache and clean up */
- CACHE_REMOVE(&inst->inst_cache, e);
- cache_unlock_entry( &inst->inst_cache, e );
- CACHE_RETURN( &inst->inst_cache, &e );
+ if (e_in_cache) {
+ CACHE_REMOVE(&inst->inst_cache, e);
+ cache_unlock_entry(&inst->inst_cache, e);
+ CACHE_RETURN(&inst->inst_cache, &e);
+ }
if (tombstone_in_cache) {
if (CACHE_ADD( &inst->inst_cache, tombstone, NULL ) == 0) {
tombstone_in_cache = 1;
@@ -1127,7 +1142,7 @@ common_return:
/* Need to return to cache after post op plugins are called */
if (retval) { /* error case */
- if (e) {
+ if (e && e_in_cache) {
cache_unlock_entry( &inst->inst_cache, e );
CACHE_RETURN( &inst->inst_cache, &e );
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index 7351166..bdc2967 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
@@ -151,7 +151,7 @@ int modify_update_all(backend *be, Slapi_PBlock *pb,
* Update the ID to Entry index.
* Note that id2entry_add replaces the entry, so the Entry ID stays the same.
*/
- retval = id2entry_add_ext( be, mc->new_entry, txn, mc->attr_encrypt );
+ retval = id2entry_add_ext( be, mc->new_entry, txn, mc->attr_encrypt, NULL );
if ( 0 != retval ) {
if (DB_LOCK_DEADLOCK != retval)
{
@@ -444,7 +444,7 @@ ldbm_back_modify( Slapi_PBlock *pb )
txn.back_txn_txn = NULL; /* ready to create the child transaction */
for (retry_count = 0; retry_count < RETRY_TIMES; retry_count++) {
-
+ int cache_rc = 0;
if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) {
dblayer_txn_abort(li,&txn);
slapi_pblock_set(pb, SLAPI_TXN, parent_txn);
@@ -456,15 +456,31 @@ ldbm_back_modify( Slapi_PBlock *pb )
slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods);
ldap_mods_free(mods, 1);
slapi_pblock_set(pb, SLAPI_MODIFY_MODS, copy_mods(mods_original));
- backentry_free(&ec);
+ if (ec_in_cache) {
+ /* New entry 'ec' is in the entry cache.
+ * Remove it from the cache once. */
+ CACHE_REMOVE(&inst->inst_cache, ec);
+ cache_unlock_entry(&inst->inst_cache, e);
+ CACHE_RETURN(&inst->inst_cache, ec);
+ } else {
+ backentry_free(&ec);
+ }
slapi_pblock_set( pb, SLAPI_MODIFY_EXISTING_ENTRY, original_entry->ep_entry );
ec = original_entry;
if ( (original_entry = backentry_dup( e )) == NULL ) {
ldap_result_code= LDAP_OPERATIONS_ERROR;
goto error_return;
}
-
- LDAPDebug( LDAP_DEBUG_TRACE, "Modify Retrying Transaction\n", 0, 0, 0 );
+ /* Put new entry 'ec' into the entry cache. */
+ if (ec_in_cache && CACHE_ADD(&inst->inst_cache, ec, NULL) < 0) {
+ LDAPDebug1Arg(LDAP_DEBUG_ANY,
+ "ldbm_back_modify: adding %s to cache failed\n",
+ slapi_entry_get_dn_const(ec->ep_entry));
+ ldap_result_code = LDAP_OPERATIONS_ERROR;
+ goto error_return;
+ }
+ LDAPDebug0Args(LDAP_DEBUG_BACKLDBM,
+ "Modify Retrying Transaction\n");
#ifndef LDBM_NO_BACKOFF_DELAY
{
PRIntervalTime interval;
@@ -503,9 +519,10 @@ ldbm_back_modify( Slapi_PBlock *pb )
/*
* Update the ID to Entry index.
- * Note that id2entry_add replaces the entry, so the Entry ID stays the same.
+ * Note that id2entry_add replaces the entry, so the Entry ID
+ * stays the same.
*/
- retval = id2entry_add( be, ec, &txn );
+ retval = id2entry_add_ext( be, ec, &txn, 1, &cache_rc );
if (DB_LOCK_DEADLOCK == retval)
{
/* Abort and re-try */
@@ -518,7 +535,14 @@ ldbm_back_modify( Slapi_PBlock *pb )
MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);
goto error_return;
}
- ec_in_cache = 1;
+ /*
+ * id2entry_add tries to put ec into the entry cache,
+ * but due to the conflict with original 'e',
+ * the cache_add (called vai id2entry_add) could fail.
+ */
+ if (0 == cache_rc) {
+ ec_in_cache = 1;
+ }
retval = index_add_mods( be, mods, e, ec, &txn );
if (DB_LOCK_DEADLOCK == retval)
{
@@ -596,6 +620,7 @@ ldbm_back_modify( Slapi_PBlock *pb )
MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);
goto error_return;
}
+ ec_in_cache = 1;
postentry = slapi_entry_dup( ec->ep_entry );
slapi_pblock_set( pb, SLAPI_ENTRY_POST_OP, postentry );
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index f9aef3a..08a8841 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -764,14 +764,30 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
slapi_sdn_free(&dn_newsuperiordn);
slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn);
orig_dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn);
-
- backentry_free(&ec);
+ if (ec_in_cache) {
+ /* New entry 'ec' is in the entry cache.
+ * Remove it from teh cache once. */
+ CACHE_REMOVE(&inst->inst_cache, ec);
+ cache_unlock_entry(&inst->inst_cache, e);
+ CACHE_RETURN(&inst->inst_cache, ec);
+ } else {
+ backentry_free(&ec);
+ }
slapi_pblock_set( pb, SLAPI_MODRDN_EXISTING_ENTRY, original_entry->ep_entry );
ec = original_entry;
if ( (original_entry = backentry_dup( ec )) == NULL ) {
ldap_result_code= LDAP_OPERATIONS_ERROR;
goto error_return;
}
+ if (ec_in_cache &&
+ /* Put the resetted entry 'ec' into the cache again. */
+ (cache_add_tentative( &inst->inst_cache, ec, NULL ) != 0)) {
+ LDAPDebug1Arg(LDAP_DEBUG_ANY,
+ "ldbm_back_modrdn: adding %s to cache failed\n",
+ slapi_entry_get_dn_const(ec->ep_entry));
+ ldap_result_code = LDAP_OPERATIONS_ERROR;
+ goto error_return;
+ }
slapi_pblock_get(pb, SLAPI_MODRDN_TARGET_ENTRY, &target_entry );
slapi_entry_free(target_entry);
@@ -801,7 +817,8 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
goto error_return;
}
/* We're re-trying */
- LDAPDebug( LDAP_DEBUG_TRACE, "Modrdn Retrying Transaction\n", 0, 0, 0 );
+ LDAPDebug0Args(LDAP_DEBUG_BACKLDBM,
+ "Modrdn Retrying Transaction\n");
}
retval = dblayer_txn_begin(li,parent_txn,&txn);
if (0 != retval) {
diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
index f93358b..03e0b11 100644
--- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
@@ -220,7 +220,7 @@ int has_children( struct ldbminfo *li, struct backentry *p, back_txn *txn, int *
* id2entry.c
*/
int id2entry_add( backend *be, struct backentry *e, back_txn *txn );
-int id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt );
+int id2entry_add_ext( backend *be, struct backentry *e, back_txn *txn, int encrypt, int *cache_res );
int id2entry_delete( backend *be, struct backentry *e, back_txn *txn );
struct backentry * id2entry( backend *be, ID id, back_txn *txn, int *err );
More information about the 389-commits
mailing list