Re: [PATCH v2 5/5] compact: provide user more information on after-compaction failures

Subject: Re: [PATCH v2 5/5] compact: provide user more information on after-compaction failures

Date: Thu, 14 Nov 2013 19:23:14 +0200

To: David Bremner, Jani Nikula, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


On Thu, Nov 14 2013, David Bremner <david@tethera.net> wrote:

> Jani Nikula <jani@nikula.org> writes:
>
>> On Wed, 13 Nov 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>> After database has been compacted, there are steps to put the new
>>> database into place -- and these steps may fail. In case such
>>> failure happens, provide better information how to resolve it.
>>
>> I disagree with having a library spew all this information out. For each
>> case, I think it should be sufficient to just say what happened
>> (e.g. "rename a -> b failed" + strerror). I don't think a library's
>> error messages should be a tutorial on how to fix things.
>>
>
> I can live with whatever the concensus level of verbosity, but not the
> fprintf's; as I mentioned, for once we have a log hook.

Jani is right there that *library* should not dump all that info
to ... stderr. If so much info is printed, the client code should
do that -- but doing that cleanly is not a trivial job. The idea of
amending the namual page to inform user is a good one.

The log hook in it's current form is problematic as it doesn't provide
way to distinguish progress reporting from error reporting. Currently
lib/database.cc writes error messages with fprintf(stderr, ...) everywhere.

I suggest that this problem is fixed in one big sweep during 0.18
development -- the suggestion Jani pastebin'd a few days ago is
a good one and I'm willing to take part of that development...
And now take this approach of fprintf()ing (basically I would
also ask developers using the library wait for 0.18 before starting
to use the compact functionality (if ever), as the we have yet
another soname bump with changing interface coming...


Just that the log interface needs to be in format

void (*log_cb)(void * closure, int level, const char * message)

And we need to decide between the (non-NIH) syslog levels vs.
our own levels...

This way we can distinguish debug (DEBUG), progress (NOTICE, INFO) 
and error (WARN, EER, CRIT, ALERT, EMERG) conditions.


For someone who wished logf(closure, level, fmt, ...) we can provide
helper functions

_log_printf(log_cb, closure, level, fmt, ...) and
_log_vprintf(log_cb, closure, level, fmt, ap)


>
> That might also somewhat comfort Jani, since the library client has the
> option of ignoring the spewing.

String maybe!

>
> d

Tomi


Thread: