andykimpe / rpms / 389-ds-base

Forked from rpms/389-ds-base 5 months ago
Clone
Blob Blame History Raw
From 669d0b288ca55a144fd1f5ba30199d2d2bb82061 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Thu, 7 Mar 2019 15:38:25 -0500
Subject: [PATCH] Ticket 50260 - backend txn plugins can corrupt entry cache

Bug Description:  If a nested backend txn plugin fails, any updates
                  it made that went into the entry cache still persist
                  after the database transaction is aborted.

Fix Description:  In order to be sure the entry cache is not corrupted
                  after a backend txn plugin failure we need to flush
                  all the cache entries that were added to the cache
                  after the parent operation was started.

                  To do this we record the start time the original operation,
                  (or parent operation), and we record the time any entry
                  is added to the cache.  Then on failure we do a comparision
                  and remove the entry from the cache if it's not in use.
                  If it is in use we add a "invalid" flag which triggers
                  the entry to be removed when the cache entry is returned
                  by the owner.

https://pagure.io/389-ds-base/issue/50260

CI tested and ASAN approved.

Reviewed by: firstyear, tbordaz, and lkrispen (Thanks!!!)

(cherry picked from commit 7ba8a80cfbaed9f6d727f98ed8c284943b3295e1)
---
 dirsrvtests/tests/suites/betxns/betxn_test.py | 114 ++++++++++++++++--
 ldap/servers/slapd/back-ldbm/back-ldbm.h      |  68 ++++++-----
 ldap/servers/slapd/back-ldbm/backentry.c      |   3 +-
 ldap/servers/slapd/back-ldbm/cache.c          | 112 ++++++++++++++++-
 ldap/servers/slapd/back-ldbm/ldbm_add.c       |  14 +++
 ldap/servers/slapd/back-ldbm/ldbm_delete.c    |  14 +++
 ldap/servers/slapd/back-ldbm/ldbm_modify.c    |  14 +++
 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c    |  32 +++--
 .../servers/slapd/back-ldbm/proto-back-ldbm.h |   1 +
 ldap/servers/slapd/slapi-plugin.h             |   6 +
 10 files changed, 321 insertions(+), 57 deletions(-)

diff --git a/dirsrvtests/tests/suites/betxns/betxn_test.py b/dirsrvtests/tests/suites/betxns/betxn_test.py
index 48181a9ea..f03fb93cc 100644
--- a/dirsrvtests/tests/suites/betxns/betxn_test.py
+++ b/dirsrvtests/tests/suites/betxns/betxn_test.py
@@ -7,12 +7,10 @@
 # --- END COPYRIGHT BLOCK ---
 #
 import pytest
-import six
 import ldap
 from lib389.tasks import *
 from lib389.utils import *
 from lib389.topologies import topology_st
-
 from lib389._constants import DEFAULT_SUFFIX, PLUGIN_7_BIT_CHECK, PLUGIN_ATTR_UNIQUENESS, PLUGIN_MEMBER_OF
 
 logging.getLogger(__name__).setLevel(logging.DEBUG)
@@ -249,8 +247,8 @@ def test_betxn_memberof(topology_st, dynamic_plugins):
     log.info('test_betxn_memberof: PASSED')
 
 
-def test_betxn_modrdn_memberof(topology_st):
-    """Test modrdn operartions and memberOf
+def test_betxn_modrdn_memberof_cache_corruption(topology_st):
+    """Test modrdn operations and memberOf
 
     :id: 70d0b96e-b693-4bf7-bbf5-102a66ac5994
 
@@ -285,18 +283,18 @@ def test_betxn_modrdn_memberof(topology_st):
 
     # Create user and add it to group
     users = UserAccounts(topology_st.standalone, basedn=DEFAULT_SUFFIX)
-    user = users.create(properties=TEST_USER_PROPERTIES)
+    user = users.ensure_state(properties=TEST_USER_PROPERTIES)
     if not ds_is_older('1.3.7'):
         user.remove('objectClass', 'nsMemberOf')
 
     group.add_member(user.dn)
 
     # Attempt modrdn that should fail, but the original entry should stay in the cache
