Issue 5939 - During an update, if the target entry is reverted in the…

… entry cache, the server should not retry to lock it

Bug description:
	During an update if an BETXN plugin fails the full TXN is aborted and the DB
	returns to the previous state.
	However potential internal updates, done by BETXN plugins, are also applied
	on the entry cache.
	Even if the TXN is aborted some entries in the entry cache are left in a state
	that does not reflect the DB state.
	The fix https://pagure.io/389-ds-base/issue/50260 "reverts" those
	entries, setting their state to INVALID.

	A problem is that reverted entries stay in the entry cache, until refcnt is 0.
	During that period, an update targeting that entry fails to retrieve the
	entry from the entry cache and fails to add it again as it already exist
	the entry.
	The update iterates 1000 times, trying to read the entry and to fetch it
	from DB.
	This is a pure waste as the reverted entry stays too long.

	The signature of this issue is a message in the error log: "Retry count exceeded"

Fix description:
	The fix consiste in the loops (fetch on DN or NSUNIQUEID) to test if the
        entry state is INVALID.
	In such case it aborts the loop and return a failure.

relates: #5939

Reviewed by:

ldap/servers/slapd/back-ldbm/cache.c

@@ -1744,6 +1744,22 @@ cache_lock_entry(struct cache *cache, struct backentry *e)
     return 0;
 }
 
+int
+cache_is_reverted_entry(struct cache *cache, struct backentry *e)
+{
+    struct backentry *dummy_e;
+
+    cache_lock(cache);
+    if (find_hash(cache->c_idtable, &e->ep_id, sizeof(ID), (void **)&dummy_e)) {
+        if (dummy_e->ep_state & ENTRY_STATE_INVALID) {
+            slapi_log_err(SLAPI_LOG_WARNING, "cache_is_reverted_entry", "Entry reverted = %d (0x%X)  [entry: 0x%X] refcnt=%d\n", dummy_e->ep_state, pthread_self(), dummy_e, dummy_e->ep_refcnt);
+            cache_unlock(cache);
+            return 1;
+        }
+    }
+    cache_unlock(cache);
+    return 0;
+}
 /* the opposite of above */
 void
 cache_unlock_entry(struct cache *cache __attribute__((unused)), struct backentry *e)

ldap/servers/slapd/back-ldbm/findentry.c

@@ -98,6 +98,7 @@ find_entry_internal_dn(
     size_t tries = 0;
     int isroot = 0;
     int op_type;
+    int reverted_entry = 0;
 
     /* get the managedsait ldap message control */
     slapi_pblock_get(pb, SLAPI_MANAGEDSAIT, &managedsait);
@@ -141,12 +142,20 @@ find_entry_internal_dn(
          */
         slapi_log_err(SLAPI_LOG_ARGS, "find_entry_internal_dn",
                       "   Retrying (%s)\n", slapi_sdn_get_dn(sdn));
+        if (cache_is_reverted_entry(&inst->inst_cache, e)) {
+            reverted_entry = 1;
+            break;
+        }
         CACHE_RETURN(&inst->inst_cache, &e);
         tries++;
     }
     if (tries >= LDBM_CACHE_RETRY_COUNT) {
         slapi_log_err(SLAPI_LOG_ERR, "find_entry_internal_dn", "Retry count exceeded (%s)\n", slapi_sdn_get_dn(sdn));
     }
+    if (reverted_entry) {
+        slapi_send_ldap_result(pb, LDAP_BUSY, NULL, NULL, 0, NULL);
+        return (NULL);
+    }
     /*
      * there is no such entry in this server. see how far we
      * can match, and check if that entry contains a referral.
@@ -262,6 +271,7 @@ find_entry_internal_uniqueid(
     struct backentry *e;
     int err;
     size_t tries = 0;
+    int reverted_entry = 0;
 
     while ((tries < LDBM_CACHE_RETRY_COUNT) &&
            (e = uniqueid2entry(be, uniqueid, txn, &err)) != NULL) {
@@ -283,6 +293,10 @@ find_entry_internal_uniqueid(
          */
         slapi_log_err(SLAPI_LOG_ARGS,
                       "   find_entry_internal_uniqueid", "Retrying; uniqueid = (%s)\n", uniqueid);
+        if (cache_is_reverted_entry(&inst->inst_cache, e)) {
+            reverted_entry = 1;
+            break;
+        }
         CACHE_RETURN(&inst->inst_cache, &e);
         tries++;
     }
@@ -292,9 +306,14 @@ find_entry_internal_uniqueid(
                       uniqueid);
     }
 
-    /* entry not found */
-    slapi_send_ldap_result(pb, (0 == err || DBI_RC_NOTFOUND == err) ? LDAP_NO_SUCH_OBJECT : LDAP_OPERATIONS_ERROR, NULL /* matched */, NULL,
-                           0, NULL);
+    if (reverted_entry) {
+        slapi_send_ldap_result(pb, LDAP_BUSY, NULL, NULL, 0, NULL);
+        return (NULL);
+    } else {
+        /* entry not found */
+        slapi_send_ldap_result(pb, (0 == err || DBI_RC_NOTFOUND == err) ? LDAP_NO_SUCH_OBJECT : LDAP_OPERATIONS_ERROR, NULL /* matched */, NULL,
+                               0, NULL);
+    }
     slapi_log_err(SLAPI_LOG_TRACE,
                   "find_entry_internal_uniqueid", "<= not found; uniqueid = (%s)\n",
                   uniqueid);

ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

@@ -61,6 +61,7 @@ int cache_replace(struct cache *cache, void *oldptr, void *newptr);
 int cache_has_otherref(struct cache *cache, void *bep);
 int cache_is_in_cache(struct cache *cache, void *ptr);
 void revert_cache(ldbm_instance *inst, struct timespec *start_time);
+int cache_is_reverted_entry(struct cache *cache, struct backentry *e);
 
 #ifdef CACHE_DEBUG
 void check_entry_cache(struct cache *cache, struct backentry *e);