Issue 4443 - Internal unindexed searches in syncrepl/retro changelog

Bug Description:

When a non-system index is added to a backend it is
disabled until the database is initialized or reindexed.
So in the case of the retro changelog the changenumber index
is alway disabled by default since it is never initialized.
This leads to unexpected unindexed searches of the retro
changelog.

Fix Description:

If an index has "nsSystemIndex" set to "true" then enable it
immediately.

relates:  #4443

Reviewed by: spichugi & tbordaz(Thanks!!)

dirsrvtests/tests/suites/retrocl/retrocl_indexing_test.py

@@ -0,0 +1,68 @@
+import logging
+import pytest
+import os
+from lib389._constants import RETROCL_SUFFIX, DEFAULT_SUFFIX
+from lib389.topologies import topology_st as topo
+from lib389.plugins import RetroChangelogPlugin
+from lib389.idm.user import UserAccounts
+from lib389._mapped_object import DSLdapObjects
+log = logging.getLogger(__name__)
+
+
+def test_indexing_is_online(topo):
+    """Test that the changenmumber index is online right after enabling the plugin
+
+    :id: 16f4c001-9e0c-4448-a2b3-08ac1e85d40f
+    :setup: Standalone Instance
+    :steps:
+        1. Enable retro cl
+        2. Perform some updates
+        3. Search for "(changenumber>=-1)", and it is not partially unindexed
+        4. Search for "(&(changenumber>=-1)(targetuniqueid=*))", and it is not partially unindexed
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+        4. Success
+    """
+
+    # Enable plugin
+    topo.standalone.config.set('nsslapd-accesslog-logbuffering',  'off')
+    plugin = RetroChangelogPlugin(topo.standalone)
+    plugin.enable()
+    topo.standalone.restart()
+
+    # Do a bunch of updates
+    users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)
+    user_entry = users.create(properties={
+        'sn': '1',
+        'cn': 'user 1',
+        'uid': 'user1',
+        'uidNumber': '11',
+        'gidNumber': '111',
+        'givenname': 'user1',
+        'homePhone': '0861234567',
+        'carLicense': '131D16674',
+        'mail': 'user1@whereever.com',
+        'homeDirectory': '/home'
+    })
+    for count in range(0, 10):
+        user_entry.replace('mail', f'test{count}@test.com')
+
+    # Search the retro cl, and check for error messages
+    filter_simple = '(changenumber>=-1)'
+    filter_compound = '(&(changenumber>=-1)(targetuniqueid=*))'
+    retro_changelog_suffix = DSLdapObjects(topo.standalone, basedn=RETROCL_SUFFIX)
+    retro_changelog_suffix.filter(filter_simple)
+    assert not topo.standalone.searchAccessLog('Partially Unindexed Filter')
+
+    # Search the retro cl again with compound filter
+    retro_changelog_suffix.filter(filter_compound)
+    assert not topo.standalone.searchAccessLog('Partially Unindexed Filter')
+
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main(["-s", CURRENT_FILE])

ldap/servers/plugins/retrocl/retrocl_create.c

@@ -133,7 +133,7 @@ retrocl_create_be(const char *bedir)
     val.bv_len = strlen(val.bv_val);
     slapi_entry_add_values(e, "cn", vals);
 
-    val.bv_val = "false";
+    val.bv_val = "true"; /* enables the index */
     val.bv_len = strlen(val.bv_val);
     slapi_entry_add_values(e, "nssystemindex", vals);
 

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

@@ -25,7 +25,7 @@ int ldbm_instance_index_config_delete_callback(Slapi_PBlock *pb, Slapi_Entry *en
 #define INDEXTYPE_NONE 1
 
 static int
-ldbm_index_parse_entry(ldbm_instance *inst, Slapi_Entry *e, const char *trace_string, char **index_name)
+ldbm_index_parse_entry(ldbm_instance *inst, Slapi_Entry *e, const char *trace_string, char **index_name, PRBool *is_system_index, char *err_buf)
 {
     Slapi_Attr *attr;
     const struct berval *attrValue;
@@ -67,6 +67,15 @@ ldbm_index_parse_entry(ldbm_instance *inst, Slapi_Entry *e, const char *trace_st
         }
     }
 
+    *is_system_index = PR_FALSE;
+    if (0 == slapi_entry_attr_find(e, "nsSystemIndex", &attr)) {
+        slapi_attr_first_value(attr, &sval);
+        attrValue = slapi_value_get_berval(sval);
+        if (strcasecmp(attrValue->bv_val, "true") == 0) {
+            *is_system_index = PR_TRUE;
+        }
+    }
+
     /* ok the entry is good to process, pass it to attr_index_config */
     if (attr_index_config(inst->inst_be, (char *)trace_string, 0, e, 0, 0)) {
         slapi_ch_free_string(index_name);
@@ -90,9 +99,10 @@ ldbm_index_init_entry_callback(Slapi_PBlock *pb __attribute__((unused)),
                                void *arg)
 {
     ldbm_instance *inst = (ldbm_instance *)arg;
+    PRBool is_system_index = PR_FALSE;
 
     returntext[0] = '\0';
-    *returncode = ldbm_index_parse_entry(inst, e, "from ldbm instance init", NULL);
+    *returncode = ldbm_index_parse_entry(inst, e, "from ldbm instance init", NULL, &is_system_index /* not used */, NULL);
     if (*returncode == LDAP_SUCCESS) {
         return SLAPI_DSE_CALLBACK_OK;
     } else {
@@ -115,17 +125,21 @@ ldbm_instance_index_config_add_callback(Slapi_PBlock *pb __attribute__((unused))
 {
     ldbm_instance *inst = (ldbm_instance *)arg;
     char *index_name = NULL;
+    PRBool is_system_index = PR_FALSE;
 
     returntext[0] = '\0';
-    *returncode = ldbm_index_parse_entry(inst, e, "from DSE add", &index_name);
+    *returncode = ldbm_index_parse_entry(inst, e, "from DSE add", &index_name, &is_system_index, returntext);
     if (*returncode == LDAP_SUCCESS) {
         struct attrinfo *ai = NULL;
         /* if the index is a "system" index, we assume it's being added by
          * by the server, and it's okay for the index to go online immediately.
          * if not, we set the index "offline" so it won't actually be used
          * until someone runs db2index on it.
+         * If caller wants to add an index that they want to be online
+         * immediately they can also set "nsSystemIndex" to "true" in the
+         * index config entry (e.g. is_system_index).
          */
-        if (!ldbm_attribute_always_indexed(index_name)) {
+        if (!is_system_index && !ldbm_attribute_always_indexed(index_name)) {
             ainfo_get(inst->inst_be, index_name, &ai);
             PR_ASSERT(ai != NULL);
             ai->ai_indexmask |= INDEX_OFFLINE;
@@ -361,13 +375,14 @@ ldbm_instance_index_config_enable_index(ldbm_instance *inst, Slapi_Entry *e)
     char *index_name = NULL;
     int rc = LDAP_SUCCESS;
     struct attrinfo *ai = NULL;
+    PRBool is_system_index = PR_FALSE;
 
     index_name = slapi_entry_attr_get_charptr(e, "cn");
     if (index_name) {
         ainfo_get(inst->inst_be, index_name, &ai);
     }
     if (!ai) {
-        rc = ldbm_index_parse_entry(inst, e, "from DSE add", &index_name);
+        rc = ldbm_index_parse_entry(inst, e, "from DSE add", &index_name, &is_system_index /* not used */, NULL);
     }
     if (rc == LDAP_SUCCESS) {
         /* Assume the caller knows if it is OK to go online immediately */

src/lib389/lib389/_mapped_object.py

@@ -147,6 +147,19 @@ def exists(self):
 
         return True
 
+    def search(self, scope="subtree", filter='objectclass=*'):
+        search_scope = ldap.SCOPE_SUBTREE
+        if scope == 'base':
+            search_scope = ldap.SCOPE_BASE
+        elif scope == 'one':
+            search_scope = ldap.SCOPE_ONE
+        elif scope == 'subtree':
+            search_scope = ldap.SCOPE_SUBTREE
+        return self._instance.search_ext_s(self._dn, search_scope, filter,
+                                           serverctrls=self._server_controls,
+                                           clientctrls=self._client_controls,
+                                           escapehatch='i am sure')
+
     def display(self, attrlist=['*']):
         """Get an entry but represent it as a string LDIF