Issue 6430 - implement read-only bdb #6431

progier389 · 2024-12-03T12:50:38Z

This PR:

implement librobdb as derivated work from rpm lib
add configure flag to allow 389ds to use librobdb with some glue code interfacing bdb api toward the read only functions.
Modify spec file to build 389ds with that librairy if libdb is not available

Issue: #6430

Reviewed by: @tbordaz and @vashirov (Thanks)

tbordaz

The overall looks good. Few comments

tbordaz · 2024-12-04T10:15:40Z

ldap/servers/slapd/back-ldbm/db-bdb/bdb_bdbreader_glue.c

+#include "bdb_layer.h"
+#include <robdb.h>
+
+#define DEBUG 1


Shouldn't it be 0 by default

should not even be defined at all (since we are using #ifdef)

tbordaz · 2024-12-04T10:31:22Z

ldap/servers/slapd/back-ldbm/db-bdb/bdb_bdbreader_glue.c

+{
+    int rc = bdbreader_cur_lookup(db->cur->impl, key->data, key->size);
+    if (rc == DB_SUCCESS) {
+        rc = dbc_get(db->cur, key, data, flags);


Shouldn't it call db->cur->c_get than dbc_get ?

db->cur->c_get == dbc_get so it does not really matter.
But to respect the layered approach since we are in the glue code that maps bdb API to robdb API,
bdb_bdbreader_glue.c code should use either internal or librobdb function rather than the bdb API
(So dbc_get looks good.)

tbordaz · 2024-12-04T10:32:30Z

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

@@ -397,6 +397,7 @@ const char *dblayer_op2str(dbi_op_t op)
    return str[idx];
 }

+<<<<<<< HEAD


Probably a leftover from a merge.

Indeed: bad conflict resolution when rebasing to main ...

tbordaz · 2024-12-04T10:48:05Z

ldap/servers/slapd/back-ldbm/db-bdb/bdb_bdbreader_glue.c

+
+int dbc_close(DBC *dbc)
+{
+     bdbreader_bdb_close(dbc->impl);


Did you mean bdbreader_cur_close ?

progier389 · 2024-12-04T14:45:27Z

Fixed review cooments about DEBUG, bdbreader_cur_close, and rebase conflict issue

vashirov · 2024-12-04T14:40:45Z

rpm/389-ds-base.spec.in

@@ -540,11 +582,11 @@ if ! make DESTDIR="$RPM_BUILD_ROOT" check; then cat ./test-suite.log && false; f

 %post
 if [ -n "$DEBUGPOSTTRANS" ] ; then
-    output=$DEBUGPOSTTRANS


Indentation was removed here and below, is this intentional?

Good catch!

vashirov · 2024-12-04T14:41:41Z

lib/librobdb/robdb.spec

+Summary:        Provide basic functions to search and read Berkeley Database records
+
+License:        GPL-2.0-or-later
+URL:            https://github.com/389ds/389-ds-base/lib/librobdb


That is expected:
Right now, lib/librobdb does not exists yet on main branch, but it will exist once the PR get merged.

Then it would be something like this (note the tree and main branch in the URL):
https://github.com/389ds/389-ds-base/tree/main/lib/librobdb

lib/librobdb/robdb.spec

vashirov · 2024-12-04T14:47:44Z

lib/librobdb/robdb.spec

+%doc %{_defaultdocdir}/%{name}-devel/README.md
+%{_libdir}/*.so
+%{_includedir}/*
+


Since there is test.c for robdb, maybe we should run it as part of %check section?

Added a make localtest to be able to run the test without having to install the rpm first
and added a %check section to use it

progier389 · 2024-12-04T15:39:28Z

Pushed commit about Viktor's review comments

progier389 · 2024-12-05T14:16:49Z

Pushed new commit to fix the URL

tbordaz

Impressive patch. Copyrights may be set to 2025 (rather than 2024)
LGTM
(could you restart the (failing) tests in the PR before pushing)

vashirov

I can't build the package without robdb, configure picks it up anyway:

checking for db.h... using lib/librobdb/lib/robdb.h

And then fails with

error: Installed (but unpackaged) file(s) found:
   /usr/lib/debug/usr/lib64/dirsrv/librobdb.so-3.1.1.202501071235git0db4461d6-1.fc41.aarch64.debug
   /usr/lib64/dirsrv/librobdb.so

RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/lib/debug/usr/lib64/dirsrv/librobdb.so-3.1.1.202501071235git0db4461d6-1.fc41.aarch64.debug
   /usr/lib64/dirsrv/librobdb.so
make: *** [rpm.mk:178: rpms] Error 1

I provided some suggestions to the spec file, but I need to test this more.

rpm/389-ds-base.spec.in

vashirov · 2025-01-07T12:29:58Z

rpm.mk

+ifeq ($(BUNDLE_LIBDB), 1)
+BUNDLE_BDBREADERS ?= 0
+else
+ifeq (,$(wildcard /usr/include/db.h#))


Is # a typo? With this I'm always getting robdb-libs package generated.

Indeed! I think that I added the # for some testing and forgot to remove it afterwards 😉

rpm.mk

rpm/389-ds-base.spec.in

vashirov · 2025-01-07T12:44:04Z

Makefile.am

@@ -325,7 +332,7 @@ bin_PROGRAMS = dbscan \
 # based on defines
 # ----------------------------------------------------------------------------------------

-server_LTLIBRARIES = libslapd.la libldaputil.la libns-dshttpd.la librewriters.la
+server_LTLIBRARIES = libslapd.la libldaputil.la libns-dshttpd.la librewriters.la librobdb.la


Shouldn't this be also behind a condition?

Indeed! I will fix it.

progier389 · 2025-01-15T20:33:44Z

fixed the build break:

logic was flawed and rpmbuild was called without robdb but configure was called with robdb
avoid to try to install binaries before building them 😉
need to exclude robdb library from main package

progier389 · 2025-01-16T11:22:34Z

Build is now ok, but there is a problem with the CI test:

Completion plugin: Generating completion cache...
--> Starting dependency resolution
--> Finished dependency resolution
Error: 
 Problem 1: conflicting requests
  - nothing provides 3.1.1.202501152030git3a8fe1d-1.fc40 needed by 389-ds-base-3.1.1.202501152030git3a8fe1d-1.fc40.x86_64 from @commandline
  - nothing provides 389-ds-base-robdb-libs= needed by 389-ds-base-3.1.1.202501152030git3a8fe1d-1.fc40.x86_64 from @commandline

Probably an error in the Require dependency:
should be 389-ds-base-robdb-libs-3.1.1.202501152021git3a8fe1dd9-1.fc40.x86_64 instead of 389-ds-base-robdb-libs-3.1.1.202501152021git3a8fe1dd9-1

vashirov · 2025-01-16T14:10:32Z

nothing provides 389-ds-base-robdb-libs=

There should be space before =

progier389 · 2025-01-17T13:25:41Z

Fixed the dependency as viktor proposed and now the tests results are the expected one:

most bdb tests are failing because the instance cannot be started in bdb mode.
same for the few lmdb tests that try to switch to bdb
and a few lmdb tests that are known to be unreliable

progier389 · 2025-01-21T13:57:09Z

Still some work to do around the CI tests to skip all the bdb tests and the few lmdb tests that tries to generate a bdb instance - should also a a test to check bdb migration as if the system was upgraded

progier389 force-pushed the i0_bdb_ro branch from 94576bf to 172c87f Compare December 3, 2024 12:56

progier389 linked an issue Dec 3, 2024 that may be closed by this pull request

Solution to be able to upgrade existing instance when bdb will be no more available #6430

Open

tbordaz reviewed Dec 4, 2024

View reviewed changes

vashirov reviewed Dec 4, 2024

View reviewed changes

progier389 force-pushed the i0_bdb_ro branch from e38e67e to 875f395 Compare December 4, 2024 15:35

tbordaz approved these changes Jan 7, 2025

View reviewed changes

vashirov requested changes Jan 7, 2025

View reviewed changes

progier389 added the work in progress Work in Progress - can be reviewed, but not ready for merge. label Jan 15, 2025

progier389 force-pushed the i0_bdb_ro branch from d633be9 to 3a8fe1d Compare January 15, 2025 20:29

progier389 force-pushed the i0_bdb_ro branch 2 times, most recently from 5da66d9 to 03f1436 Compare January 16, 2025 15:30

progier389 removed the work in progress Work in Progress - can be reviewed, but not ready for merge. label Jan 17, 2025

progier389 force-pushed the i0_bdb_ro branch from 9ec1c75 to d0f3e01 Compare January 20, 2025 15:15

progier389 added the work in progress Work in Progress - can be reviewed, but not ready for merge. label Jan 21, 2025

progier389 force-pushed the i0_bdb_ro branch 8 times, most recently from 0f87706 to fb81fe8 Compare January 23, 2025 15:00

progier389 added 7 commits January 23, 2025 16:01

Issue 6430 - implement read-only bdb

afe29b7

issue 6430 - Fix review comments

d58cb26

Issue 6430 - Fix spec file URL

35c8ad1

bdb read-only: fix some of Viktor's review comments

340c68d

Fix broken build

80e7102

fix dependency toward robdb lib

44cebbd

Skip bdb CI tests and fix librobdb symbols in ns-slapd and lib389

089db2a

progier389 force-pushed the i0_bdb_ro branch from fb81fe8 to 089db2a Compare January 24, 2025 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 6430 - implement read-only bdb #6431

Issue 6430 - implement read-only bdb #6431

progier389 commented Dec 3, 2024 •

edited

Loading

tbordaz left a comment

tbordaz Dec 4, 2024

progier389 Dec 4, 2024

tbordaz Dec 4, 2024

progier389 Dec 4, 2024

tbordaz Dec 4, 2024

progier389 Dec 4, 2024

tbordaz Dec 4, 2024

progier389 commented Dec 4, 2024

vashirov Dec 4, 2024

progier389 Dec 4, 2024

vashirov Dec 4, 2024

progier389 Dec 4, 2024

vashirov Dec 5, 2024

vashirov Dec 4, 2024

progier389 Dec 4, 2024

progier389 commented Dec 4, 2024

progier389 commented Dec 5, 2024

tbordaz left a comment •

edited

Loading

vashirov left a comment

vashirov Jan 7, 2025

progier389 Jan 9, 2025

vashirov Jan 7, 2025

progier389 Jan 9, 2025

progier389 commented Jan 15, 2025

progier389 commented Jan 16, 2025

vashirov commented Jan 16, 2025 •

edited

Loading

progier389 commented Jan 17, 2025

progier389 commented Jan 21, 2025

Issue 6430 - implement read-only bdb #6431

Are you sure you want to change the base?

Issue 6430 - implement read-only bdb #6431

Conversation

progier389 commented Dec 3, 2024 • edited Loading

tbordaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

progier389 commented Dec 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

progier389 commented Dec 4, 2024

progier389 commented Dec 5, 2024

tbordaz left a comment • edited Loading

Choose a reason for hiding this comment

vashirov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

progier389 commented Jan 15, 2025

progier389 commented Jan 16, 2025

vashirov commented Jan 16, 2025 • edited Loading

progier389 commented Jan 17, 2025

progier389 commented Jan 21, 2025

progier389 commented Dec 3, 2024 •

edited

Loading

tbordaz left a comment •

edited

Loading

vashirov commented Jan 16, 2025 •

edited

Loading