-    with pytest.raises(ldap.OBJECTCLASS_VIOLATION):
+    with pytest.raises(ldap.OBJECT_CLASS_VIOLATION):
         group.rename('cn=group_to_people', newsuperior=peoplebase)
 
     # Should fail, but not with NO_SUCH_OBJECT as the original entry should still be in the cache
-    with pytest.raises(ldap.OBJECTCLASS_VIOLATION):
+    with pytest.raises(ldap.OBJECT_CLASS_VIOLATION):
         group.rename('cn=group_to_people', newsuperior=peoplebase)
 
     #
@@ -305,6 +303,108 @@ def test_betxn_modrdn_memberof(topology_st):
     log.info('test_betxn_modrdn_memberof: PASSED')
 
 
+def test_ri_and_mep_cache_corruption(topology_st):
+    """Test RI plugin aborts change after MEP plugin fails.
+    This is really testing the entry cache for corruption
+
+    :id: 70d0b96e-b693-4bf7-bbf5-102a66ac5995
+
+    :setup: Standalone instance
+
+    :steps: 1. Enable and configure mep and ri plugins
+            2. Add user and add it to a group
+            3. Disable MEP plugin and remove MEP group
+            4. Delete user
+            5. Check that user is still a member of the group
+
+    :expectedresults:
+            1. Success
+            2. Success
+            3. Success
+            4. It fails with NO_SUCH_OBJECT
+            5. Success
+
+    """
+    # Start plugins
+    topology_st.standalone.config.set('nsslapd-dynamic-plugins', 'on')
+    mep_plugin = ManagedEntriesPlugin(topology_st.standalone)
+    mep_plugin.enable()
+    ri_plugin = ReferentialIntegrityPlugin(topology_st.standalone)
+    ri_plugin.enable()
+
+    # Add our org units
+    ous = OrganizationalUnits(topology_st.standalone, DEFAULT_SUFFIX)
+    ou_people = ous.create(properties={'ou': 'managed_people'})
+    ou_groups = ous.create(properties={'ou': 'managed_groups'})
+
+    # Configure MEP
+    mep_templates = MEPTemplates(topology_st.standalone, DEFAULT_SUFFIX)
+    mep_template1 = mep_templates.create(properties={
+        'cn': 'MEP template',
+        'mepRDNAttr': 'cn',
+        'mepStaticAttr': 'objectclass: posixGroup|objectclass: extensibleObject'.split('|'),
+        'mepMappedAttr': 'cn: $cn|uid: $cn|gidNumber: $uidNumber'.split('|')
+    })
+    mep_configs = MEPConfigs(topology_st.standalone)
+    mep_configs.create(properties={'cn': 'config',
+                                                'originScope': ou_people.dn,
+                                                'originFilter': 'objectclass=posixAccount',
+                                                'managedBase': ou_groups.dn,
+                                                'managedTemplate': mep_template1.dn})
+
+    # Add an entry that meets the MEP scope
+    users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX,
+                         rdn='ou={}'.format(ou_people.rdn))
+    user = users.create(properties={
+        'uid': 'test-user1',
+        'cn': 'test-user',
+        'sn': 'test-user',
+        'uidNumber': '10011',
+        'gidNumber': '20011',
+        'homeDirectory': '/home/test-user1'
+    })
+
+    # Add group
+    groups = Groups(topology_st.standalone, DEFAULT_SUFFIX)
+    user_group = groups.ensure_state(properties={'cn': 'group', 'member': user.dn})
+
+    # Check if a managed group entry was created
+    mep_group = Group(topology_st.standalone, dn='cn={},{}'.format(user.rdn, ou_groups.dn))
+    if not mep_group.exists():
+        log.fatal("MEP group was not created for the user")
+        assert False
+
+    # Mess with MEP so it fails
+    mep_plugin.disable()
+    mep_group.delete()
+    mep_plugin.enable()
+
+    # Add another group for verify entry cache is not corrupted
+    test_group = groups.create(properties={'cn': 'test_group'})
+
+    # Delete user, should fail, and user should still be a member
+    with pytest.raises(ldap.NO_SUCH_OBJECT):
+        user.delete()
+
+    # Verify membership is intact
+    if not user_group.is_member(user.dn):
+        log.fatal("Member was incorrectly removed from the group!!  Or so it seems")
+
+        # Restart server and test again in case this was a cache issue
+        topology_st.standalone.restart()
+        if user_group.is_member(user.dn):
+            log.info("The entry cache was corrupted")
+            assert False
+
+        assert False
+
+    # Verify test group is still found in entry cache by deleting it
+    test_group.delete()
+
+    # Success
+    log.info("Test PASSED")
+
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode
diff --git a/ldap/servers/slapd/back-ldbm/back-ldbm.h b/ldap/servers/slapd/back-ldbm/back-ldbm.h
index 4727961a9..6cac605c0 100644
--- a/ldap/servers/slapd/back-ldbm/back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/back-ldbm.h
@@ -312,48 +312,52 @@ typedef struct
 
 struct backcommon
 {
-    int ep_type;                   /* to distinguish backdn from backentry */
-    struct backcommon *ep_lrunext; /* for the cache */
-    struct backcommon *ep_lruprev; /* for the cache */
-    ID ep_id;                      /* entry id */
-    char ep_state;                 /* state in the cache */
-#define ENTRY_STATE_DELETED    0x1 /* entry is marked as deleted */
-#define ENTRY_STATE_CREATING   0x2 /* entry is being created; don't touch it */
-#define ENTRY_STATE_NOTINCACHE 0x4 /* cache_add failed; not in the cache */
-    int ep_refcnt;                 /* entry reference cnt */
-    size_t ep_size;                /* for cache tracking */
+    int32_t ep_type;                /* to distinguish backdn from backentry */
+    struct backcommon *ep_lrunext;  /* for the cache */
+    struct backcommon *ep_lruprev;  /* for the cache */
+    ID ep_id;                       /* entry id */
+    uint8_t ep_state;               /* state in the cache */
+#define ENTRY_STATE_DELETED    0x1  /* entry is marked as deleted */
+#define ENTRY_STATE_CREATING   0x2  /* entry is being created; don't touch it */
+#define ENTRY_STATE_NOTINCACHE 0x4  /* cache_add failed; not in the cache */
+#define ENTRY_STATE_INVALID    0x8  /* cache entry is invalid and needs to be removed */
+    int32_t ep_refcnt;              /* entry reference cnt */
+    size_t ep_size;                 /* for cache tracking */
+    struct timespec ep_create_time; /* the time the entry was added to the cache */
 };
 
