andykimpe / rpms / 389-ds-base

Forked from rpms/389-ds-base 5 months ago
Clone

Blame SOURCES/0006-Ticket-50260-backend-txn-plugins-can-corrupt-entry-c.patch

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