WIP: lmdb support #46

uhliarik · 2023-09-27T12:52:06Z

There is still WIP.

minfrin · 2023-09-27T13:02:21Z

Not had a chance to review, but definite +1 in principle.

uhliarik · 2023-09-27T15:49:31Z

Since I had to rebase this code to the trunk, I'm no longer able to run tests. If anyone can see where I'm missing linking the library, feel free to review the commit.

$ make test
cd test && make all check
make[1]: Entering directory '/home/luhliari/playground/UPSTREAM/APR/apr_lmdb/test'
make[2]: Entering directory '/home/luhliari/playground/UPSTREAM/APR/apr_lmdb/test'
/bin/sh /home/luhliari/playground/UPSTREAM/APR/apr_lmdb/libtool --silent --mode=compile --tag=CC gcc -g -O2 -pthread   -DHAVE_CONFIG_H  -DLINUX -D_REENTRANT -D_GNU_SOURCE   -I../include -I./../include  -o globalmutexchild.lo -c globalmutexchild.c && touch globalmutexchild.lo
/bin/sh /home/luhliari/playground/UPSTREAM/APR/apr_lmdb/libtool --silent --mode=link --tag=CC gcc -g -O2 -pthread   -DHAVE_CONFIG_H  -DLINUX -D_REENTRANT -D_GNU_SOURCE   -I../include -I./../include   -no-install    -o globalmutexchild globalmutexchild.lo ../libapr-2.la   -lcrypt -lrt -lcrypt  -lpthread -ldl -lexpat
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_drop'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_env_close'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_cursor_get'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_del'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_env_open'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_strerror'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_cursor_open'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_txn_commit'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_dbi_close'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_dbi_open'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_cursor_close'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_env_create'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_txn_begin'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_env_set_mapsize'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_put'
/usr/bin/ld: ../.libs/libapr-2.so: undefined reference to `mdb_get'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:125: globalmutexchild] Error 1
make[2]: Leaving directory '/home/luhliari/playground/UPSTREAM/APR/apr_lmdb/test'
make[1]: *** [/home/luhliari/playground/UPSTREAM/APR/apr_lmdb/build/apr_rules.mk:118: all-recursive] Error 1
make[1]: Leaving directory '/home/luhliari/playground/UPSTREAM/APR/apr_lmdb/test'

Add -llmdb to test/Makefile.in so test can be compiled without any error.

notroj · 2023-09-28T07:48:52Z

build.conf

@@ -22,6 +22,7 @@ paths =
  crypto/crypt_blowfish.c
  dbm/apr_dbm_sdbm.c
  dbm/apr_dbm.c
+  dbm/apr_dbm_lmdb.c


This should be removed, the DBM drivers which can be built as DSOs should not be listed here otherwise it will get linked twice.

notroj · 2023-09-28T07:50:42Z

With the line marked in my review above removed, and e147886 reverted, I can build with:

"./configure" \
"--with-lmdb" \
"--with-berkeley-db" \
"--with-dbm=db5" \
"--disable-modular-dso"

and this passes the tests. Thanks @uhliarik!

notroj · 2023-09-28T10:08:38Z

I've tested this with mod_dav and it seems to work well. As well as the changes described above I changed the _usednames implementation to pass back the "-lock" file as well:

notroj@26afe6b

- Remove lmdb driver from build.conf so it won't get linked twice! - Revert e147886

uhliarik · 2023-09-28T10:42:50Z

Thanks Joe for reviewing my changes. Previously, I was configurating the lmdb driver with:

./configure --with-lmdb --with-dbm=lmdb

And I was having troubles with loading lmdb driver as DSO. It was failing during DSO loading phase. I hope there is no issue with this and it is just some configuration thing.

- Removed wrong lines from build.conf in ad34f18. Revering and removing correct line

if using MDB_NOSUBDIR per http://www.lmdb.tech/doc/group__mdb.html#ga32a193c6bf4d7d5c5d579e71f22e9340

@notroj

if using MDB_NOSUBDIR per http://www.lmdb.tech/doc/group__mdb.html#ga32a193c6bf4d7d5c5d579e71f22e9340 Merging the change suggested by @notroj apache@26afe6b

a non-existent database.

(just like apr_status_t) map those values directly; map all other errors as APR_EGENERAL. APR-internal APIs should not use error codes within the APR_OS_START_USEERR number region (mistake common to dbm/*.c.)

overhead of per-transaction fdatasync() calls, bringing comparable behaviour/performance to BDB etc. Arguably this behaviour could be controlled by a new flag passed in the mode parameter of apr_dbm_open*.

Lmdb support

which was originally used for dynamic database resizing in case the database was full. This function is no longer used and the size of the database is set to UINT32_MAX in vt_lmdb_open() function.

notroj · 2023-09-29T13:57:41Z

Thanks a lot for the contribution, @uhliarik

Initial commit for lmdb support

f0379e7

Add -llmdb

e147886

Add -llmdb to test/Makefile.in so test can be compiled without any error.

notroj reviewed Sep 28, 2023

View reviewed changes

Remove lmdb driver from build.conf and revert e147886

ad34f18

- Remove lmdb driver from build.conf so it won't get linked twice! - Revert e147886

uhliarik and others added 14 commits September 28, 2023 12:46

Removed wrong lines from build.conf in ad34f18

fcf0580

- Removed wrong lines from build.conf in ad34f18. Revering and removing correct line

Pass the -lock file back via _usednames. This seems to be reliable

c1e6ae7

if using MDB_NOSUBDIR per http://www.lmdb.tech/doc/group__mdb.html#ga32a193c6bf4d7d5c5d579e71f22e9340

Pass the -lock file back via _usednames. This seems to be reliable

7bf4a66

if using MDB_NOSUBDIR per http://www.lmdb.tech/doc/group__mdb.html#ga32a193c6bf4d7d5c5d579e71f22e9340 Merging the change suggested by @notroj apache@26afe6b

Move #include <lmdb.h> inside conditional.

61a15b8

Add LMDB test build.

d5c6f8a

Use --with-lmdb for LMDB test build (duh).

45bf67d

Fix Debian package name.

11fee2b

Fix GCC warnings.

43bcfef

Add BDB test.

6df0b13

Remove unnecessary apr_dbm_t struct and add test for opening

f10d2ce

a non-existent database.

Simplify error handling - since mdb_* can return errno as positive

f8f484d

(just like apr_status_t) map those values directly; map all other errors as APR_EGENERAL. APR-internal APIs should not use error codes within the APR_OS_START_USEERR number region (mistake common to dbm/*.c.)

Default to using MDB_NOSYNC when opening the database to avoid

49e1023

overhead of per-transaction fdatasync() calls, bringing comparable behaviour/performance to BDB etc. Arguably this behaviour could be controlled by a new flag passed in the mode parameter of apr_dbm_open*.

Merge pull request #1 from notroj/lmdb_support

e142d06

Lmdb support

Remove lmdb_retry function

3a539c1

which was originally used for dynamic database resizing in case the database was full. This function is no longer used and the size of the database is set to UINT32_MAX in vt_lmdb_open() function.

asfgit closed this in 4e6f06b Sep 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: lmdb support #46

WIP: lmdb support #46

uhliarik commented Sep 27, 2023

minfrin commented Sep 27, 2023

uhliarik commented Sep 27, 2023 •

edited

notroj Sep 28, 2023

notroj commented Sep 28, 2023

notroj commented Sep 28, 2023

uhliarik commented Sep 28, 2023

notroj commented Sep 29, 2023

WIP: lmdb support #46

WIP: lmdb support #46

Conversation

uhliarik commented Sep 27, 2023

minfrin commented Sep 27, 2023

uhliarik commented Sep 27, 2023 • edited

notroj Sep 28, 2023

Choose a reason for hiding this comment

notroj commented Sep 28, 2023

notroj commented Sep 28, 2023

uhliarik commented Sep 28, 2023

notroj commented Sep 29, 2023

uhliarik commented Sep 27, 2023 •

edited