-/* From ep_type through ep_size MUST be identical to backcommon */
+/* From ep_type through ep_create_time MUST be identical to backcommon */
 struct backentry
 {
-    int ep_type;                   /* to distinguish backdn from backentry */
-    struct backcommon *ep_lrunext; /* for the cache */
-    struct backcommon *ep_lruprev; /* for the cache */
-    ID ep_id;                      /* entry id */
-    char ep_state;                 /* state in the cache */
-    int ep_refcnt;                 /* entry reference cnt */
-    size_t ep_size;                /* for cache tracking */
-    Slapi_Entry *ep_entry;         /* real entry */
+    int32_t ep_type;                /* to distinguish backdn from backentry */
+    struct backcommon *ep_lrunext;  /* for the cache */
+    struct backcommon *ep_lruprev;  /* for the cache */
+    ID ep_id;                       /* entry id */
+    uint8_t ep_state;               /* state in the cache */
+    int32_t ep_refcnt;              /* entry reference cnt */
+    size_t ep_size;                 /* for cache tracking */
+    struct timespec ep_create_time; /* the time the entry was added to the cache */
+    Slapi_Entry *ep_entry;          /* real entry */
     Slapi_Entry *ep_vlventry;
-    void *ep_dn_link;     /* linkage for the 3 hash */
-    void *ep_id_link;     /*     tables used for */
-    void *ep_uuid_link;   /*     looking up entries */
-    PRMonitor *ep_mutexp; /* protection for mods; make it reentrant */
+    void *ep_dn_link;               /* linkage for the 3 hash */
+    void *ep_id_link;               /*     tables used for */
+    void *ep_uuid_link;             /*     looking up entries */
+    PRMonitor *ep_mutexp;           /* protection for mods; make it reentrant */
 };
 
