Resolves: RHEL-58070 - lmdb reindex is broken if index type is specified

This commit is contained in:
Viktor Ashirov 2024-09-16 12:57:54 +02:00
parent 53cbf5eef2
commit 396f783b35
2 changed files with 244 additions and 6 deletions

View File

@ -0,0 +1,237 @@
From a251914c8defce11c3f8496406af8dec6cf50c4b Mon Sep 17 00:00:00 2001
From: progier389 <progier@redhat.com>
Date: Fri, 6 Sep 2024 18:07:17 +0200
Subject: [PATCH] Issue 6316 - lmdb reindex is broken if index type is
specified (#6318)
While reindexing using task or offline reindex, if the attribute name contains the index type (for example :eq,pres)
Then the attribute is not reindexed. Problem occurs when lmdb is used, things are working fine with bdb.
Solution: strip the index type in reindex as it is done in bdb case.
Anyway the reindex design requires that for a given attribute all the configured index types must be rebuild.
Issue: #6316
Reviewed by: @tbordaz, @droideck (Thanks!)
---
.../tests/suites/indexes/regression_test.py | 141 +++++++++++++++++-
.../slapd/back-ldbm/db-mdb/mdb_import.c | 10 +-
2 files changed, 147 insertions(+), 4 deletions(-)
diff --git a/dirsrvtests/tests/suites/indexes/regression_test.py b/dirsrvtests/tests/suites/indexes/regression_test.py
index 51f88017d..e98ca6172 100644
--- a/dirsrvtests/tests/suites/indexes/regression_test.py
+++ b/dirsrvtests/tests/suites/indexes/regression_test.py
@@ -10,6 +10,9 @@ import time
import os
import pytest
import ldap
+import logging
+import glob
+import re
from lib389._constants import DEFAULT_BENAME, DEFAULT_SUFFIX
from lib389.backend import Backend, Backends, DatabaseConfig
from lib389.cos import CosClassicDefinition, CosClassicDefinitions, CosTemplate
@@ -31,6 +34,8 @@ SUFFIX2 = 'dc=example2,dc=com'
BENAME2 = 'be2'
DEBUGGING = os.getenv("DEBUGGING", default=False)
+logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
@pytest.fixture(scope="function")
@@ -83,6 +88,7 @@ def add_a_group_with_users(request, topo):
'cn': USER_NAME,
'uidNumber': f'{num}',
'gidNumber': f'{num}',
+ 'description': f'Description for {USER_NAME}',
'homeDirectory': f'/home/{USER_NAME}'
})
users_list.append(user)
@@ -95,9 +101,10 @@ def add_a_group_with_users(request, topo):
# If the server crashed, start it again to do the cleanup
if not topo.standalone.status():
topo.standalone.start()
- for user in users_list:
- user.delete()
- group.delete()
+ if not DEBUGGING:
+ for user in users_list:
+ user.delete()
+ group.delete()
request.addfinalizer(fin)
@@ -124,6 +131,38 @@ def set_small_idlistscanlimit(request, topo):
request.addfinalizer(fin)
+
+@pytest.fixture(scope="function")
+def set_description_index(request, topo, add_a_group_with_users):
+ """
+ Set some description values and description index without reindexing.
+ """
+ inst = topo.standalone
+ backends = Backends(inst)
+ backend = backends.get(DEFAULT_BENAME)
+ indexes = backend.get_indexes()
+ attr = 'description'
+
+ def fin(always=False):
+ if always or not DEBUGGING:
+ try:
+ idx = indexes.get(attr)
+ idx.delete()
+ except ldap.NO_SUCH_OBJECT:
+ pass
+
+ request.addfinalizer(fin)
+ fin(always=True)
+ index = indexes.create(properties={
+ 'cn': attr,
+ 'nsSystemIndex': 'false',
+ 'nsIndexType': ['eq', 'pres', 'sub']
+ })
+ # Restart needed with lmdb (to open the dbi handle)
+ inst.restart()
+ return (indexes, attr)
+
+
#unstable or unstatus tests, skipped for now
@pytest.mark.flaky(max_runs=2, min_passes=1)
@pytest.mark.skipif(ds_is_older("1.4.4.4"), reason="Not implemented")
@@ -346,6 +385,102 @@ def test_task_status(topo):
assert reindex_task.get_exit_code() == 0
+def count_keys(inst, bename, attr, prefix=''):
+ indexfile = os.path.join(inst.dbdir, bename, attr + '.db')
+ # (bdb - we should also accept a version number for .db suffix)
+ for f in glob.glob(f'{indexfile}*'):
+ indexfile = f
+
+ inst.stop()
+ output = inst.dbscan(None, None, args=['-f', indexfile, '-A'], stopping=False).decode()
+ inst.start()
+ count = 0
+ regexp = f'^KEY: {re.escape(prefix)}'
+ for match in re.finditer(regexp, output, flags=re.MULTILINE):
+ count += 1
+ log.info(f"count_keys found {count} keys starting with '{prefix}' in {indexfile}")
+ return count
+
+
+def test_reindex_task_with_type(topo, set_description_index):
+ """Check that reindex task works as expected when index type is specified.
+
+ :id: 0c7f2fda-69f6-11ef-9eb8-083a88554478
+ :setup: Standalone instance
+ - with 100 users having description attribute
+ - with description:eq,pres,sub index entry but not yet reindexed
+ :steps:
+ 1. Set description in suffix entry
+ 2. Count number of equality keys in description index
+ 3. Start a Reindex task on description:eq,pres and wait for completion
+ 4. Check the task status and exit code
+ 5. Count the equality, presence and substring keys in description index
+ 6. Start a Reindex task on description and wait for completion
+ 7. Check the task status and exit code
+ 8. Count the equality, presence and substring keys in description index
+
+ :expectedresults:
+ 1. Success
+ 2. Should be either no key (bdb) or a single one (lmdb)
+ 3. Success
+ 4. Success
+ 5. Should have: more equality keys than in step 2
+ one presence key
+ some substrings keys
+ 6. Success
+ 7. Success
+ 8. Should have same counts than in step 5
+ """
+ (indexes, attr) = set_description_index
+ inst = topo.standalone
+ if not inst.is_dbi_supported():
+ pytest.skip('This test requires that dbscan supports -A option')
+ # modify indexed value
+ Domain(inst, DEFAULT_SUFFIX).replace(attr, f'test_before_reindex')
+
+ keys1 = count_keys(inst, DEFAULT_BENAME, attr, prefix='=')
+ assert keys1 <= 1
+
+ tasks = Tasks(topo.standalone)
+ # completed reindex tasks MUST have a status because freeipa check it.
+
+ # Reindex attr with eq,pres types
+ log.info(f'Reindex {attr} with eq,pres types')
+ tasks.reindex(
+ suffix=DEFAULT_SUFFIX,
+ attrname=f'{attr}:eq,pres',
+ args={TASK_WAIT: True}
+ )
+ reindex_task = Task(topo.standalone, tasks.dn)
+ assert reindex_task.status()
+ assert reindex_task.get_exit_code() == 0
+
+ keys2e = count_keys(inst, DEFAULT_BENAME, attr, prefix='=')
+ keys2p = count_keys(inst, DEFAULT_BENAME, attr, prefix='+')
+ keys2s = count_keys(inst, DEFAULT_BENAME, attr, prefix='*')
+ assert keys2e > keys1
+ assert keys2p > 0
+ assert keys2s > 0
+
+ # Reindex attr without types
+ log.info(f'Reindex {attr} without types')
+ tasks.reindex(
+ suffix=DEFAULT_SUFFIX,
+ attrname=attr,
+ args={TASK_WAIT: True}
+ )
+ reindex_task = Task(topo.standalone, tasks.dn)
+ assert reindex_task.status()
+ assert reindex_task.get_exit_code() == 0
+
+ keys3e = count_keys(inst, DEFAULT_BENAME, attr, prefix='=')
+ keys3p = count_keys(inst, DEFAULT_BENAME, attr, prefix='+')
+ keys3s = count_keys(inst, DEFAULT_BENAME, attr, prefix='*')
+ assert keys3e == keys2e
+ assert keys3p == keys2p
+ assert keys3s == keys2s
+
+
def test_task_and_be(topo, add_backend_and_ldif_50K_users):
"""Check that backend is writable after finishing a tasks
diff --git a/ldap/servers/slapd/back-ldbm/db-mdb/mdb_import.c b/ldap/servers/slapd/back-ldbm/db-mdb/mdb_import.c
index cfd9de268..5f8e36cdc 100644
--- a/ldap/servers/slapd/back-ldbm/db-mdb/mdb_import.c
+++ b/ldap/servers/slapd/back-ldbm/db-mdb/mdb_import.c
@@ -1150,6 +1150,8 @@ process_db2index_attrs(Slapi_PBlock *pb, ImportCtx_t *ctx)
* TBD
*/
char **attrs = NULL;
+ char *attrname = NULL;
+ char *pt = NULL;
int i;
slapi_pblock_get(pb, SLAPI_DB2INDEX_ATTRS, &attrs);
@@ -1157,7 +1159,13 @@ process_db2index_attrs(Slapi_PBlock *pb, ImportCtx_t *ctx)
for (i = 0; attrs && attrs[i]; i++) {
switch (attrs[i][0]) {
case 't': /* attribute type to index */
- slapi_ch_array_add(&ctx->indexAttrs, slapi_ch_strdup(attrs[i] + 1));
+ attrname = slapi_ch_strdup(attrs[i] + 1);
+ /* Strip index type */
+ pt = strchr(attrname, ':');
+ if (pt != NULL) {
+ *pt = '\0';
+ }
+ slapi_ch_array_add(&ctx->indexAttrs, attrname);
break;
case 'T': /* VLV Search to index */
slapi_ch_array_add(&ctx->indexVlvs, get_vlv_dbname(attrs[i] + 1));
--
2.46.0

View File

@ -487,6 +487,8 @@ Source4: 389-ds-base.sysusers
Source5: https://fedorapeople.org/groups/389ds/libdb-5.3.28-59.tar.bz2 Source5: https://fedorapeople.org/groups/389ds/libdb-5.3.28-59.tar.bz2
%endif %endif
Patch: 0001-Issue-6316-lmdb-reindex-is-broken-if-index-type-is-s.patch
%description %description
389 Directory Server is an LDAPv3 compliant server. The base package includes 389 Directory Server is an LDAPv3 compliant server. The base package includes
the LDAP server and command line utilities for server administration. the LDAP server and command line utilities for server administration.
@ -558,11 +560,6 @@ SNMP Agent for the 389 Directory Server base package.
%if %{with bundle_libdb} %if %{with bundle_libdb}
%package bdb %package bdb
Summary: Berkeley Database backend for 389 Directory Server Summary: Berkeley Database backend for 389 Directory Server
%description bdb
Berkeley Database backend for 389 Directory Server
Warning! This backend is deprecated in favor of lmdb and its support
may be removed in future versions.
Requires: %{name} = %{version}-%{release} Requires: %{name} = %{version}-%{release}
Requires: %{name}-libs = %{version}-%{release} Requires: %{name}-libs = %{version}-%{release}
# Berkeley DB database libdb was marked as deprecated since F40: # Berkeley DB database libdb was marked as deprecated since F40:
@ -570,8 +567,12 @@ Requires: %{name}-libs = %{version}-%{release}
# because libdb was marked as deprecated since F33 # because libdb was marked as deprecated since F33
# https://fedoraproject.org/wiki/Changes/Libdb_deprecated # https://fedoraproject.org/wiki/Changes/Libdb_deprecated
Provides: deprecated() Provides: deprecated()
%endif
%description bdb
Berkeley Database backend for 389 Directory Server
Warning! This backend is deprecated in favor of lmdb and its support
may be removed in future versions.
%endif
%package -n python%{python3_pkgversion}-lib389 %package -n python%{python3_pkgversion}-lib389
Summary: A library for accessing, testing, and configuring the 389 Directory Server Summary: A library for accessing, testing, and configuring the 389 Directory Server