diff --git a/dirsrvtests/tests/suites/betxns/betxn_test.py b/dirsrvtests/tests/suites/betxns/betxn_test.py index b5ca01047f..d40fbafd73 100644 --- a/dirsrvtests/tests/suites/betxns/betxn_test.py +++ b/dirsrvtests/tests/suites/betxns/betxn_test.py @@ -8,6 +8,7 @@ # import pytest import ldap +import os from lib389.tasks import * from lib389.utils import * from lib389.topologies import topology_st @@ -15,6 +16,7 @@ MemberOfPlugin, ManagedEntriesPlugin, ReferentialIntegrityPlugin, MEPTemplates, MEPConfigs) +from lib389.plugins import MemberOfPlugin from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES from lib389.idm.organizationalunit import OrganizationalUnits from lib389.idm.group import Groups, Group @@ -356,6 +358,92 @@ def test_ri_and_mep_cache_corruption(topology_st): # Success log.info("Test PASSED") +def test_revert_cache(topology_st, request): + """Tests that reversion of the entry cache does not occur + during 'normal' failure (like schema violation) but only + when a plugin fails + + :id: a2361285-b939-4da0-aa80-7fc54d12c981 + + :setup: Standalone instance + + :steps: + 1. Create a user account (with a homeDirectory) + 2. Remove the error log file + 3. Add a second value of homeDirectory + 4. Check that error log does not contain entry cache reversion + 5. Configure memberof to trigger a failure + 6. Do a update the will fail in memberof plugin + 7. Check that error log does contain entry cache reversion + + :expectedresults: + 1. Succeeds + 2. Fails with OBJECT_CLASS_VIOLATION + 3. Succeeds + 4. Succeeds + 5. Succeeds + 6. Fails with OBJECT_CLASS_VIOLATION + 7. Succeeds + """ + # Create an test user entry + log.info('Create a user without nsMemberOF objectclass') + try: + users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX, rdn='ou=people') + user = users.create_test_user() + user.replace('objectclass', ['top', 'account', 'person', 'inetorgperson', 'posixAccount', 'organizationalPerson', 'nsAccount']) + except ldap.LDAPError as e: + log.fatal('Failed to create test user: error ' + e.args[0]['desc']) + assert False + + # Remove the current error log file + topology_st.standalone.stop() + lpath = topology_st.standalone.ds_error_log._get_log_path() + os.unlink(lpath) + topology_st.standalone.start() + + # + # Add a second value to 'homeDirectory' + # leads to ldap.OBJECT_CLASS_VIOLATION + # And check that the entry cache was not reverted + try: + topology_st.standalone.modify_s(user.dn, [(ldap.MOD_ADD, 'homeDirectory', ensure_bytes('/home/user_new'))]) + assert False + except ldap.OBJECT_CLASS_VIOLATION: + pass + assert not topology_st.standalone.ds_error_log.match('.*WARN - flush_hash - Upon BETXN callback failure, entry cache is flushed during.*') + + # Prepare memberof so that it will fail during a next update + # If memberof plugin can not add 'memberof' to the + # member entry, it retries after adding + # 'memberOfAutoAddOC' objectclass to the member. + # If it fails again the plugin fails with 'object + # violation' + # To trigger this failure, set 'memberOfAutoAddOC' + # to a value that does *not* allow 'memberof' attribute + memberof = MemberOfPlugin(topology_st.standalone) + memberof.enable() + memberof.replace('memberOfAutoAddOC', 'account') + memberof.replace('memberofentryscope', DEFAULT_SUFFIX) + topology_st.standalone.restart() + + # Try to add the user to demo_group + # It should fail because user entry has not 'nsmemberof' objectclass + # As this is a BETXN plugin that fails it should revert the entry cache + try: + GROUP_DN = "cn=demo_group,ou=groups," + DEFAULT_SUFFIX + topology_st.standalone.modify_s(GROUP_DN, + [(ldap.MOD_REPLACE, 'member', ensure_bytes(user.dn))]) + assert False + except ldap.OBJECT_CLASS_VIOLATION: + pass + assert topology_st.standalone.ds_error_log.match('.*WARN - flush_hash - Upon BETXN callback failure, entry cache is flushed during.*') + + def fin(): + user.delete() + memberof = MemberOfPlugin(topology_st.standalone) + memberof.replace('memberOfAutoAddOC', 'nsmemberof') + + request.addfinalizer(fin) if __name__ == '__main__': # Run isolated diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c index 6d6b6864be..604c0d77d9 100644 --- a/ldap/servers/slapd/back-ldbm/cache.c +++ b/ldap/servers/slapd/back-ldbm/cache.c @@ -556,7 +556,12 @@ flush_hash(struct cache *cache, struct timespec *start_time, int32_t type) { Hashtable *ht = cache->c_idtable; /* start with the ID table as it's in both ENTRY and DN caches */ void *e, *laste = NULL; + char flush_etime[ETIME_BUFSIZ] = {0}; + struct timespec duration; + struct timespec flush_start; + struct timespec flush_end; + clock_gettime(CLOCK_MONOTONIC, &flush_start); cache_lock(cache); for (size_t i = 0; i < ht->size; i++) { @@ -638,6 +643,11 @@ flush_hash(struct cache *cache, struct timespec *start_time, int32_t type) } cache_unlock(cache); + + clock_gettime(CLOCK_MONOTONIC, &flush_end); + slapi_timespec_diff(&flush_end, &flush_start, &duration); + snprintf(flush_etime, ETIME_BUFSIZ, "%" PRId64 ".%.09" PRId64 "", (int64_t)duration.tv_sec, (int64_t)duration.tv_nsec); + slapi_log_err(SLAPI_LOG_WARNING, "flush_hash", "Upon BETXN callback failure, entry cache is flushed during %s\n", flush_etime); } void diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index 27e9be748c..ed0c126f83 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -98,6 +98,7 @@ ldbm_back_add(Slapi_PBlock *pb) int op_id; int result_sent = 0; int32_t parent_op = 0; + int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ struct timespec parent_time; if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) { @@ -919,6 +920,9 @@ ldbm_back_add(Slapi_PBlock *pb) slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); slapi_log_err(SLAPI_LOG_DEBUG, "ldbm_back_add", "SLAPI_PLUGIN_BE_TXN_PRE_ADD_FN plugin failed: %d\n", ldap_result_code ? ldap_result_code : retval); + if (retval) { + betxn_callback_fails = 1; + } goto error_return; } } @@ -1229,11 +1233,15 @@ 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); + if (retval) { + betxn_callback_fails = 1; + } goto error_return; } retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_ADD_FN); if (retval) { + betxn_callback_fails = 1; ldbm_set_error(pb, retval, &ldap_result_code, &ldap_result_message); goto error_return; } @@ -1326,9 +1334,15 @@ ldbm_back_add(Slapi_PBlock *pb) slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &opreturn); } slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); + if (retval) { + betxn_callback_fails = 1; + } } /* the repl postop needs to be called for aborted operations */ retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_ADD_FN); + if (retval) { + betxn_callback_fails = 1; + } if (addingentry) { if (inst && cache_is_in_cache(&inst->inst_cache, addingentry)) { CACHE_REMOVE(&inst->inst_cache, addingentry); @@ -1366,7 +1380,7 @@ ldbm_back_add(Slapi_PBlock *pb) } /* Revert the caches if this is the parent operation */ - if (parent_op) { + if (parent_op && betxn_callback_fails) { revert_cache(inst, &parent_time); } } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 2542ff7eb0..5f07121ab4 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -80,6 +80,7 @@ ldbm_back_delete(Slapi_PBlock *pb) int result_sent = 0; Connection *pb_conn; int32_t parent_op = 0; + int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ struct timespec parent_time; if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) { @@ -434,6 +435,9 @@ ldbm_back_delete(Slapi_PBlock *pb) &ldap_result_code : &retval ); } slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); + if (retval) { + betxn_callback_fails = 1; + } goto error_return; } } @@ -717,6 +721,9 @@ ldbm_back_delete(Slapi_PBlock *pb) ldap_result_code ? &ldap_result_code : &retval); } slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); + if (retval) { + betxn_callback_fails = 1; + } goto error_return; } } @@ -746,6 +753,9 @@ ldbm_back_delete(Slapi_PBlock *pb) } /* retval is -1 */ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); + if (rc) { + betxn_callback_fails = 1; + } goto error_return; } slapi_pblock_set(pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry); @@ -1266,6 +1276,7 @@ ldbm_back_delete(Slapi_PBlock *pb) slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &retval); } slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); + betxn_callback_fails = 1; goto error_return; } if (parent_found) { @@ -1281,6 +1292,7 @@ ldbm_back_delete(Slapi_PBlock *pb) retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_DELETE_FN); if (retval) { + betxn_callback_fails = 1; ldbm_set_error(pb, retval, &ldap_result_code, &ldap_result_message); goto error_return; } @@ -1416,8 +1428,12 @@ ldbm_back_delete(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); + betxn_callback_fails = 1; } retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_DELETE_FN); + if (retval) { + betxn_callback_fails = 1; + } /* Release SERIAL LOCK */ dblayer_txn_abort(be, &txn); /* abort crashes in case disk full */ @@ -1437,7 +1453,7 @@ ldbm_back_delete(Slapi_PBlock *pb) } /* Revert the caches if this is the parent operation */ - if (parent_op) { + if (parent_op && betxn_callback_fails) { revert_cache(inst, &parent_time); } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index 4d86472237..1b5c1fecdc 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -522,6 +522,7 @@ ldbm_back_modify(Slapi_PBlock *pb) int ec_locked = 0; int result_sent = 0; int32_t parent_op = 0; + int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ struct timespec parent_time; Slapi_Mods *smods_add_rdn = NULL; @@ -817,6 +818,9 @@ 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); + if (retval) { + betxn_callback_fails = 1; + } goto error_return; } @@ -1017,10 +1021,12 @@ 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); + betxn_callback_fails = 1; goto error_return; } retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_MODIFY_FN); if (retval) { + betxn_callback_fails = 1; ldbm_set_error(pb, retval, &ldap_result_code, &ldap_result_message); goto error_return; } @@ -1083,8 +1089,12 @@ ldbm_back_modify(Slapi_PBlock *pb) if (!opreturn) { slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code : &retval); } + betxn_callback_fails = 1; } retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_MODIFY_FN); + if (retval) { + betxn_callback_fails = 1; + } /* It is safer not to abort when the transaction is not started. */ /* Release SERIAL LOCK */ @@ -1096,7 +1106,7 @@ ldbm_back_modify(Slapi_PBlock *pb) rc = SLAPI_FAIL_GENERAL; } /* Revert the caches if this is the parent operation */ - if (parent_op) { + if (parent_op && betxn_callback_fails) { revert_cache(inst, &parent_time); } } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index c5e9be4aa6..659b7a33ae 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -101,6 +101,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb) int result_sent = 0; Connection *pb_conn = NULL; int32_t parent_op = 0; + int32_t betxn_callback_fails = 0; /* if a BETXN fails we need to revert entry cache */ struct timespec parent_time; Slapi_Mods *smods_add_rdn = NULL; @@ -999,6 +1000,9 @@ 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); + if (retval) { + betxn_callback_fails = 1; + } goto error_return; } @@ -1250,10 +1254,12 @@ 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); + betxn_callback_fails = 1; goto error_return; } retval = plugin_call_mmr_plugin_postop(pb, NULL,SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN); if (retval) { + betxn_callback_fails = 1; ldbm_set_error(pb, retval, &ldap_result_code, &ldap_result_message); goto error_return; } @@ -1319,7 +1325,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb) error_return: /* Revert the caches if this is the parent operation */ - if (parent_op) { + if (parent_op && betxn_callback_fails) { revert_cache(inst, &parent_time); } @@ -1401,7 +1407,9 @@ ldbm_back_modrdn(Slapi_PBlock *pb) } slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); - /* Revert the caches if this is the parent operation */ + /* As it is a BETXN plugin failure then + * revert the caches if this is the parent operation + */ if (parent_op) { revert_cache(inst, &parent_time); }