You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Joe Orton <jo...@redhat.com> on 2023/09/25 14:58:16 UTC
apr_dbm and concurrency
It is unspecified whether the apr_dbm.h interface is safe to use for
multiple processes/threads having r/w access to a single database.
Results appear to be:
- sdbm, gdbm are safe
- bdb, ndbm are not safe
(Berkeley DB obviously can be used in a way which is safe for multiple
r/w users but it appears to require using one of the more complicated
modes of operation via a DB_ENV, and changing to that would not be
backwards compatible with the current db format. Corrections welcome,
not a database expert)
This seems pretty bad, httpd's use of this interface depends on the DBM
API being safe for concurrent use, but I'm not sure there is any good
way forward.
Options I can see:
1. Implement APR-specific locking inside apr_dbm for unsafe db types,
e.g. by creating a lockfile "<pathname>.lock" and use apr_file_lock
2. Drop concurrency-unsafe db methods... but, APR 2.x only?
3. No code change. Describe the state of concurrency-safety in the API
for each db type. httpd and other users would be forced to select a DB
type appropriate to the use case.
Any other suggestions, and any preferences among the above? I'm not sure
if 3 isn't the least bad choice unfortunately.
Regards, Joe
Re: mod_dav_fs locking / Re: apr_dbm and concurrency
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Nov 23, 2023 at 05:42:10PM +0000, Emmanuel Dreyfus wrote:
> On Thu, Nov 23, 2023 at 05:36:06PM +0000, Joe Orton wrote:
> > 3) in the mean time I worked up a PR for mod_dav_fs which adds a global
> > mutex around the dbm lockdb use. This passes my stress tests for the
> > first time.
>
> How concurent is the stress test?
I've been testing with 20 concurrent clients on an 8 core machine.
> In the past, I have been badly hurt by a few WebDAV clients proactively
> exploring the filesystem using locksdiscovery. That compeltely killed the
> service. I introduced the DavLockDiscovery directive to work it around.
This is a good point. Was the load in that case PRPOFIND/depth:infinity
do you know or "just" depth:1? A global lock like in my PR would make
the PROPFIND load even worse, since the lock is held for the duration of
the response and there are no concurrent read-only locks.
It might be necessary to disable lock discovery by default then, I don't
know if any clients rely on or expose that but it's only a "MAY" that
lock discvovery is possible in RFC4918. I suspect the lock recovery
mechanism for most users & clients is to delete the lockdb.
Regards, Joe
Re: mod_dav_fs locking / Re: apr_dbm and concurrency
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Nov 23, 2023 at 05:42:10PM +0000, Emmanuel Dreyfus wrote:
> On Thu, Nov 23, 2023 at 05:36:06PM +0000, Joe Orton wrote:
> > 3) in the mean time I worked up a PR for mod_dav_fs which adds a global
> > mutex around the dbm lockdb use. This passes my stress tests for the
> > first time.
>
> How concurent is the stress test?
I've been testing with 20 concurrent clients on an 8 core machine.
> In the past, I have been badly hurt by a few WebDAV clients proactively
> exploring the filesystem using locksdiscovery. That compeltely killed the
> service. I introduced the DavLockDiscovery directive to work it around.
This is a good point. Was the load in that case PRPOFIND/depth:infinity
do you know or "just" depth:1? A global lock like in my PR would make
the PROPFIND load even worse, since the lock is held for the duration of
the response and there are no concurrent read-only locks.
It might be necessary to disable lock discovery by default then, I don't
know if any clients rely on or expose that but it's only a "MAY" that
lock discvovery is possible in RFC4918. I suspect the lock recovery
mechanism for most users & clients is to delete the lockdb.
Regards, Joe
Re: mod_dav_fs locking / Re: apr_dbm and concurrency
Posted by Emmanuel Dreyfus <ma...@netbsd.org>.
On Thu, Nov 23, 2023 at 05:36:06PM +0000, Joe Orton wrote:
> 3) in the mean time I worked up a PR for mod_dav_fs which adds a global
> mutex around the dbm lockdb use. This passes my stress tests for the
> first time.
How concurent is the stress test?
In the past, I have been badly hurt by a few WebDAV clients proactively
exploring the filesystem using locksdiscovery. That compeltely killed the
service. I introduced the DavLockDiscovery directive to work it around.
--
Emmanuel Dreyfus
manu@netbsd.org
Re: mod_dav_fs locking / Re: apr_dbm and concurrency
Posted by Emmanuel Dreyfus <ma...@netbsd.org>.
On Thu, Nov 23, 2023 at 05:36:06PM +0000, Joe Orton wrote:
> 3) in the mean time I worked up a PR for mod_dav_fs which adds a global
> mutex around the dbm lockdb use. This passes my stress tests for the
> first time.
How concurent is the stress test?
In the past, I have been badly hurt by a few WebDAV clients proactively
exploring the filesystem using locksdiscovery. That compeltely killed the
service. I introduced the DavLockDiscovery directive to work it around.
--
Emmanuel Dreyfus
manu@netbsd.org
mod_dav_fs locking / Re: apr_dbm and concurrency
Posted by Joe Orton <jo...@redhat.com>.
Adding dev@httpd to a dev@apr thread about apr_dbm locking being broken.
On Sun, Nov 12, 2023 at 07:43:13AM -0600, Greg Stein wrote:
> Or, apps can stick to an older APR. ... we don't have to carry this forward
> into future versions of APR (IMO).
>
> To your point, it is probably a single page with code samples to show how
> to do K/V with sqlite, to replace the apr_dbm calls. That's gonna be way
> easier than locking code integrated into apr_dbm.
This seems reasonable to me for the mod_dav use case where the database
is an implementation detail which users don't care about. For other use
cases in httpd I'm not so sure, but saying "dbm is dead, sqlite is the
way" is probably possible for all of them. It does mean someone writing
a lot of new modules to replace mod_auth*dbm etc tho ;)
There are a few alternative approaches I looked at:
1) we could hack fcntl-based locks to work on Linux by using F_OFD_SETLK
etc which a sane locking model rather than the weird/dumb F_SETLK which
has the traditional/POSIX non-thread-safe model. Linux-specific, so...
2) try to shoehorn "proper" locking into apr_dbm is hard because there
isn't a suitable locking primitive that can be used at this level. Maybe
the only approach that might work would be filesystem-based locking
based off open/O_EXCL but this is not exactly reliable.
3) in the mean time I worked up a PR for mod_dav_fs which adds a global
mutex around the dbm lockdb use. This passes my stress tests for the
first time. Kind of ugly but this seems like the least ugly option at
this point other than the "start again with sqlite":
https://github.com/apache/httpd/pull/395
Any comments/review/flames from either dev@ welcome.
Regards, Joe
mod_dav_fs locking / Re: apr_dbm and concurrency
Posted by Joe Orton <jo...@redhat.com>.
Adding dev@httpd to a dev@apr thread about apr_dbm locking being broken.
On Sun, Nov 12, 2023 at 07:43:13AM -0600, Greg Stein wrote:
> Or, apps can stick to an older APR. ... we don't have to carry this forward
> into future versions of APR (IMO).
>
> To your point, it is probably a single page with code samples to show how
> to do K/V with sqlite, to replace the apr_dbm calls. That's gonna be way
> easier than locking code integrated into apr_dbm.
This seems reasonable to me for the mod_dav use case where the database
is an implementation detail which users don't care about. For other use
cases in httpd I'm not so sure, but saying "dbm is dead, sqlite is the
way" is probably possible for all of them. It does mean someone writing
a lot of new modules to replace mod_auth*dbm etc tho ;)
There are a few alternative approaches I looked at:
1) we could hack fcntl-based locks to work on Linux by using F_OFD_SETLK
etc which a sane locking model rather than the weird/dumb F_SETLK which
has the traditional/POSIX non-thread-safe model. Linux-specific, so...
2) try to shoehorn "proper" locking into apr_dbm is hard because there
isn't a suitable locking primitive that can be used at this level. Maybe
the only approach that might work would be filesystem-based locking
based off open/O_EXCL but this is not exactly reliable.
3) in the mean time I worked up a PR for mod_dav_fs which adds a global
mutex around the dbm lockdb use. This passes my stress tests for the
first time. Kind of ugly but this seems like the least ugly option at
this point other than the "start again with sqlite":
https://github.com/apache/httpd/pull/395
Any comments/review/flames from either dev@ welcome.
Regards, Joe
Re: apr_dbm and concurrency
Posted by Greg Stein <gs...@gmail.com>.
On Sat, Nov 11, 2023 at 4:21 AM Nick Kew <ni...@apache.org> wrote:
> > On 9 Nov 2023, at 16:15, Greg Stein <gs...@gmail.com> wrote:
> >
> > Personally, I'd deprecate apr_dbm entirely, and direct all clients
> towards sqlite.
>
>...
> +1 to deprecting apr_dbm, provided we explain exactly why (so users
> are clear whether or not it matters to their use cases). The alternative
> would
> be smart locking code within apr_dbm, and I doubt that anyone wants to
> put that effort in.
>
> Applications can still use a dbm of their choice without the APR layer.
>
Or, apps can stick to an older APR. ... we don't have to carry this forward
into future versions of APR (IMO).
To your point, it is probably a single page with code samples to show how
to do K/V with sqlite, to replace the apr_dbm calls. That's gonna be way
easier than locking code integrated into apr_dbm.
Cheers,
-g
Re: apr_dbm and concurrency
Posted by Nick Kew <ni...@apache.org>.
> On 9 Nov 2023, at 16:15, Greg Stein <gs...@gmail.com> wrote:
>
> Personally, I'd deprecate apr_dbm entirely, and direct all clients towards sqlite.
>
> The DBMs were intended for simple key/value stores 20 years ago. We have much better tech now. Sqlite does K/V so much better, *and* it handles processes/threads. I really don't see a reason for DBM nowadays.
>
> Who can justify using (say:) ldbm over sqlite?
Legacy apps? I first used ndbm more than 30 years ago. Software from
that era is still in use, and might crop up in someone's work today.
I *think* I even wrote about the apr_dbm concurrency issue long ago,
in an era when non-thread-safe libraries were being adopted willy-nilly
and to point it out was to be labelled a stick-in-the-mud enemy of progress.
+1 to deprecting apr_dbm, provided we explain exactly why (so users
are clear whether or not it matters to their use cases). The alternative would
be smart locking code within apr_dbm, and I doubt that anyone wants to
put that effort in.
Applications can still use a dbm of their choice without the APR layer.
--
Nick Kew
Re: apr_dbm and concurrency
Posted by Greg Stein <gs...@gmail.com>.
Personally, I'd deprecate apr_dbm entirely, and direct all clients towards
sqlite.
The DBMs were intended for simple key/value stores 20 years ago. We have
much better tech now. Sqlite does K/V so much better, *and* it
handles processes/threads. I really don't see a reason for DBM nowadays.
Who can justify using (say:) ldbm over sqlite?
Cheers,
-g
On Thu, Nov 9, 2023 at 6:00 AM Joe Orton <jo...@redhat.com> wrote:
> On Mon, Sep 25, 2023 at 03:58:18PM +0100, Joe Orton wrote:
> > It is unspecified whether the apr_dbm.h interface is safe to use for
> > multiple processes/threads having r/w access to a single database.
> > Results appear to be:
> >
> > - sdbm, gdbm are safe
> > - bdb, ndbm are not safe
>
> I developed a better test case (event MPM + mod_dav + apr_dbm with
> parallel clients doing cycles of LOCK/UNLOCK)
>
> sdbm, gdbm and bdb all failed.
>
> gdbm and sdbm both use either fcntl- or flock-based locking, which
> doesn't work between threads. (apr_file_lock for sdbm)
>
> Unfortunately lmdb is also unsafe at least because *opening* database
> files has races. This is documented under the Caveats section of
> http://www.lmdb.tech/doc/ (which also says it relies on flock-based
> locking, though I only hit issues with it failing during open in
> testing)
>
> The only ways forward I see are either to give up (declare that apr_dbm
> is not safe to use from threads) which passes the buck to httpd, or
> implement a locking layer inside apr_dbm() and don't assume the
> underlying drivers do thread-safe locking at all.
>
>
Re: apr_dbm and concurrency
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Sep 25, 2023 at 03:58:18PM +0100, Joe Orton wrote:
> It is unspecified whether the apr_dbm.h interface is safe to use for
> multiple processes/threads having r/w access to a single database.
> Results appear to be:
>
> - sdbm, gdbm are safe
> - bdb, ndbm are not safe
I developed a better test case (event MPM + mod_dav + apr_dbm with
parallel clients doing cycles of LOCK/UNLOCK)
sdbm, gdbm and bdb all failed.
gdbm and sdbm both use either fcntl- or flock-based locking, which
doesn't work between threads. (apr_file_lock for sdbm)
Unfortunately lmdb is also unsafe at least because *opening* database
files has races. This is documented under the Caveats section of
http://www.lmdb.tech/doc/ (which also says it relies on flock-based
locking, though I only hit issues with it failing during open in
testing)
The only ways forward I see are either to give up (declare that apr_dbm
is not safe to use from threads) which passes the buck to httpd, or
implement a locking layer inside apr_dbm() and don't assume the
underlying drivers do thread-safe locking at all.
Re: apr_dbm and concurrency
Posted by Branko Čibej <br...@apache.org>.
On 27.09.2023 14:37, Joe Orton wrote:
> On Mon, Sep 25, 2023 at 05:37:00PM +0200, Branko Čibej wrote:
>> IIRC, Berkeley DB multi-process concurrency is managed through an on-disk
>> "register" file external to the actual key/value store. The key/value store
>> format is not affected by the presence of this file. The DB_REGISTER
>> mechanism was introduced in BDB 4.4 (now long defunct) and can be used for
>> both concurrency control and automatic database recovery. The client-side
>> code for this can be lifted from Subversion.
>>
>> (I was involved in designing this mechanism for BDB and implementing its use
>> in Subversion, but that was ages ago -- back in 2005. There may be better
>> ways do do this in newer versions of Berkeley DB).
>>
>> TL;DR: all upstream supported versions of BDB should have this mechanism
>> available and APR can detect if it's being used without changing the API,
>> and even "upgrade" existing databases with the register file on the fly
>> without affecting the actual database.
> Thanks a lot for the response Branko, this is helpful.
>
> I spent a lot more time playing with this and I can get apr_dbm to use a
> DB_ENV in the way you suggest with locking. However, the path handling
> becomes very complicated/impossible - with a DB_ENV the database is
> assumed to be inside the "db_home" directory but we want to load/save a
> database by absolute path name.
>
> Using BDB in this way also appears incompatible with apr_dbm since the
> _usednames API assumes that at most two state files are used and with a
> full BDB environment the state is a whole directory. (mod_dav relies on
> this feature working correctly)
>
> So I don't want to appear like I'm shrugging this off but *at best* it
> might be possible to fix BDB for some versions (possibly, in theory,
> with compat concerns), and that leaves older BDB releases plus NDBM
> still broken.
>
> As an httpd developer I really don't care about database internals, I
> just want this to work. I also know that Berkeley DB is deprecated and
> will be removed from my Linux distribution of choice, so I don't want to
> invest too much time in it. :(
Yes, the whole environment is a directory, not a file. Also we should
call it "Oracle Berkeley DB", as it was gobbled up shortly after the
register mechanism was made available...
Given all of the above it would be better to remove support from BDB
from httpd, or at least mark it "there but don't use unless you're
exceptionally adventurous."
-- Brane
Re: apr_dbm and concurrency
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Sep 25, 2023 at 05:37:00PM +0200, Branko Čibej wrote:
> IIRC, Berkeley DB multi-process concurrency is managed through an on-disk
> "register" file external to the actual key/value store. The key/value store
> format is not affected by the presence of this file. The DB_REGISTER
> mechanism was introduced in BDB 4.4 (now long defunct) and can be used for
> both concurrency control and automatic database recovery. The client-side
> code for this can be lifted from Subversion.
>
> (I was involved in designing this mechanism for BDB and implementing its use
> in Subversion, but that was ages ago -- back in 2005. There may be better
> ways do do this in newer versions of Berkeley DB).
>
> TL;DR: all upstream supported versions of BDB should have this mechanism
> available and APR can detect if it's being used without changing the API,
> and even "upgrade" existing databases with the register file on the fly
> without affecting the actual database.
Thanks a lot for the response Branko, this is helpful.
I spent a lot more time playing with this and I can get apr_dbm to use a
DB_ENV in the way you suggest with locking. However, the path handling
becomes very complicated/impossible - with a DB_ENV the database is
assumed to be inside the "db_home" directory but we want to load/save a
database by absolute path name.
Using BDB in this way also appears incompatible with apr_dbm since the
_usednames API assumes that at most two state files are used and with a
full BDB environment the state is a whole directory. (mod_dav relies on
this feature working correctly)
So I don't want to appear like I'm shrugging this off but *at best* it
might be possible to fix BDB for some versions (possibly, in theory,
with compat concerns), and that leaves older BDB releases plus NDBM
still broken.
As an httpd developer I really don't care about database internals, I
just want this to work. I also know that Berkeley DB is deprecated and
will be removed from my Linux distribution of choice, so I don't want to
invest too much time in it. :(
Regards, Joe
Re: apr_dbm and concurrency
Posted by Branko Čibej <br...@apache.org>.
On 25.09.2023 16:58, Joe Orton wrote:
> It is unspecified whether the apr_dbm.h interface is safe to use for
> multiple processes/threads having r/w access to a single database.
> Results appear to be:
>
> - sdbm, gdbm are safe
> - bdb, ndbm are not safe
>
> (Berkeley DB obviously can be used in a way which is safe for multiple
> r/w users but it appears to require using one of the more complicated
> modes of operation via a DB_ENV, and changing to that would not be
> backwards compatible with the current db format. Corrections welcome,
> not a database expert)
IIRC, Berkeley DB multi-process concurrency is managed through an
on-disk "register" file external to the actual key/value store. The
key/value store format is not affected by the presence of this file. The
DB_REGISTER mechanism was introduced in BDB 4.4 (now long defunct) and
can be used for both concurrency control and automatic database
recovery. The client-side code for this can be lifted from Subversion.
(I was involved in designing this mechanism for BDB and implementing its
use in Subversion, but that was ages ago -- back in 2005. There may be
better ways do do this in newer versions of Berkeley DB).
TL;DR: all upstream supported versions of BDB should have this mechanism
available and APR can detect if it's being used without changing the
API, and even "upgrade" existing databases with the register file on the
fly without affecting the actual database.
-- Brane