-/* From ep_type through ep_size MUST be identical to backcommon */
+/* From ep_type through ep_create_time MUST be identical to backcommon */
 struct backdn
 {
-    int ep_type;                   /* to distinguish backdn from backentry */
-    struct backcommon *ep_lrunext; /* for the cache */
-    struct backcommon *ep_lruprev; /* for the cache */
-    ID ep_id;                      /* entry id */
-    char ep_state;                 /* state in the cache; share ENTRY_STATE_* */
-    int ep_refcnt;                 /* entry reference cnt */
-    size_t ep_size;                /* for cache tracking */
+    int32_t ep_type;                /* to distinguish backdn from backentry */
+    struct backcommon *ep_lrunext;  /* for the cache */
+    struct backcommon *ep_lruprev;  /* for the cache */
+    ID ep_id;                       /* entry id */
+    uint8_t ep_state;               /* state in the cache; share ENTRY_STATE_* */
+    int32_t ep_refcnt;              /* entry reference cnt */
+    size_t ep_size;               /* for cache tracking */
+    struct timespec ep_create_time; /* the time the entry was added to the cache */
     Slapi_DN *dn_sdn;
-    void *dn_id_link; /* for hash table */
+    void *dn_id_link;               /* for hash table */
 };
 
 /* for the in-core cache of entries */
diff --git a/ldap/servers/slapd/back-ldbm/backentry.c b/ldap/servers/slapd/back-ldbm/backentry.c
index f2fe780db..972842bcb 100644
--- a/ldap/servers/slapd/back-ldbm/backentry.c
+++ b/ldap/servers/slapd/back-ldbm/backentry.c
@@ -23,7 +23,8 @@ backentry_free(struct backentry **bep)
         return;
     }
     ep = *bep;
-    PR_ASSERT(ep->ep_state & (ENTRY_STATE_DELETED | ENTRY_STATE_NOTINCACHE));
+
+    PR_ASSERT(ep->ep_state & (ENTRY_STATE_DELETED | ENTRY_STATE_NOTINCACHE | ENTRY_STATE_INVALID));
     if (ep->ep_entry != NULL) {
         slapi_entry_free(ep->ep_entry);
     }
diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c
index 86e1f7b39..458d7912f 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -56,11 +56,14 @@
 #define LOG(...)
 #endif
 
-#define LRU_DETACH(cache, e) lru_detach((cache), (void *)(e))
+typedef enum {
+    ENTRY_CACHE,
+    DN_CACHE,
+} CacheType;
 
+#define LRU_DETACH(cache, e) lru_detach((cache), (void *)(e))
 #define CACHE_LRU_HEAD(cache, type) ((type)((cache)->c_lruhead))
 #define CACHE_LRU_TAIL(cache, type) ((type)((cache)->c_lrutail))
-
 #define BACK_LRU_NEXT(entry, type) ((type)((entry)->ep_lrunext))
 #define BACK_LRU_PREV(entry, type) ((type)((entry)->ep_lruprev))
 
@@ -185,6 +188,7 @@ new_hash(u_long size, u_long offset, HashFn hfn, HashTestFn tfn)
 int
 add_hash(Hashtable *ht, void *key, uint32_t keylen, void *entry, void **alt)
 {
+    struct backcommon *back_entry = (struct backcommon *)entry;
     u_long val, slot;
     void *e;
 
@@ -202,6 +206,7 @@ add_hash(Hashtable *ht, void *key, uint32_t keylen, void *entry, void **alt)
         e = HASH_NEXT(ht, e);
     }
     /* ok, it's not already there, so add it */
+    back_entry->ep_create_time = slapi_current_rel_time_hr();
     HASH_NEXT(ht, entry) = ht->slot[slot];
     ht->slot[slot] = entry;
     return 1;
@@ -492,6 +497,89 @@ cache_make_hashes(struct cache *cache, int type)
     }
 }
 
