Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by wietse@porcupine.org
Wietse Venema:
> Hallvard Breien Furuseth:
> > Pushed to mdb.master.
> >
> > void assert_callback(MDB_env *env, const char *msg) {
> > void *ctx = mdb_env_get_userctx(env);
> > ...;
> > }
> >
> > mdb_env_create(&env);
> > mdb_env_set_userctx(env, ctx);
> > mdb_env_set_assert(env, assert_callback);
Having obtained the patch off-list, I confirm that this call-back
feature works satisfactorily, and that this ITS can be closed.
I am looking forward to the next LMDB release.
Wietse
7 years
Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by wietse@porcupine.org
Hallvard Breien Furuseth:
> Pushed to mdb.master.
>
> void assert_callback(MDB_env *env, const char *msg) {
> void *ctx = mdb_env_get_userctx(env);
> ...;
> }
>
> mdb_env_create(&env);
> mdb_env_set_userctx(env, ctx);
> mdb_env_set_assert(env, assert_callback);
1) Where can I find this patch? I would like to confirm that the
code works without surprises.
2) Will this code be part of LMDB 0.9.11 (or some other number)?
That allows me to use the correct #ifdef in my code.
3) What is the expected release time frame for this code? If it's
close enough to the Postfix 2.11 stable release late January, then
it can include LMDB support.
Wietse
7 years
Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by h.b.furuseth@usit.uio.no
On 2014-01-05 20:03, hyc(a)symas.com wrote:
> I made this comment to Hallvard as well. It should be:
>
> MDB_assert_func *
> mdb_env_set_assert(MDB_env *env, MDB_assert_func *func, void *info);
Yes, I missed that. Hadn't intended to take this ITS into such
a detour.
Maybe such a context argument can be useful outside asserts too
though, like a Python wrapper which wants the Python element
wrapping the MDB_env of a given txn.
If so, instead of passing the context to mdb_env_set_assert():
int mdb_env_set_userctx(env, void *ctx);
void *mdb_env_get_userctx(env);
assert() could still pass it out to the assert fallback.
--
Hallvard
7 years
Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by hyc@symas.com
wietse(a)porcupine.org wrote:
> Wietse Venema:
>>> env can be NULL. mdb_env_set_assert(env,cb) also sets a static
>>> variable with the callback for the asserts which do not know of an
>>> env. Could throw away that part later. Currently it only applies
>>> to one assert().
>>>
>>> The branch also invalidates a transaction on any page allocation
>>> error, since the callers do not always clean up afterwards. Too
>>> aggressive, can be reverted after updating the callers. But for
>>> now it means MDB_MAP_FULL & co won't let the user commit an
>>> inconsistent transaction.
>>
>> Thanks, this is more improvement than I asked for. I don't want to
>> slip my schedule, and to avoid last-minute surprises, I will postpone
>> the review and impact test of these improvements after the stable
>> Postfix release 2-3 weeks from today.
>
> It looks like you are switching from a stop-gap fix (bare-bones
> call-back) to a more permanent solution.
>
> In that case I must mention again that I need to register not only
> a call-back function pointer but also a void* pointer that is
> returned to me at call-back time.
>
> Getting the MDB_env at call-back time does not help me at all,
> because my programs open many databases (including non-LMDB).
I made this comment to Hallvard as well. It should be:
MDB_assert_func *
mdb_env_set_assert(MDB_env *env, MDB_assert_func *func, void *info);
with the info being passed thru to the callback.
> To find out which database is affected I need my own pointer to my
> own (DICT) object that encapsulates the LMDB database. Then I can
> decide if the problem is with a disposable cache. in which case the
> program can continue with degraded performance.
>
> Wietse
>
>
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
7 years
Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by wietse@porcupine.org
Wietse Venema:
> > env can be NULL. mdb_env_set_assert(env,cb) also sets a static
> > variable with the callback for the asserts which do not know of an
> > env. Could throw away that part later. Currently it only applies
> > to one assert().
> >
> > The branch also invalidates a transaction on any page allocation
> > error, since the callers do not always clean up afterwards. Too
> > aggressive, can be reverted after updating the callers. But for
> > now it means MDB_MAP_FULL & co won't let the user commit an
> > inconsistent transaction.
>
> Thanks, this is more improvement than I asked for. I don't want to
> slip my schedule, and to avoid last-minute surprises, I will postpone
> the review and impact test of these improvements after the stable
> Postfix release 2-3 weeks from today.
It looks like you are switching from a stop-gap fix (bare-bones
call-back) to a more permanent solution.
In that case I must mention again that I need to register not only
a call-back function pointer but also a void* pointer that is
returned to me at call-back time.
Getting the MDB_env at call-back time does not help me at all,
because my programs open many databases (including non-LMDB).
To find out which database is affected I need my own pointer to my
own (DICT) object that encapsulates the LMDB database. Then I can
decide if the problem is with a disposable cache. in which case the
program can continue with degraded performance.
Wietse
7 years
Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by wietse@porcupine.org
Hallvard Breien Furuseth:
> Drafted a branch "mdb/assert" in
> <http://folk.uio.no/hbf/OpenLDAP/mdb.git/> and on Ada, based
> roughly on Howard's original message to you. I didn't know about
> that at first, which has caused some confusion.
>
> So the callback usually receives an env after all, and a NULL
> context since I don't know what Howard intended there.
>
> Howard, do I push this? Feel free to grab it and tweak a bit:)
>
> env can be NULL. mdb_env_set_assert(env,cb) also sets a static
> variable with the callback for the asserts which do not know of an
> env. Could throw away that part later. Currently it only applies
> to one assert().
>
> The branch also invalidates a transaction on any page allocation
> error, since the callers do not always clean up afterwards. Too
> aggressive, can be reverted after updating the callers. But for
> now it means MDB_MAP_FULL & co won't let the user commit an
> inconsistent transaction.
Thanks, this is more improvement than I asked for. I don't want to
slip my schedule, and to avoid last-minute surprises, I will postpone
the review and impact test of these improvements after the stable
Postfix release 2-3 weeks from today.
Wietse
7 years
Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by h.b.furuseth@usit.uio.no
Drafted a branch "mdb/assert" in
<http://folk.uio.no/hbf/OpenLDAP/mdb.git/> and on Ada, based
roughly on Howard's original message to you. I didn't know about
that at first, which has caused some confusion.
So the callback usually receives an env after all, and a NULL
context since I don't know what Howard intended there.
Howard, do I push this? Feel free to grab it and tweak a bit:)
env can be NULL. mdb_env_set_assert(env,cb) also sets a static
variable with the callback for the asserts which do not know of an
env. Could throw away that part later. Currently it only applies
to one assert().
The branch also invalidates a transaction on any page allocation
error, since the callers do not always clean up afterwards. Too
aggressive, can be reverted after updating the callers. But for
now it means MDB_MAP_FULL & co won't let the user commit an
inconsistent transaction.
--
Hallvard
7 years
Re: (ITS#7774) LMDB assertion failure during Postfix cache cleanup
by wietse@porcupine.org
Hallvard Breien Furuseth:
> I wrote:
> >Wietse Venema writes:
> >> I can confirm that with LMDB-0.9.10 + ITS#7756 patch, the assertion
> >> failure goes away when I close the cursor (and its read transaction)
> >> before changing the map or the database, and restore the cursor
> >> afterwards. I guess this got broken when things were changed to use
> >> external locks in order to avoid world-writable lockfiles.
>
> Does this mean we can close this ITS?
You can close this ITS.
Wietse
7 years
Re: (ITS#7775) LMDB terminates Postfix daemon process without logfile record
by wietse@porcupine.org
Hallvard Breien Furuseth:
> wietse(a)porcupine.org writes:
> > Below is my "least painful" patch for both parties. What do you think?
>
> That's roughly what I wrote was thinking, yes.
>
> We haven't heard if it will go in at all, though:-) If this ITS
> is a reaction to ITS#7774, then the motivation is not catching
> an lmdb bug, but letting the user band-aid a user error. Still
> makes sense to me, lmdb lets the user screw up in many ways.
I agree that we're talking about a stop-gap API in the absence of
better error reporting.
And I am not talking about users violating the C language model
with invalid pointers and such; that would rightfully deserve a
core dump.
> --- a/libraries/liblmdb/mdb.c
> +++ b/libraries/liblmdb/mdb.c
Based on visual inspection, that code would work for me.
Now the question for me is whether this will be available by the
time Postfix 2.11 is released near the end of this month. Code freeze
begins in days.
I can't distribute LMDB support when daemons fall out of the sky
without logfile message, whether it's your mistake, mine, or some
middleperson distro maintainer. Prior art: Postfix does not support
GnuTLS for the same reason. That would be a support problem.
Wietse
7 years