+/*
+ * Helper function for flush_hash() to calculate if the entry should be
+ * removed from the cache.
+ */
+static int32_t
+flush_remove_entry(struct timespec *entry_time, struct timespec *start_time)
+{
+    struct timespec diff;
+
+    slapi_timespec_diff(entry_time, start_time, &diff);
+    if (diff.tv_sec >= 0) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
+/*
+ * Flush all the cache entries that were added after the "start time"
+ * This is called when a backend transaction plugin fails, and we need
+ * to remove all the possible invalid entries in the cache.
+ *
+ * If the ref count is 0, we can straight up remove it from the cache, but
+ * if the ref count is greater than 1, then the entry is currently in use.
+ * In the later case we set the entry state to ENTRY_STATE_INVALID, and
+ * when the owning thread cache_returns() the cache entry is automatically
+ * removed so another thread can not use/lock the invalid cache entry.
+ */
+static void
+flush_hash(struct cache *cache, struct timespec *start_time, int32_t type)
+{
+    void *e, *laste = NULL;
+    Hashtable *ht = cache->c_idtable;
+
+    cache_lock(cache);
+
+    for (size_t i = 0; i < ht->size; i++) {
+        e = ht->slot[i];
+        while (e) {
+            struct backcommon *entry = (struct backcommon *)e;
+            uint64_t remove_it = 0;
+            if (flush_remove_entry(&entry->ep_create_time, start_time)) {
+                /* Mark the entry to be removed */
+                slapi_log_err(SLAPI_LOG_CACHE, "flush_hash", "[%s] Removing entry id (%d)\n",
+                        type ? "DN CACHE" : "ENTRY CACHE", entry->ep_id);
+                remove_it = 1;
+            }
+            laste = e;
+            e = HASH_NEXT(ht, e);
+
+            if (remove_it) {
+                /* since we have the cache lock we know we can trust refcnt */
+                entry->ep_state |= ENTRY_STATE_INVALID;
+                if (entry->ep_refcnt == 0) {
+                    entry->ep_refcnt++;
+                    lru_delete(cache, laste);
+                    if (type == ENTRY_CACHE) {
+                        entrycache_remove_int(cache, laste);
+                        entrycache_return(cache, (struct backentry **)&laste);
+                    } else {
+                        dncache_remove_int(cache, laste);
+                        dncache_return(cache, (struct backdn **)&laste);
+                    }
+                } else {
+                    /* Entry flagged for removal */
+                    slapi_log_err(SLAPI_LOG_CACHE, "flush_hash",
+                            "[%s] Flagging entry to be removed later: id (%d) refcnt: %d\n",
+                            type ? "DN CACHE" : "ENTRY CACHE", entry->ep_id, entry->ep_refcnt);
+                }
+            }
+        }
+    }
+
+    cache_unlock(cache);
+}
+
+void
+revert_cache(ldbm_instance *inst, struct timespec *start_time)
+{
+    flush_hash(&inst->inst_cache, start_time, ENTRY_CACHE);
+    flush_hash(&inst->inst_dncache, start_time, DN_CACHE);
+}
+
 /* initialize the cache */
 int
 cache_init(struct cache *cache, uint64_t maxsize, long maxentries, int type)
@@ -1142,7 +1230,7 @@ entrycache_return(struct cache *cache, struct backentry **bep)
     } else {
         ASSERT(e->ep_refcnt > 0);
         if (!--e->ep_refcnt) {
-            if (e->ep_state & ENTRY_STATE_DELETED) {
+            if (e->ep_state & (ENTRY_STATE_DELETED | ENTRY_STATE_INVALID)) {
                 const char *ndn = slapi_sdn_get_ndn(backentry_get_sdn(e));
                 if (ndn) {
                     /*
@@ -1154,6 +1242,13 @@ entrycache_return(struct cache *cache, struct backentry **bep)
                         LOG("entrycache_return -Failed to remove %s from dn table\n", ndn);
                     }
                 }
+                if (e->ep_state & ENTRY_STATE_INVALID) {
+                    /* Remove it from the hash table before we free the back entry */
+                    slapi_log_err(SLAPI_LOG_CACHE, "entrycache_return",
+                            "Finally flushing invalid entry: %d (%s)\n",
+                            e->ep_id, backentry_get_ndn(e));
+                    entrycache_remove_int(cache, e);
+                }
                 backentry_free(bep);
             } else {
                 lru_add(cache, e);
@@ -1535,7 +1630,7 @@ cache_lock_entry(struct cache *cache, struct backentry *e)
 
     /* make sure entry hasn't been deleted now */
     cache_lock(cache);
-    if (e->ep_state & (ENTRY_STATE_DELETED | ENTRY_STATE_NOTINCACHE)) {
+    if (e->ep_state & (ENTRY_STATE_DELETED | ENTRY_STATE_NOTINCACHE | ENTRY_STATE_INVALID)) {
         cache_unlock(cache);
         PR_ExitMonitor(e->ep_mutexp);
         LOG("<= cache_lock_entry (DELETED)\n");
@@ -1696,7 +1791,14 @@ dncache_return(struct cache *cache, struct backdn **bdn)
     } else {
         ASSERT((*bdn)->ep_refcnt > 0);
         if (!--(*bdn)->ep_refcnt) {
-            if ((*bdn)->ep_state & ENTRY_STATE_DELETED) {
+            if ((*bdn)->ep_state & (ENTRY_STATE_DELETED | ENTRY_STATE_INVALID)) {
+                if ((*bdn)->ep_state & ENTRY_STATE_INVALID) {
+                    /* Remove it from the hash table before we free the back dn */
+                    slapi_log_err(SLAPI_LOG_CACHE, "dncache_return",
+                            "Finally flushing invalid entry: %d (%s)\n",
+                            (*bdn)->ep_id, slapi_sdn_get_dn((*bdn)->dn_sdn));
+                    dncache_remove_int(cache, (*bdn));
+                }
                 backdn_free(bdn);
             } else {
                 lru_add(cache, (void *)*bdn);
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c
index 32c8e71ff..aa5b59aea 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
@@ -97,6 +97,8 @@ ldbm_back_add(Slapi_PBlock *pb)
     PRUint64 conn_id;
     int op_id;
     int result_sent = 0;
+    int32_t parent_op = 0;
+    struct timespec parent_time;
 
     if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) {
         conn_id = 0; /* connection is NULL */
@@ -147,6 +149,13 @@ ldbm_back_add(Slapi_PBlock *pb)
     slapi_entry_delete_values(e, numsubordinates, NULL);
 
     dblayer_txn_init(li, &txn);
+
+    if (txn.back_txn_txn == NULL) {
+        /* This is the parent operation, get the time */
+        parent_op = 1;
+        parent_time = slapi_current_rel_time_hr();
+    }
+
     /* the calls to perform searches require the parent txn if any
        so set txn to the parent_txn until we begin the child transaction */
     if (parent_txn) {
@@ -1212,6 +1221,11 @@ ldbm_back_add(Slapi_PBlock *pb)
             slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
         }
         slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
+
+        /* Revert the caches if this is the parent operation */
+        if (parent_op) {
+            revert_cache(inst, &parent_time);
+        }
         goto error_return;
     }
 
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index f5f6c1e3a..3f687eb91 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -79,6 +79,8 @@ ldbm_back_delete(Slapi_PBlock *pb)
     ID tomb_ep_id = 0;
     int result_sent = 0;
     Connection *pb_conn;
+    int32_t parent_op = 0;
+    struct timespec parent_time;
 
     if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) {
         conn_id = 0; /* connection is NULL */
@@ -100,6 +102,13 @@ ldbm_back_delete(Slapi_PBlock *pb)
     dblayer_txn_init(li, &txn);
     /* the calls to perform searches require the parent txn if any
        so set txn to the parent_txn until we begin the child transaction */
+
+    if (txn.back_txn_txn == NULL) {
+        /* This is the parent operation, get the time */
+        parent_op = 1;
+        parent_time = slapi_current_rel_time_hr();
+    }
+
     if (parent_txn) {
         txn.back_txn_txn = parent_txn;
     } else {
@@ -1270,6 +1279,11 @@ replace_entry:
             slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &retval);
         }
         slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
+
+        /* Revert the caches if this is the parent operation */
+        if (parent_op) {
+            revert_cache(inst, &parent_time);
+        }
         goto error_return;
     }
     if (parent_found) {
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index cc4319e5f..b90b3e0f0 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
@@ -412,6 +412,8 @@ ldbm_back_modify(Slapi_PBlock *pb)
     int fixup_tombstone = 0;
     int ec_locked = 0;
     int result_sent = 0;
+    int32_t parent_op = 0;
+    struct timespec parent_time;
 
     slapi_pblock_get(pb, SLAPI_BACKEND, &be);
     slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li);
@@ -426,6 +428,13 @@ ldbm_back_modify(Slapi_PBlock *pb)
     dblayer_txn_init(li, &txn); /* must do this before first goto error_return */
     /* the calls to perform searches require the parent txn if any
        so set txn to the parent_txn until we begin the child transaction */
+
+    if (txn.back_txn_txn == NULL) {
+        /* This is the parent operation, get the time */
+        parent_op = 1;
+        parent_time = slapi_current_rel_time_hr();
+    }
+
     if (parent_txn) {
         txn.back_txn_txn = parent_txn;
     } else {
@@ -864,6 +873,11 @@ ldbm_back_modify(Slapi_PBlock *pb)
             slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
         }
         slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
+
+        /* Revert the caches if this is the parent operation */
+        if (parent_op) {
+            revert_cache(inst, &parent_time);
+        }
         goto error_return;
     }
     retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_MODIFY_FN);
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index e4d0337d4..73e50ebcc 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -97,6 +97,8 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
     int op_id;
     int result_sent = 0;
     Connection *pb_conn = NULL;
+    int32_t parent_op = 0;
+    struct timespec parent_time;
 
     if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) {
         conn_id = 0; /* connection is NULL */
@@ -134,6 +136,13 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
 
     /* dblayer_txn_init needs to be called before "goto error_return" */
     dblayer_txn_init(li, &txn);
+
+    if (txn.back_txn_txn == NULL) {
+        /* This is the parent operation, get the time */
+        parent_op = 1;
+        parent_time = slapi_current_rel_time_hr();
+    }
+
     /* the calls to perform searches require the parent txn if any
        so set txn to the parent_txn until we begin the child transaction */
     if (parent_txn) {
@@ -1208,6 +1217,11 @@ ldbm_back_modrdn(Slapi_PBlock *pb)
             slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
         }
         slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
+
+        /* Revert the caches if this is the parent operation */
+        if (parent_op) {
+            revert_cache(inst, &parent_time);
+        }
         goto error_return;
     }
 	retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN);
@@ -1353,8 +1367,13 @@ error_return:
                     slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval);
                 }
                 slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
+
+                /* Revert the caches if this is the parent operation */
+                if (parent_op) {
+                    revert_cache(inst, &parent_time);
+                }
             }
-	retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN);
+            retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN);
 
             /* Release SERIAL LOCK */
             dblayer_txn_abort(be, &txn); /* abort crashes in case disk full */
@@ -1411,17 +1430,6 @@ common_return:
                                       "operation failed, the target entry is cleared from dncache (%s)\n", slapi_entry_get_dn(ec->ep_entry));
             CACHE_REMOVE(&inst->inst_dncache, bdn);
             CACHE_RETURN(&inst->inst_dncache, &bdn);
-            /*
-             * If the new/invalid entry (ec) is in the cache, that means we need to
-             * swap it out with the original entry (e) --> to undo the swap that
-             * modrdn_rename_entry_update_indexes() did.
-             */
-            if (cache_is_in_cache(&inst->inst_cache, ec)) {
-                if (cache_replace(&inst->inst_cache, ec, e) != 0) {
-                        slapi_log_err(SLAPI_LOG_ALERT, "ldbm_back_modrdn",
-                                "failed to replace cache entry after error\n");
-                 }
-            }
         }
 
         if (ec && inst) {
diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
index b56f6ef26..e68765bd4 100644
--- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
@@ -55,6 +55,7 @@ void cache_unlock_entry(struct cache *cache, struct backentry *e);
 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);
 
 #ifdef CACHE_DEBUG
 void check_entry_cache(struct cache *cache, struct backentry *e);
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 54c195eef..0bc3a6fab 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6765,6 +6765,12 @@ time_t slapi_current_time(void) __attribute__((deprecated));
  * \return timespec of the current relative system time.
  */
 struct timespec slapi_current_time_hr(void);
+/**
+ * Returns the current system time as a hr clock
+ *
+ * \return timespec of the current monotonic time.
+ */
+struct timespec slapi_current_rel_time_hr(void);
 /**
  * Returns the current system time as a hr clock in UTC timezone.
  * This clock adjusts with ntp steps, and should NOT be
-- 
2.17.2